RFR 8201799: Build failures after JDK-8195099 (Concurrent safe-memory-reclamation mechanism) (original) (raw)

Doerr, Martin martin.doerr at sap.com
Tue Apr 24 17:02:21 UTC 2018

Hi Erik,

I was not suggesting removing the more conservative ordering property, only allowing using seqcst, when you know that is the correct semantics. Ok, thanks for clarifying.

There are both leading sync and trailing sync conventions for implementing C++11 seqcst on PPC I'm not convinced that this can be freely selected. Is there a GCC switch? What about xlC on AIX?

The trailing sync convention would have a different weakness: Store + Cmpxchg The store will have to be SeqCst in order to generate correct code on PPC64.

Even if we can select it in C++, it would be unfortunate if it differs from JMM. JMM should not be changed for the sake of C++ implementation cleanup. We have "JEP 188: Java Memory Model Update" for that.

From the other mail:

With C++11, you can choose both what memory ordering semantics successful CAS and failing CAS should have You're right. We just need to use it correctly.

Thanks and best regards, Martin

-----Original Message----- From: Erik Österlund [mailto:erik.osterlund at oracle.com] Sent: Dienstag, 24. April 2018 18:25 To: Doerr, Martin <martin.doerr at sap.com>; Andrew Haley <aph at redhat.com>; David Holmes <david.holmes at oracle.com>; Aleksey Shipilev <shade at redhat.com>; hotspot-dev at openjdk.java.net; Lindenmaier, Goetz <goetz.lindenmaier at sap.com> Subject: Re: RFR 8201799: Build failures after JDK-8195099 (Concurrent safe-memory-reclamation mechanism)

Hi Martin,

On 2018-04-24 15:43, Doerr, Martin wrote:

Hi Erik,

I highly appreciate if we can get the VM code to work correctly with C++ 2011 semantics on PPC64. But I think this needs to be done very carefully in order to avoid missing sync instructions between CAS and Load.

I was not suggesting removing the more conservative ordering property, only allowing using seq_cst, when you know that is the correct semantics.

E.g. the new global counter implementation uses (after fix): Atomic::add(memoryorderconservative) + OrderAccess::loadacquire

In C++2011 this should become: Cmpxchg SeqCst + Load Seq Cst Cmpxchg SeqCst + any other Load would be incorrect on PPC64. So we will need to double-check all corresponding loads.

This might be getting out of topic, but here goes...

There are both leading sync and trailing sync conventions for implementing C++11 seq_cst on PPC. What you are saying is true for the leading sync convention, but not for the trailing sync convention. In HotSpot, I think it is probably more desirable to bind the expectations for all seq_cst accesses to have trailing sync convention to avoid this issue of seq_cst store followed by load_acquire being reordered. I don't think we want a situation where PPC implements leading sync (despite being able to choose trailing sync), when AArch64 uses trailing sync. But perhaps this is derailing a bit...

I've seen that you have moved the supportIRIWfornotmultiplecopyatomiccpu fence into RawAccessBarrier::loadinternal. Ideally, RawAccessBarrier::storeinternal should use OrderAccess::releasestore without fence in case of supportIRIWfornotmultiplecopyatomiccpu, but I'm not requesting to change this now. We'd get this by moving to C++ 2011 semantics (which assumes supportIRIWfornotmultiplecopyatomiccpu), right?

That code should really call into Atomic/OrderAccess with seq_cst, and let the platform layer do what it needs to do.

Thanks, /Erik

Best regards, Martin

-----Original Message----- From: Erik Österlund [mailto:erik.osterlund at oracle.com] Sent: Montag, 23. April 2018 12:41 To: Andrew Haley <aph at redhat.com>; Doerr, Martin <martin.doerr at sap.com>; David Holmes <david.holmes at oracle.com>; Aleksey Shipilev <shade at redhat.com>; hotspot-dev at openjdk.java.net; Lindenmaier, Goetz <goetz.lindenmaier at sap.com> Subject: Re: RFR 8201799: Build failures after JDK-8195099 (Concurrent safe-memory-reclamation mechanism) Hi Andrew, On 2018-04-23 12:20, Andrew Haley wrote: On 04/18/2018 05:37 PM, Doerr, Martin wrote:

It'd be great if we could specify semantics for Atomic:add like we do for Atomic::cmpchgx. However, the double-fence is a very bad default from performance perspective. I wonder if PPC64 is the only platform which gets hit. Probably not. Would it be acceptable to set the default to memoryorderacquirerelease and specify memoryorderconservative for the new usage? I think this would fit perfectly for PPC64, but some people may not like it. One could say PPC64 is the problem, but one could also say the VM code is the problem �� Indeed. In as much as we use stronger memory ordering than we need in more than one place, the VM code is the problem. I don't really like the straight forward fix to insert a fence with #ifdef AIX. I agree. It looks to me like it's sufficient if the increment of globalcounter and the load of thread->getrcucounter() are sequentially consistent. On at least one platform, AArch64, we can do that without additional fencing. I would also like to have seqcst available. In the Access API, I need MOSEQCST for JMM volatile support. I would like the semantic requirements to go straight into Atomic, instead of doing a "if (supportIRIWfornotmultiplecopyatomiccpu) { OrderAccess::fence(); }" dance in shared code, that may or may not make sense at a given platform (among other reasons because it enforces whether leading or trailing sync convention is used in shared code, rather than leave that decision to the platform level). Plus I have a mouse pad that says I love sequential consistency, although that might not be a valid argument to introduce this to Atomic. Thanks, /Erik

More information about the hotspot-dev mailing list