RFR: 8341260: Add Float16 to jdk.incubator.vector [v10]

Joe Darcy darcy at openjdk.org
Wed Oct 30 05:15:08 UTC 2024


On Tue, 29 Oct 2024 14:03:17 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add support for proper String -> Float16 conversion.
>
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line 445:
> 
>> 443: 
>> 444:         if (trialResult == 0.0 // handles signed zeros
>> 445:             || Math.abs(trialResult) > (65504.0 + 32.0) || // Float.MAX_VALUE + ulp(MAX_VALUE)
> 
> Suggestion:
> 
>             || Math.abs(trialResult) > (65504.0 + 16.0) || // Float.MAX_VALUE + ulp(MAX_VALUE) / 2

When writing the code, I didn't think through all the possible boundary conditions near Float16.MAX_VALUE + 0.5*ulp(Float16.MAX_VALUE) so I bumped out the overflow limit.

> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line 504:
> 
>> 502:                     // without the period.
>> 503:                     hexSignificand.append(s.substring(digitStart,      periodIndex));
>> 504:                     hexSignificand.append(s.substring(periodIndex + 1, pIndex));
> 
> Suggestion:
> 
>                     hexSignificand.append(s, digitStart, periodIndex);
>                     hexSignificand.append(s, periodIndex + 1, pIndex);

Good refactoring; thanks for the suggestion.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1821885437
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1821887086


More information about the core-libs-dev mailing list