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

[Hamlin Li]

On Wed, 29 Nov 2023 09:30:31 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> 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);
>
> Ah, I haven't checked the slow-path yet. Could it handle negative/positive infinity cases too? If that is true, then we might give it a new name instead of `float16_to_float_nan_path` which might indicates a path only for NAN-handling.

Yes, it can handle inf cases.
Done, also add some comments to state the background and reason.

>> The latest message I got is that it will be pushed into kernel soon, we can wait for it landing in kernel if you'd like to.
>
> Maybe wait a while for it to land?

Sure, let's wait for it.

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

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