RFR: 8365206: RISC-V: compiler/c2/irTests/TestFloat16ScalarOperations.java is failing on riscv64 [v2]

[Hamlin Li]

On Wed, 20 Aug 2025 03:12:48 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   minors
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2394:
> 
>> 2392:   __ bind(stub.entry());
>> 2393: 
>> 2394:   __ float_to_float16_NaN(src, dst, t0, t1);
> 
> I see `t1` which is the rFlagsReg register for riscv is passed as a temp register. But this side effect (clobbering of `t1`) doesn't reflect on the callsite `float_to_float16` in `instruct convF2HF_reg_reg`. So I guess you might want to pass `tmp` instead of `t1` here. Like: `__ float_to_float16_NaN(src, dst, t0, tmp);`

Nice catch! it's a typo, fixed.

> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 5959:
> 
>> 5957: // this is a utility method to process the slow path of NaN when converting float to float16
>> 5958: // check j.l.Float.floatToFloat16
>> 5959: void MacroAssembler::float_to_float16_NaN(FloatRegister src, Register dst,
> 
> Can you swap the first two params? I perfer to have `dst` which holds the result as the first one. I think that's what we always do for other assembler routines as well.

sure

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26838#discussion_r2287549482
PR Review Comment: https://git.openjdk.org/jdk/pull/26838#discussion_r2287549915