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

David Holmes david.holmes at oracle.com
Mon Aug 12 22:46:21 UTC 2019


Hi Martin,

On 13/08/2019 3:33 am, Doerr, Martin wrote:
> Hi,
> 
> I recently noticed that the selection of weak memory model platforms is 
> outdated in the task queue implementation:
> 
> s390 is unnecessarily treated as weak memory model platform.
> 
> I could simply fix it by adding “defined S390”, but I’d like to get rid 
> of the platform whitelist in the middle of the implementation.
> 
> My favorite implementation looks like this:
> 
> http://cr.openjdk.java.net/~mdoerr/8229422_multi-copy-atomic/webrev.00/
> 
> I’ve moved the selection to the platform files. They define if they have 
> the required property.

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?

> I have also cleaned up some PPC64 related stuff which should use the 
> same property.

Can't comment on ppc64 specifics.

> The change really improves only s390. It’s only cleanup for other ones 
> (no functional change).

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.

Thanks,
David
-----

> I’d like to get reviews from GC first. I guess I’ll have to get reviews 
> from runtime and compiler afterwards, too.
> 
> Thanks and best regards,
> 
> Martin
> 



More information about the hotspot-gc-dev mailing list