RFR: 8318227: RISC-V: C2 ConvHF2F [v2]

[Hamlin Li]

On Tue, 28 Nov 2023 13:23:43 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   optimize perf with stub out-of-line
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1704:
> 
>> 1702:   // check whether it's a NaN.
>> 1703:   mv(t0, 0x7c00);
>> 1704:   andr(tmp, src, t0);
> 
> I see from the exponent encoding of float16 on [1], it could be a negative/positive infinity as well when exponent is 0b11111. It depends on whether the significand is zero or not. So it this checking for NAN sufficient?
> 
> [1] https://en.wikipedia.org/wiki/Half-precision_floating-point_format

Goot catch!

Your observation is right and wrong. :)
We could have a patch like below, but it will scrafise the performance of the normal case(non-NaN, non-Inf), as it adds one extra instructions at the critical path.
Maybe a solution is to add some comments here, to state that NaN and Inf are processed in slow path, and slow path is necessary to NaN, but not necessary to Inf.
How do you think about it?


$ git diff src/
diff --git a/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp b/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp
index 1b6140242b8..7413767395f 100644
--- a/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp
+++ b/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp
@@ -1700,10 +1700,11 @@ void C2_MacroAssembler::float16_to_float(FloatRegister dst, Register src, Regist
   auto stub = C2CodeStub::make<FloatRegister, Register, Register>(dst, src, tmp, 20, float16_to_float_nan_path);
 
   // check whether it's a NaN.
-  mv(t0, 0x7c00);
+  mv(t0, 0x7fff);
   andr(tmp, src, t0);
+  mv(t0, 0x7c00);
   // jump to stub processing NaN case
-  beq(t0, tmp, stub->entry());
+  bgt(tmp, t0, stub->entry());
 
   // non-NaN cases, just use built-in instructions.
   fmv_h_x(dst, src);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16802#discussion_r1407928062