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

[Doerr, Martin]

Hi Lutz,

thanks for reviewing.

Can I get more reviews, please?

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