RFR: 8356159: RISC-V: Add Zabha

Fei Yang fyang at openjdk.org
Fri May 16 07:23:59 UTC 2025


On Thu, 15 May 2025 14:08:48 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

> Hi, please consider.
> 
> This adds the byte and halfword atomic memory operations (Zabha) - https://github.com/riscv/riscv-zabha.
> All amo-instructions, except load-reserve and store-conditional, can also be performed on natural aligned half-words and bytes. (i.e. the extension do not add lr.h/b or sc.h/b) This includes amocas if zacas extension is present.
> 
> The majority of this patch is to support amocas.h/b. We are now starting to really feel the pain of all these extensions, as CAS:ing 16/8-bits can now be done in three different ways:
> - lr.w/sc.w 'narrow' CAS (no extension)
> - amocas.w 'narrow' CAS (Zacas)
> - amocas.h/b (Zacas + Zabha)
> 
> There is no hwprobe support yet.
> 
> Ran t1-3 with Zacas+Zabha and t1 without Zabha in qemu.
> 
> Thanks, Robbin

Nice cleanup! Some minor comments after a cursory look. Thanks.

src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 3842:

> 3840:                                                  Register shift, Register mask, Register aligned_addr) {
> 3841:   assert(size == int8 || size == int16, "unsupported operand size");
> 3842:   assert(!(UseZacas && UseZabha), "Use amocas");

Seems more obvious to move this two assertions to the entry of its callers?
`MacroAssembler::cmpxchg_narrow_value` and `MacroAssembler::weak_cmpxchg_narrow_value`.

src/hotspot/cpu/riscv/riscv.ad line 5275:

> 5273:     __ cmpxchg(as_Register($mem$$base), $oldval$$Register, $newval$$Register, Assembler::int8,
> 5274:                            Assembler::relaxed /* acquire */, Assembler::rl /* release */, $res$$Register,
> 5275:                            true /* result as bool */);

Indentation.

src/hotspot/cpu/riscv/riscv.ad line 5692:

> 5690:   ins_encode %{
> 5691:     __ cmpxchg(as_Register($mem$$base), $oldval$$Register, $newval$$Register, Assembler::int16,
> 5692:                             /*acquire*/ Assembler::relaxed, /*release*/ Assembler::rl, $res$$Register);

Indentation.

src/hotspot/cpu/riscv/riscv.ad line 5820:

> 5818:   ins_encode %{
> 5819:     __ cmpxchg(as_Register($mem$$base), $oldval$$Register, $newval$$Register, Assembler::int8,
> 5820:               /*acquire*/ Assembler::aq, /*release*/ Assembler::rl, $res$$Register);

Indentation.

src/hotspot/cpu/riscv/riscv.ad line 6050:

> 6048:   ins_encode %{
> 6049:     __ weak_cmpxchg(as_Register($mem$$base), $oldval$$Register, $newval$$Register, Assembler::int16,
> 6050:                                  /*acquire*/ Assembler::relaxed, /*release*/ Assembler::rl, $res$$Register);

Indentation.

src/hotspot/cpu/riscv/riscv.ad line 6176:

> 6174:   ins_encode %{
> 6175:     __ weak_cmpxchg(as_Register($mem$$base), $oldval$$Register, $newval$$Register, Assembler::int8,
> 6176:                                  /*acquire*/ Assembler::aq, /*release*/ Assembler::rl, $res$$Register);

Indentation.

src/hotspot/cpu/riscv/riscv.ad line 6224:

> 6222:   ins_encode %{
> 6223:     __ weak_cmpxchg(as_Register($mem$$base), $oldval$$Register, $newval$$Register, Assembler::int16,
> 6224:                                  /*acquire*/ Assembler::aq, /*release*/ Assembler::rl, $res$$Register);

Indentation.

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

PR Review: https://git.openjdk.org/jdk/pull/25252#pullrequestreview-2845700502
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2092458964
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2092478881
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2092476052
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2092476378
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2092476725
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2092477356
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2092477657


More information about the hotspot-dev mailing list