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