On Wed, 16 Sep 2026 07:25:22 GMT, Guoxiong Li <[email protected]> wrote:
>> Gui Cao has updated the pull request with a new target base due to a merge >> or a rebase. The pull request now contains 26 commits: >> >> - Merge remote-tracking branch 'upstream/master' into JDK-8358959 >> - Update for Axel code review >> - RISC-V: Gate native AtomicAccess Zalasr dispatch on a post-validated flag >> - Code format >> - RISC-V: Zalasr code review followups >> - Apply code review >> - RISC-V: Use Zalasr for the ordered accesses in AtomicAccess >> - Merge remote-tracking branch 'upstream/master' into JDK-8358959 >> - RISC-V: Align C1 volatile load dispatch with AArch64 >> - Merge remote-tracking branch 'upstream/master' into JDK-8358959 >> - ... and 16 more: https://git.openjdk.org/jdk/compare/b0ac803f...bcc84174 > > src/hotspot/cpu/riscv/gc/shenandoah/shenandoahBarrierSetAssembler_riscv.cpp > line 603: > >> 601: "acquire path requires address to be base-only"); >> 602: __ lw_aq(dst, src.base()); >> 603: __ zext(dst, dst, 32); > > Could we use `lwu_acquire` here? It seems we need to judge whether`UseZalasr` > is true. Or we should add an assert statement to judge that `UseZalasr` is > true. Fixed. > src/hotspot/cpu/riscv/gc/shenandoah/shenandoahBarrierSetAssembler_riscv.cpp > line 611: > >> 609: assert(src.getMode() == Address::base_plus_offset && src.offset() >> == 0, >> 610: "acquire path requires address to be base-only"); >> 611: __ ld_aq(dst, src.base()); > > Could we use `ld_acquire` here? Same as above. Added `assert(UseZalasr, ...)` next to the existing addressing-mode assert, rather than switching to `ld_acquire` (which would just re-check `UseZalasr` at runtime and pick the same instruction, since this branch is only reachable when it's already true). Fixed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4027404398 PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4027431173
