RFR: 8352607: RISC-V: use cmove in min/max when Zicond is supported
Robbin Ehn
rehn at openjdk.org
Mon Mar 24 08:45:11 UTC 2025
On Mon, 24 Mar 2025 04:55:12 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> Hi,
>> Can you help to review this patch?
>> We can let min/max to use cmove if Zicond is supported rather than a branch.
>> At this same time, this patch also simplify the code of min/max.
>>
>> Thanks!
>
> src/hotspot/cpu/riscv/riscv.ad line 9073:
>
>> 9071: ins_encode %{
>> 9072: __ cmov_gt(as_Register($dst$$reg), as_Register($src$$reg),
>> 9073: as_Register($dst$$reg), as_Register($src$$reg));
>
> But the `ins_cost` isn't updated to reflect this change? It is still `BRANCH_COST + ALU_COST` which will only reflect the branch code. Seems better to create seperate match rules for these cmove cases with `UseZicond` as the predicate and proper costs. We already have two sets of match rules for Max and Min. The other one which should be the most efficient is in file `riscv_b.ad` [1]. Do we have a hardware which implements `Zicond` but doesn't have `Zbb`?
>
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/riscv_b.ad#L436
Yes, better to have seperate match rules.
But I also agree that it's a bit messy and unclear how many will actually use that version.
And all of these versions increase test cost.
I think we should aim for RVA23 as the well supported and tested set of extensions.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24153#discussion_r2009716924
More information about the hotspot-compiler-dev
mailing list