On Tue, 23 Jun 2026 06:01:28 GMT, Jatin Bhateja <[email protected]> wrote:

>> As per the discussion on JDK-8370691-Float16Vector Support pull request 
>> https://github.com/openjdk/jdk/pull/28002#issuecomment-4652163477
>> adding the handling to Canonicalize NaN lane encodings held in Float16Vector 
>> lanes so that all NaN representations,
>> including signaling ones, computes the same hash code.
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Jatin Bhateja has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Review comments resolution

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/X-Vector.java.template
 line 5739:

> 5737:     String toString() {
> 5738:         // now that toArray is strongly typed, we can define this
> 5739:         return Arrays.toString(toArray());

`toFloat16Array()` can also be used in `toString()`:
Suggestion:

#if[FP16]
        // use Float16.toString instead of the raw short values
        return Arrays.toString(toFloat16Array());
#else[FP16]
        // now that toArray is strongly typed, we can define this
        return Arrays.toString(toArray());
#end[FP16]

(might need a different issue, or increase the scope of this one)

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/X-Vector.java.template
 line 5770:

> 5768:         // so that all NaN representations contribute the same hash 
> code.
> 5769:         return Objects.hash(species(), 
> Arrays.hashCode(toFloat16Array()));
> 5770: #else[FP16]

This comment should probably be moved into the `#else[FP16]` branch:
Suggestion:

#if[FP16]
        // Hash the lanes as Float16 values; Float16.hashCode canonicalizes NaN
        // so that all NaN representations contribute the same hash code.
        return Objects.hash(species(), Arrays.hashCode(toFloat16Array()));
#else[FP16]
        // now that toArray is strongly typed, we can define this

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31587#discussion_r3462027787
PR Review Comment: https://git.openjdk.org/jdk/pull/31587#discussion_r3462035971

Reply via email to