RFR: 8352504: RISC-V: implement and enable CMoveI/L

[Fei Yang]

On Mon, 7 Apr 2025 14:23:52 GMT, Hamlin Li <mli at openjdk.org> wrote:

> Hi,
> Can you help to review this patch?
> On riscv, CMoveI/L already were implemented, but there are some gap:
> 1. CMoveI/L does not support comparison with float/double, corresponding tests are not turn on either.
> 2. Some optimization of C2 is not turned on, e.g. `Phi -> CMove -> min_max`.
> 3. lack of some corresponding performance tests.
> 
> Also there are some issue with current Zicond:
> 1. UseZicond is turned on automatically by hwprobe, but jmh tests show that it's not always bring benefit, in some situation it even brings regression, the reason is the generated code by Zicond is much larger than branch version, in particular when it's in a loop and unrolled.
> 
> This patch on riscv is to:
> 1. add CMoveI/L comparing float/double, and corresponding tests,
> 2. enable more C2 optimization,
> 3. add more benchmark tests,
> 4. turn off UseZicond by default.
> 
> Thanks!

Hi, @Hamlin-Li , Thanks for looking at this part. I once created JBS https://bugs.openjdk.org/browse/JDK-8346786 about `ConditionalMoveLimit` on RISC-V. I have two questions after a cursory look.

src/hotspot/cpu/riscv/vm_version_riscv.cpp line 257:

> 255:   if (ConditionalMoveLimit > 0) {
> 256:     FLAG_SET_DEFAULT(ConditionalMoveLimit, 0);
> 257:   }

Maybe we should check `UseZicond` and only enable `UseCMoveUnconditionally` & `ConditionalMoveLimit` conditionally? I don't see how enabling CMove will bring us performance benefit without `Zicond`. It's done with conditional branches in CPU backend.

src/hotspot/cpu/riscv/vm_version_riscv.cpp line 461:

> 459:     FLAG_SET_DEFAULT(UseZicond, false);
> 460:     warning("UseZicond is turned off automatically. Turn it on with -XX:+UseZicond explicitly.");
> 461:   }

Does this mean `UseZicond` could not be enabled on the command line? And I witnessed quite some warning when doing a native build. If `UseZicond` causes regression for some cases, is it more reasonable to not auto-enable it through hwprobe [1]? Or only enable it for debug builds like https://github.com/openjdk/jdk/pull/24478 does?

[1] https://github.com/openjdk/jdk/blob/master/src/hotspot/os_cpu/linux_riscv/riscv_hwprobe.cpp#L228

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

PR Review: https://git.openjdk.org/jdk/pull/24490#pullrequestreview-2748525865
PR Review Comment: https://git.openjdk.org/jdk/pull/24490#discussion_r2032530242
PR Review Comment: https://git.openjdk.org/jdk/pull/24490#discussion_r2032292830