RFR: 8352607: RISC-V: use cmove in min/max when Zicond is supported

[Hamlin Li]

On Mon, 24 Mar 2025 08:42:34 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> 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.

For performance consideration please check existing `CMoveI`, which calls `enc_cmove` which calls `cmov_xx`. So if `CMoveI` brings benefit when `UseZicond == true` over `UseZicond != true`, this refactoring should also works expected, as they use the same code.

As for the question, whether a hardware will support `zicond` but not `zbb`, I have no answer.

But, anyway in fact you can just consider this as a code cleanup, in this sense seems it should be good?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24153#discussion_r2009784521