On Tue, 8 Apr 2025 10:29:38 GMT, Hamlin Li <[email protected]> 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
