RFR: 8356159: RISC-V: Add Zabha [v9]

Fei Yang fyang at openjdk.org
Tue May 27 03:17:05 UTC 2025


On Mon, 26 May 2025 13:39:13 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
>
> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
> 
>  - Reg limits fixed
>  - Merge branch 'master' into 8356159
>  - Fixed reg selection
>  - More indention
>  - Indention
>  - Merge branch 'master' into 8356159
>  - Review comments
>  - Merge branch 'master' into 8356159
>  - Review fix
>  - Bug and review fixes
>  - ... and 5 more: https://git.openjdk.org/jdk/compare/da098e23...27fc528d

A few more minor comments. Looks good otherwise.

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

> 5261:   match(Set res (CompareAndSwapB mem (Binary oldval newval)));
> 5262: 
> 5263:   ins_cost(LOAD_COST + STORE_COST + ALU_COST * 10 + BRANCH_COST * 4);

You might want to align the `ins_cost` with the I & L variants as well. Same for compareAndSwapS.

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

> 5440:   match(Set res (CompareAndSwapB mem (Binary oldval newval)));
> 5441: 
> 5442:   ins_cost(LOAD_COST + STORE_COST + ALU_COST * 10 + BRANCH_COST * 4);

You might want to align the `ins_cost` with the I & L acquire variants as well. Same for compareAndSwapSAcq.

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

> 5444:   format %{
> 5445:     "cmpxchg $mem, $oldval, $newval\t# (byte) if $mem == $oldval then $mem <-- $newval\n\t"
> 5446:     "mv $res, $res == $oldval\t# $res <-- ($res == $oldval ? 1 : 0), #@compareAndSwapB"

s/#@compareAndSwapB/#@compareAndSwapBAcq

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

> 5491:   format %{
> 5492:     "cmpxchg $mem, $oldval, $newval\t# (short) if $mem == $oldval then $mem <-- $newval\n\t"
> 5493:     "mv $res, $res == $oldval\t# $res <-- ($res == $oldval ? 1 : 0), #@compareAndSwapS"

s/#@compareAndSwapS/#@compareAndSwapSAcq

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

> 5627:   match(Set res (CompareAndExchangeB mem (Binary oldval newval)));
> 5628: 
> 5629:   ins_cost(LOAD_COST + STORE_COST + BRANCH_COST * 3 + ALU_COST * 5);

You might want to align the `ins_cost` with the I & L variants as well. Same for compareAndExchangeS.

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

> 5791:   match(Set res (CompareAndExchangeB mem (Binary oldval newval)));
> 5792: 
> 5793:   ins_cost(LOAD_COST + STORE_COST + BRANCH_COST * 3 + ALU_COST * 5);

You might want to align the `ins_cost` with the I & L variants as well. Same for compareAndExchangeSAcq.

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

> 5960:   match(Set res (WeakCompareAndSwapB mem (Binary oldval newval)));
> 5961: 
> 5962:   ins_cost(LOAD_COST + STORE_COST + BRANCH_COST * 2 + ALU_COST * 6);

You might want to align the `ins_cost` with the I & L variants as well. Same for weakCompareAndSwapS.

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

> 6132:   match(Set res (WeakCompareAndSwapB mem (Binary oldval newval)));
> 6133: 
> 6134:   ins_cost(LOAD_COST + STORE_COST + BRANCH_COST * 2 + ALU_COST * 6);

You might want to align the `ins_cost` with the I & L variants as well. Same for weakCompareAndSwapSAcq.

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

PR Review: https://git.openjdk.org/jdk/pull/25252#pullrequestreview-2869424338
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2108061526
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2108062897
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2108055010
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2108055793
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2108066103
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2108067280
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2108068279
PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2108068689


More information about the hotspot-dev mailing list