RFR: 8368303: AlwaysAtomicAccesses is excessively strict [v2]

[Aleksey Shipilev]

On Thu, 2 Oct 2025 17:14:41 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> In C1, the experimental flag `AlwaysAtomicAccesses` treats accesses to all fields as if they were volatile fields. This is correct but excessive: we only need single-copy atomicity to satisfy `AlwaysAtomicAccesses`. 
>> 
>> We need this in order to allow progress on JDK-8365147.
>
> Andrew Haley has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Next try

This is a good compromise.

At the risk of dragging it ever further, I think we can make it ever crisper by catching the fact we are going for `volatile_field_(load|store)` either for proper volatile, or because we wanted to force atomicity.


 bool needs_atomic = AlwaysAtomicAccesses && !access_is_atomic(result->type());
 ...
 if ((is_volatile || needs_atomic) && !needs_patching) {
   gen->volatile_field_store(...)
 }


It looks like we really want to push the `membar()`-s down to `LIRGenerator`, though. Especially as AArch64 emits more membars on top:


void LIRGenerator::volatile_field_load(LIR_Address* address, LIR_Opr result,
                                       CodeEmitInfo* info) {
  // 8179954: We need to make sure that the code generated for
  // volatile accesses forms a sequentially-consistent set of
  // operations when combined with STLR and LDAR.  Without a leading
  // membar it's possible for a simple Dekker test to fail if loads
  // use LD;DMB but stores use STLR.  This can happen if C2 compiles
  // the stores in one method and C1 compiles the loads in another.
  if (!CompilerConfig::is_c1_only_no_jvmci()) {
    __ membar();
  }
  __ volatile_load_mem_reg(address, result, info);
}


But that can be left for future refactoring.

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27432#pullrequestreview-3303449966