RFR: 8252857: AArch64: Shenandoah C1 CAS is not sequentially consistent [v2]

[Aleksey Shipilev]

On Tue, 22 Sep 2020 09:55:56 GMT, Nick Gasson <ngasson at openjdk.org> wrote:

>> C1 atomic operations are supposed to be sequentially consistent by
>> default but the variant in the Shenandoah C1 barrier set assembler only
>> provides a half-barrier when the CAS succeeds. Added a trailing full
>> barrier and load-acquire to exactly match the non-Shenandoah C1 CAS
>> implementation. This prevents any memory accesses following the CAS
>> operation being observed before it.
>> 
>> Also adjusted the documentation for ShenandoahBarrierSetAssembler
>> ::cmpxchg_oop as it currently claims to be sequentially consistent by
>> default but it's not clear what the "default" values of acquire and
>> release should be, and the comment for acquire/release implies to me
>> that setting them to true would relax the ordering guarantees but
>> actually it's the opposite. I tried to simplify this and make it less
>> ambiguous.
>> 
>> One other thing I noticed when reading this: the comment
>> 
>>    // Step 4. CAS has failed because the value most recently fetched
>>    // from addr (which is now held in tmp1) ...
>> 
>> is wrong on non-LSE systems because tmp1 is rscratch1 and that is
>> clobbered by the call to __ cmpxchg() at the end of step3. Although it
>> doesn't matter because the value in tmp1 is not used after that point.
>> Adjusted the comment to clarify this.
>> 
>> Tested hotspot_all_no_apps, jdk_core plus jcstress with -jvmArgs
>> '-XX:+UseShenandoahGC -XX:ShenandoahGCMode=iu -XX:TieredStopAtLevel=1'.
>
> Nick Gasson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Only insert trailing barrier when is_c1_or_interpreter_only()

Marked as reviewed by shade (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/280