On Wed, 16 Sep 2026 15:28:27 GMT, Gui Cao <[email protected]> wrote:
>> src/hotspot/cpu/riscv/riscv.ad line 5094:
>>
>>> 5092: instruct loadKlass(iRegPNoSp dst, memory mem)
>>> 5093: %{
>>> 5094: predicate(!needs_acquiring_load(n));
>>
>> Now, we don't have a `instruction` rule to match `needs_acquiring_load` is
>> true. Is it a intentional behavior?
>
> Intentional, and it's actually guaranteed by construction rather than just
> "not currently exercised":
>
> `needs_acquiring_load(n)` forwards to `n->as_Load()->is_acquire()`, which
> just checks the `_mo` field baked into the Load node at construction time.
> - For ordinary Java field loads (`LoadP`/`LoadN`), `_mo` is set to
> `MemNode::acquire` in `BarrierSetC2` 's decorator-resolution logic
> (`barrierSetC2.cpp`) whenever the field being accessed is volatile — that's
> exactly the path `loadP_volatile`/`loadN_volatile` exist for.
> - `LoadKlassNode` never goes through that path. Its constructor is private,
> and the only way to build one is `LoadKlassNode::make()` (`memnode.cpp`),
> which hardcodes `MemNode::unordered` unconditionally, for both the narrow and
> wide cases. There's no call path anywhere in the graph that can produce a
> `LoadKlassNode` with `_mo == acquire`.
>
> So `needs_acquiring_load(n)` is structurally always false for a `LoadKlass`
> node
Thanks for your explanation.
>> src/hotspot/cpu/riscv/templateTable_riscv.cpp line 2649:
>>
>>> 2647: __ bind(notVolatile);
>>> 2648: }
>>> 2649:
>>
>> Should we always generate `s{b|h|w|d}.rl` and `l{b|h|w|d}.aq` instructions
>> in interpreter when `UseZalasr` is true? Then we don't need such additional
>> check. It could be investigated and done in another PR.
>
> Agreed this would remove the extra check, but I'd like to keep it out of this
> PR — it's a bigger change than it looks. Right now the interpreter's volatile
> field accesses use the same plain-load/plain-store + explicit membar scheme
> regardless of UseZalasr (see getfield_or_static/putfield_or_static);
> volatile_load_leading_fence() only bolts on one extra fence to fix the
> specific cross-tier interop gap where C2 emits a bare s.rl but the
> interpreter's default load has no leading fence to pair with it via RVWMO
> rule 7.
Yes, it is also a large change. Someone could investigate this issue in the
future.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4028013205
PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4028038605
