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

Fei Yang fyang at openjdk.org
Wed Apr 9 06:57:42 UTC 2025


On Tue, 8 Apr 2025 10:29:38 GMT, Hamlin Li <mli at openjdk.org> wrote:

> > Maybe we should check UseZicond and only enable UseCMoveUnconditionally & ConditionalMoveLimit conditionally?
> 
> Not sure what do you mean here.

Sorry for not being clear enough. I am suggesting this:


  if (UseZicond) {
    FLAG_SET_DEFAULT(ConditionalMoveLimit, 3);
  }


Without `Zicond` extension, conditional moves composed by C2 are simply emulated with regular conditional branches on riscv, which I think is not good in respect of performance.

>> 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
>
> This is to not enable Zicond automatically, but user can still turn it on manually if they want to try or make sure it bring benefit on the specific hardware.
> Currently it's based on bananapi result, so maybe  in the future we should adjust the default value of UseZicond.
> I'm fine with either default value.

I just witnessed a couple of warnings (`UseZicond is turned off automatically. Turn it on with -XX:+UseZicond explicitly.`) when doing a native build on my P550 SBC which is not equipped with `Zicond` extension. I don't think that is expected? And I agree that it might be better to keep this option disabled by default and let users decide whether to enable it based on their cases. But what I see is that `UseZicond` will be auto-enabled through hwprobe [1] on my BPI-F3. So I am suggesting to not to do that in my previous comment. Make sense?

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

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24490#discussion_r2034572731
PR Review Comment: https://git.openjdk.org/jdk/pull/24490#discussion_r2034595744


More information about the core-libs-dev mailing list