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