On Fri, 11 Sep 2026 05:15:53 GMT, Gui Cao <[email protected]> wrote: >> 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.
Hi, I think we could create a small separate PR for this: https://github.com/openjdk/jdk/pull/32833 . If we handle this part of the logic here, the subsequent Zalasr changes will be much cleaner — we would only need to add some Zalasr-specific adaptation inside load_volatile. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r3986634856
