RFR(S): 8229422: Taskqueue: Outdated selection of weak memory model platforms
david.holmes at oracle.com
Sun Sep 1 22:52:57 UTC 2019
On 30/08/2019 9:14 pm, Doerr, Martin wrote:
> Hi Thomas,
> good proposal.
> Here's the minimal version:
> I've removed the compiler part. I can create a separate issue for making C1 and C2 consistent.
> Arm32/aarch64 folks can create new issues if they like further changes.
> I don't have any further requirements for s390 and PPC64 at the moment.
> Can I consider it as reviewed by Thomas, David and Derek?
The Aarch64 comment begs the question as to why it is not defining
CPU_MULTI_COPY_ATOMIC, but otherwise the changes are okay. My suggestion
for Aarch64 would be:
// Aarch64 was not originally defined as multi-copy-atomic, but now is.
// See: "Simplifying ARM Concurrency: Multicopy-atomic Axiomatic and
Operational Models for ARMv8"
// So we could #define CPU_MULTI_COPY_ATOMIC but historically we have
not done so.
> Best regards,
>> -----Original Message-----
>> From: Thomas Schatzl <thomas.schatzl at oracle.com>
>> Sent: Freitag, 30. August 2019 10:33
>> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-
>> dev at openjdk.java.net; 'hotspot-compiler-dev at openjdk.java.net' <hotspot-
>> compiler-dev at openjdk.java.net>
>> Cc: hotspot-gc-dev at openjdk.java.net; David Holmes
>> (david.holmes at oracle.com) <david.holmes at oracle.com>; Derek White
>> <derekw at marvell.com>
>> Subject: Re: RFR(S): 8229422: Taskqueue: Outdated selection of weak
>> memory model platforms
>> On 26.08.19 15:04, Doerr, Martin wrote:
>>> Hi all,
>>> I had noticed that the platforms selection which need a fence in
>> taskqueue.inline.hpp should get updated.
>>> My initial webrev
>>> was already reviewed on hotspot-gc-dev. It is an attempt to make things
>> more consistent, especially the property "CPU_MULTI_COPY_ATOMIC".
>>> Also the compiler constant
>> "support_IRIW_for_not_multiple_copy_atomic_cpu" depends on this
>> property (currently only used on PPC64).
>>> We could go one step further and move even more #defines into the
>> platform files to give platform maintainers more control.
>>> I haven't got feedback from arm/aarch64 folks about this addition, yet:
>>> With this proposal, each platform which is "CPU_MULTI_COPY_ATOMIC" is
>> supposed to define this macro.
>>> Other platforms must define
>> SUPPORT_IRIW_FOR_NOT_MULTI_COPY_ATOMIC_CPU and
>> IRIW_WITH_RELEASE_VOLATILE_IN_CONSTRUCTOR for fine-grained control
>> of the memory ordering behavior.
>>> We can even control them dynamically (added an experimental switch for
>> PPC64 as an example).
>>> Note that neither webrev.00 nor webrev.01 contain any functional changes
>> other than the taskqueue update for s390 (and the experimental switch for
>> PPC64 in webrev.01).
>>> Feedback is welcome. Also if you have a preference wrt. webrev.00 vs.
>> for pushing I would prefer the minimal amount of changes to solve the
>> original issue, and move all other changes to a different CR.
>> Also, I would prefer if all globalDefinitions files contained all
>> defines, commented out if needed. I.e. to try to show that not defining
>> a particular macro has been deliberate and not an oversight.
>> (Like in the 00 webrev where the code at least states for aarch64:
>> 37 // aarch64 is not CPU_MULTI_COPY_ATOMIC
>> I am aware that this is not correct given new information, but in
>> context of the CR it is/was)
>> Further, let's avoid "TODOs" in the sources, the correct place for those
>> is JIRA imho. :)
More information about the hotspot-gc-dev