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

[Fei Yang]

On Fri, 22 Aug 2025 12:35:12 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you help to review this patch?
>> 
>> This is a follow-up of https://github.com/openjdk/jdk/pull/26838, fixes the vector version in a similar way.
>> 
>> Thanks!
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   comments & readability

Overall looks fine to me. I have a question about the test change.

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.

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2517:

> 2515:   const int fp16_mantissa_bits = 10;
> 2516: 
> 2517:   // preserve the sign bit and exponent.

Suggestion: `// preserve the sign bit and exponent, clear mantissa.`

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?

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

PR Review: https://git.openjdk.org/jdk/pull/26883#pullrequestreview-3149218656
PR Review Comment: https://git.openjdk.org/jdk/pull/26883#discussion_r2296547748
PR Review Comment: https://git.openjdk.org/jdk/pull/26883#discussion_r2296550895
PR Review Comment: https://git.openjdk.org/jdk/pull/26883#discussion_r2296660079