On Mon, 14 Sep 2026 11:32:29 GMT, Albert Mingkun Yang <[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 19 commits: >> >> - 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 >> - Merge branch 'master' into JDK-8358959 >> >> # Conflicts: >> # src/hotspot/cpu/riscv/riscv.ad >> - Code format >> - Merge remote-tracking branch 'upstream/master' into JDK-8358959 >> - RISC-V: Use register operands for Zalasr access helpers >> - Code Format >> - Fix for merge code >> - Merge branch 'master' into JDK-8358959 >> >> # Conflicts: >> # src/hotspot/cpu/riscv/downcallLinker_riscv.cpp >> # src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp >> # src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp >> - ... and 9 more: https://git.openjdk.org/jdk/compare/89365a0f...479d1168 > > src/hotspot/cpu/riscv/assembler_riscv.hpp line 1189: > >> 1187: assert_cond(UseZalasr); >> 1188: >> 1189: if (funct5 == ZALASR_LOAD_ACQUIRE) { > > I wonder whether `if constexpr` can be used here. Fixed. > src/hotspot/cpu/riscv/assembler_riscv.hpp line 1204: > >> 1202: memory_order = (memory_order == aqrl) ? aqrl : rl; >> 1203: } else { >> 1204: ShouldNotReachHere(); > > Can probably use `static_assert(funct5 == ...` instead of this dynamic > checking. Fixed. > src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1980: > >> 1978: >> 1979: void LIR_Assembler::load_volatile(LIR_Address* from_addr, LIR_Opr >> dest, BasicType type, CodeEmitInfo* info) { >> 1980: if (!UseZalasr) { > > This feels a bit unbalanced; how about: > > > if (UseZalasr) { > load_acquire(from_addr, dest, type, info); > } else { > load_unordered(from_addr, dest, type, /* wide */ false, info); > membar_acquire(); > } Fixed. > src/hotspot/cpu/riscv/jniFastGetField_riscv.cpp line 148: > >> 146: __ fmv_x_d(result, f28); // d{63--0}-->x >> 147: break; >> 148: } > > Preexisting: why this path can't use fall-through for float/double? Fixed. > src/hotspot/cpu/riscv/riscv.ad line 1288: > >> 1286: >> 1287: bool needs_acquiring_load(const Node *n) { >> 1288: if (!UseZalasr) return false; > > I'd suggest breaking this into multiple lines to make the early-return more > visible. Fixed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4007039390 PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4007039034 PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4007038410 PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4007037359 PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4007036546
