On Tue, 29 Oct 2024 04:30:57 GMT, Joe Darcy <da...@openjdk.org> wrote:
>> Port of Float16 from java.lang in the lworld+fp16 branch to
>> jdk.incubabor.vector.
>
> 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
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);
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line
508:
> 506: } else {
> 507: // All integer digits, no fraction digits
> 508: hexSignificand.append(s.substring(digitStart,
> pIndex));
Suggestion:
hexSignificand.append(s, digitStart, pIndex);
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line
549:
> 547: long maskedValue = dAsLong & mask;
> 548: // Can't have all-zeros or all-ones in low-order bits
> 549: return maskedValue != 0L && maskedValue != mask;
Suggestion:
long mask = 0x03FF_FFFF_FFFFL; // 42 low-order bits
long maskedValue = dAsLong & mask;
// not halfway between two adjacent Float16 values
return maskedValue != 0x0200_0000_0000L;
I think this should be sufficient?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1820870830
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1820870999
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1820871061
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1820871350
