RFR(M): 8202080: Introduce ordering semantics for Atomic::add and other RMW atomics

[Robbin Ehn]

On 2018-05-03 12:50, Doerr, Martin wrote:
> Hi Lutz,
> 
> thanks for reviewing.
> 
> Can I get more reviews, please?

I think this looks good, thanks for fixing!

/Robbin

> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Schmidt, Lutz
> Sent: Mittwoch, 2. Mai 2018 16:20
> To: Doerr, Martin <martin.doerr at sap.com>; David Holmes <david.holmes at oracle.com>; Erik Österlund <erik.osterlund at oracle.com>; hotspot-runtime-dev at openjdk.java.net; Aleksey Shipilev <shade at redhat.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Michihiro Horie (HORIE at jp.ibm.com) <HORIE at jp.ibm.com>; Andrew Haley (aph at redhat.com) <aph at redhat.com>; John Paul Adrian Glaubitz <glaubitz at physik.fu-berlin.de>
> Subject: Re: RFR(M): 8202080: Introduce ordering semantics for Atomic::add and other RMW atomics
> 
> Hi Martin,
> 
> your changes look good.
> 
> For the additional synchronization on s390 in particular, I do not expect a serious performance impact. The checkpoint synchronization part has always been the expensive, but rarely required, part. PPC may be another story. We knoow that a full-blown sync really hurts. We will see...
> 
> Thanks,
> Lutz
> 
> On 26.04.18, 16:23, "hotspot-runtime-dev on behalf of Doerr, Martin" <hotspot-runtime-dev-bounces at openjdk.java.net on behalf of martin.doerr at sap.com> wrote:
> 
>      Hi,
>      
>      I have renamed "cmpxchg_memory_order" to "atomic_memory_order" and added support to all Read-Modify-Write atomics with support for all C++11 semantics except seq_cst. The latter has issues on PPC64 so we're currently using our own memory_order_conservative instead, which is the default.
>      
>      Please review my new webrev:
>      http://cr.openjdk.java.net/~mdoerr/8202080_atomic_add/webrev.01/
>      
>      We'll test Windows, MacOS, linux x86+ppc64+s390, AIX and Solaris SPARC.
>      It'd be great if somebody could check arm/aarch64 and zero.
>      
>      This change may enable optimizations for arm/aarch64 (not yet included).
>      
>      Best regards,
>      Martin
>      
>      
>      -----Original Message-----
>      From: Doerr, Martin
>      Sent: Donnerstag, 26. April 2018 11:20
>      To: 'David Holmes' <david.holmes at oracle.com>; Erik Österlund <erik.osterlund at oracle.com>; hotspot-runtime-dev at openjdk.java.net; Aleksey Shipilev <shade at redhat.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Michihiro Horie (HORIE at jp.ibm.com) <HORIE at jp.ibm.com>
>      Subject: RE: RFR(M): 8202080: Introduce ordering semantics for Atomic::add
>      
>      Hi David,
>      
>      sounds like we are on the same page, now. I think we should have some kind of summary of what was analyzed. But that belongs to the other thread (JDK- 8154736).
>      
>      > The bugs in this kind of code are very subtle and rarely exposed through normal levels of testing.
>      Right. We have experienced this often enough. So it is definitely in our interest to have correct memory barriers. Mistakes will hit us (PPC64). So I appreciate that you insist on careful analysis.
>      
>      I'll prepare a new webrev with conservative semantics for all read-modify-write atomics by default.
>      
>      Best regards,
>      Martin
>      
>      
>