RFR(S): 8229422: Taskqueue: Outdated selection of weak memory model platforms

[Doerr, Martin]

Hi Thomas,

good proposal.

Here's the minimal version:
http://cr.openjdk.java.net/~mdoerr/8229422_multi-copy-atomic/webrev.02/

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?

Best regards,
Martin


> -----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
> 
> Hi,
> 
> 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
> > http://cr.openjdk.java.net/~mdoerr/8229422_multi-copy-
> atomic/webrev.00/
> > 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:
> > http://cr.openjdk.java.net/~mdoerr/8229422_multi-copy-
> atomic/webrev.01/
> > 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.
> webrev.01.
> 
>    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. :)
> 
> Thanks,
>    Thomas
>