On Fri, 21 Nov 2025 03:35:03 GMT, Fei Yang <[email protected]> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   replace assert with log_warning
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1590:
> 
>> 1588:     // jump if cmp1 < cmp2 or either is NaN
>> 1589:     // not jump (i.e. move src to dst) if cmp1 >= cmp2
>> 1590:     float_blt(cmp1, cmp2, no_set);
> 
> I compared this with the existing `MacroAssembler::cmov_cmp_fp_ge` [1] and I 
> witnessed some difference in the case of `NaN` handling. In 
> `MacroAssembler::cmov_cmp_fp_ge`, we set the `is_unordered` param to true 
> when calling `float_blt` or `double_blt`, which is not the case here. I 
> assume we need similar handling here as well, right?
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp#L1338

Make sense, fixed.

> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1636:
> 
>> 1634:     // jump if cmp1 <= cmp2 or either is NaN
>> 1635:     // not jump (i.e. move src to dst) if cmp1 > cmp2
>> 1636:     float_ble(cmp1, cmp2, no_set);
> 
> Same question here.

Make sense, fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28309#discussion_r2556004073
PR Review Comment: https://git.openjdk.org/jdk/pull/28309#discussion_r2556004423

Reply via email to