On Tue, 8 Sep 2026 03:38:12 GMT, Albert Mingkun Yang <[email protected]> wrote:
>> Gui Cao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Code format
>
> src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1976:
>
>> 1974: // Volatile stores need no counterpart here:
>> BarrierSetC1::store_at_resolved
>> 1975: // already brackets them with a leading release fence and a trailing
>> full fence.
>> 1976: void LIR_Assembler::load_volatile(LIR_Address* from_addr, LIR_Opr
>> dest, BasicType type, CodeEmitInfo* info) {
>
> `LIRGenerator::volatile_field_load` and `LIR_Assembler::load_volatile` appear
> to be two stages of the same path:
>
> `volatile_field_load` -> `lir_move_volatile` -> `volatile_move_op` ->
> `load_volatile`
>
> Would it make sense to keep the `UseZalasr` decision entirely in
> `LIR_Assembler::load_volatile`?
>
> `volatile_field_load` could then only emit `volatile_load_mem_reg`, while
> `volatile_move_op` would always call `load_volatile` for address-to-register
> moves. `load_volatile` could use the current Zalasr implementation when
> enabled, and otherwise fall back to:
>
>
> move_op(src, dest, type, lir_patch_none, info, /* wide */ false);
> membar_acquire();
>
>
> That should preserve the current generated code while keeping the choice
> between `load.aq` and `load; fence` in one place in C1. It would also avoid
> having the generator depend on how the assembler implements the volatile load.
Hi, Thanks for the suggestion. I’ve aligned the dispatch with AArch64:
`volatile_move_op` routes loads through `mem2reg(src, dest, type,
lir_patch_none, info, /* wide */ false, /* is_volatile */ true);`, which calls
load_volatile.
Acquire ordering is now handled entirely by `load_volatile`, using a Zalasr
`load-acquire` when enabled or `load_unordered` followed by `membar_acquire()`
otherwise. Consequently, `volatile_field_load` only emits
`volatile_load_mem_reg` and no longer needs to emit a separate acquire barrier.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r3986131339
