RFR: 8356159: RISC-V: Add Zabha

Fei Yang fyang at openjdk.org
Mon May 19 10:47:51 UTC 2025


On Mon, 19 May 2025 08:02:00 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

> Yes, Rs2 is at 20->24 : https://riscv-software-src.github.io/riscv-unified-db/manual/html/isa/isa_20240411/insts/sc.w.html We have several instruction where Rs1 and Rs2 have the wrong places.

I think I know what's going on here. 
The riscv spec says: `SC.W conditionally writes a word in rs2 to the address in rs1`.
Even though the encoding of `rs1` and `rs2` doesn't look correct in jdk head, but the callers of `sc_w/d` still work by swapping the two params: address and new value. This means that you also need following addon changes in this PR:

diff --git a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
index 2d3edbb1bee..da47edec785 100644
--- a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
+++ b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
@@ -3823,11 +3823,11 @@ void MacroAssembler::store_conditional(Register dst,
                                        Assembler::Aqrl release) {
   switch (size) {
     case int64:
-      sc_d(dst, new_val, addr, release);
+      sc_d(dst, addr, new_val, release);
       break;
     case int32:
     case uint32:
-      sc_w(dst, new_val, addr, release);
+      sc_w(dst, addr, new_val, release);
       break;
     default:
       ShouldNotReachHere();
@@ -3908,7 +3908,7 @@ void MacroAssembler::cmpxchg_narrow_value(Register addr, Register expected,

     andr(scratch0, result, scratch1); // scratch1 is ~mask
     orr(scratch0, scratch0, new_val);
-    sc_w(scratch0, scratch0, aligned_addr, release);
+    sc_w(scratch0, aligned_addr, scratch0, release);
     bnez(scratch0, retry);
   }

@@ -3980,7 +3980,7 @@ void MacroAssembler::weak_cmpxchg_narrow_value(Register addr, Register expected,

     andr(scratch0, result, scratch1); // scratch1 is ~mask
     orr(scratch0, scratch0, new_val);
-    sc_w(scratch0, scratch0, aligned_addr, release);
+    sc_w(scratch0, aligned_addr, scratch0, release);
     bnez(scratch0, fail);
   }

> I found it!
> 
> ```
> void sc_w(Register Rd, Register Rs2, Register Rs1, Aqrl memory_order = aqrl)
> void NAME(Register Rd, Register Rs1, Register Rs2, Aqrl memory_order = relaxed)
> ```
> 
> lr/sc had relaxed as default memory ordering while other atomics had aqrl, I accidentaly upgraded to aqrl.
> 
> Which explains why it's working, but slower!
> 
> Thanks!
> 
> EDIT:
> 
> amocas have now default aqrl which seems wrong to have different ordering on cas than lr/sc default. But I'll not address this here instead just make sure we have same defaults as today.

Seems you can default to `aqrl` as I didn't see a caller with uses the default value for now.

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

PR Comment: https://git.openjdk.org/jdk/pull/25252#issuecomment-2890538865
PR Comment: https://git.openjdk.org/jdk/pull/25252#issuecomment-2890544460


More information about the hotspot-dev mailing list