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

Fei Yang fyang at openjdk.org
Mon May 19 14:46:53 UTC 2025


On Mon, 19 May 2025 09:34:11 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 four additional commits since the last revision:
> 
>  - Revert back to default relaxed
>  - Merge branch 'master' into 8356159
>  - Fixed ws
>  - Initial draft

src/hotspot/cpu/riscv/assembler_riscv.hpp line 1066:

> 1064: 
> 1065:   void lr_w(Register Rd, Register Rs1, Aqrl memory_order = relaxed) {
> 1066:     amo_base<AMO_LR, AMO_WIDTH_WORD>(Rd, Rs1, 0, aqrl);

I see you always use `aqrl` here as the last param, which doesn't seem correct to me. But we should pass `memory_order` instead as that's the param? This seems to explain the bad performance impact.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25252#discussion_r2095878791


More information about the hotspot-dev mailing list