RFR: 8365772: RISC-V: correctly prereserve NaN payload when converting from float to float16 in vector way [v2]

[Hamlin Li]

On Sun, 24 Aug 2025 07:30:03 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   comments & readability
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2495:
> 
>> 2493:   __ bind(stub.entry());
>> 2494: 
>> 2495:   // mul is already set to mf2 in float_to_float16_v.
> 
> Although not directly related, can you rename `tmp` to `vtmp` and add an assertion about the three vector regiters (just like we do in `C2_MacroAssembler::float_to_float16_v`)? And it would help if we add some extra code comment about `v0` mask register which indicates which elements are NaNs. Or maybe better to pass `v0` as well?
> 
> What I mean is something like: 
> 
> assert_different_registers(dst, src, vtmp);
> 
> // Active elements (NaNs) are marked in v0 mask register
> // and mul is already set to mf2 in float_to_float16_v.

I'll add the comment you suggested.
But Seems passing `v0` is not necessary and weird here, as in the code we don't use `v0` directly.

> test/hotspot/jtreg/compiler/vectorization/TestFloatConversionsVectorNaN.java line 92:
> 
>> 90:         // Setup
>> 91:         for (int i = 0; i < ARRLEN; i++) {
>> 92:             if (i%3 == 0) {
> 
> Question: What is this change for? Do you have more details?

`39` was to make the NaN distributed sparsely in the array.
As we face the NaN calculation issue, so the change is to test NaN more frequently with a random shift.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26883#discussion_r2300455908
PR Review Comment: https://git.openjdk.org/jdk/pull/26883#discussion_r2300456375