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

[Derek White]

Hi Martin,

This is a good area to clean up! I have 2 issues to consider with this patch:

Re - the AArch64 side of things:
Arm retconned the ARMv8 spec [1][2] to decide that multi-copy atomicity was a good idea after all (after checking that no CPU implementations took advantage of this level of weakness).

However, setting CPU_MULTI_COPY_ATOMIC for AArch64 would result in changing behavior (removing fence in taskqueue) that should be looked at and tested by the aarch64 folks, so if Andrew Haley agrees, I suggest deferring changing this AArch64 behavior to a separate issue.

BTW, this could be a nice improvement for AArch64 - Thanks for bringing this up!

	[1] https://www.cl.cam.ac.uk/~pes20/armv8-mca/
	[2] https://bugs.openjdk.java.net/browse/JDK-8165058

Re - the patch generally:

There changes around CPU_NOT_MULTIPLE_COPY_ATOMIC in src/hotspot/share/utilities/globalDefinitions.hpp are not simply a change to GC behavior for S390, but are changing interpreter and compiler behavior arm32 (and maybe Aarch64?).

I believe this change will remove required fences from arm32 interpreter and Jitted code relating to JMM volatile accesses. 

In any case I think it should be reviewed beyond the context of hotspot-gc-dev.

Thanks again bringing this up!
 - Derek

-----Original Message-----
From: hotspot-gc-dev <hotspot-gc-dev-bounces at openjdk.java.net> On Behalf Of Doerr, Martin
Sent: Tuesday, August 13, 2019 6:30 AM
To: David Holmes <david.holmes at oracle.com>; hotspot-gc-dev at openjdk.java.net; Kim Barrett <kim.barrett at oracle.com>
Subject: [EXT] RE: RFR(S): 8229422: Taskqueue: Outdated selection of weak memory model platforms

External Email

----------------------------------------------------------------------
Hi Kim and David,

thank you for looking into this issue.

@Kim:
I've replied to your comment in the issue.

@David:
> I find the inversion of the ifdef slightly confusing. I also don't 
> like a comment to say we don't have a given property. Wouldn't it be 
> better to set CPU_MULTI_COPY_ATOMIC to 0 or 1 as appropriate?
Hmm. We could change that. I'm not sure what is better.
I think it should be designed such that correct usage is easy and wrong usage is difficult.

It has already happened that people used an #ifdef for a macro which is always defined (0 or 1) by mistake.
That's why I'm not a big fan of defining things to 0 or 1.

With the #define or not define approach, all platforms except those which explicitly specify the property are conservatively treated as non-multi-copy atomic.

But if your version is preferred by all reviewers, I can use it.


> Can't comment on ppc64 specifics.
I'll ask for additional reviews once the main issue was reviewed.


> It's not at all obvious to me that the need for the fence() in 
> pop_global is directly related to CPU_MULTI_COPY_ATOMIC. I prefer to 
> see that define connected only with the IRIW issue as it currently is.
This was explained in the email thread a few emails later:
http://mail.openjdk.java.net/pipermail/hotspot-dev/2013-March/008853.html


Best regards,
Martin