On Wed, 16 Sep 2026 07:52:39 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/c1_LIRAssembler_riscv.cpp line 1994:
> 
>> 1992:   // address is computed into t1.
>> 1993:   Address addr = as_Address(from_addr);
>> 1994:   assert(addr.getMode() == Address::base_plus_offset, "unsupported 
>> addressing mode");
> 
> What about asserting `UseZalasr` is true at the beginning of the method?

Fixed.

> src/hotspot/cpu/riscv/gc/shenandoah/shenandoahBarrierSetAssembler_riscv.cpp 
> line 640:
> 
>> 638:       assert(dst.getMode() == Address::base_plus_offset && dst.offset() 
>> == 0,
>> 639:           "volatile path requires address to be base-only");
>> 640:       __ sw_rl(src, dst.base());
> 
> Same as `load_c2`. May use `sw_release` or assert `UseZalasr` is always true.

Fixed.

> src/hotspot/cpu/riscv/gc/z/z_riscv.ad line 164:
> 
>> 162:     guarantee($mem$$disp == 0, "impossible encoding");
>> 163:     z_store_barrier(masm, this, Address($mem$$Register), 
>> $src$$Register, $tmp1$$Register, $tmp2$$Register, false /* is_atomic */);
>> 164:     __ sd_rl($tmp1$$Register, $mem$$Register);
> 
> Same as above. May use `sd_release` or assert `UseZalasr` is always true.

The predicate here already guards this — `needs_releasing_store(n)` returns 
false whenever `!UseZalasr`, so this instruct only matches when `UseZalasr` is 
true. No need for an extra assert.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4027487328
PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4027493269
PR Review Comment: https://git.openjdk.org/jdk/pull/32309#discussion_r4027515716

Reply via email to