RFR: 8310939: [c1] The visibility of write-volatile requires membar_volatile instead of membar

[SUN Guoyun]

On Wed, 28 Jun 2023 10:22:57 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> For c1 now, a volatile write case:
>> 
>> membar_release // LoadStore | StoreStore
>> write volatile
>> membar
>> 
>> Just like c2, here `membar` should be defined `membar_volatile` clearly, then for risc-v, ppc and loongarch can use StoreLoad for `membar_volatile` for better performance.
>> 
>> Testing:
>> GHA testing
>> jtreg tier1-3 for loongarch64
>
> We need to be really careful about this, because we had the issues with inconsistent barrier schemes in C1 and C2:
> https://bugs.openjdk.org/browse/JDK-8179954
> 
> Modern jcstress is able to run tests with one side compiled by C1 and another one compiled by C2, exactly to catch these things. So I suggest you get at least one clean jcstress run with this patch, at least on AArch64 (with both `-UseLSE` and `+UseLSE`).

@shipilev thanks for the reminder.   After the patch for AArch64, we also can see

  STLR #1 -> [x] | STLR #1 -> [y]

  DMB | DMB
  LDR [y] -> r1 | LDR [x] -> r1
  DMB LD | DMB LD
  
 On AArch64, Assembler::AnyAny and Assembler::StoreLoad also be defined ISH, so not found regression. And this patch can not cause regression on X86 and S390, because also used same instruction in membar() and membar_volatile().

I had run jcstress `org.openjdk.jcstress.samples.jmm.basic.BasicJMM_07_Consensus` on AArch64 and LoongArch64, and result is

`(Results: 102 planned; 102 passed, 0 failed, 0 soft errs, 0 hard errs)`

But I am not sure for risc-v, ppc. Maybe please @RealFYang @TheRealMDoerr help to confirm this patch.

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

PR Comment: https://git.openjdk.org/jdk/pull/14677#issuecomment-1612350131