On Tue, 22 Oct 2024 23:47:33 GMT, Joe Darcy <[email protected]> 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 equals/hashCode implementation; tests to follow.
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line
60:
> 58: * either {@link Float}/{@link Double} or {@link Math}/{@link
> 59: * StrictMath}. Unless otherwise specified, the handling of special
> 60: * floating-point values such as {@linkplain #isNaN(Float16) NaN}
That said, and given Float has "equivalenceRelation" and
"decimalToBinaryConversion" anchors, do we need similar anchors here that
redirects to the Double ones?
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line
90:
> 88: * @param bits a short value.
> 89: */
> 90: private Float16(short bits ) {
Suggestion:
private Float16(short bits) {
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line
405:
> 403: * @return the {@code Float16} value represented by the string
> 404: * argument.
> 405: * @throws NullPointerException if the string is null
Should we move this NPE clause to the class specification, i.e. `Unless
otherwise specified, all methods in this class throw {@code
NullPointerException} when passed a {@code null}`? I think this applies to the
APIs taking `BigDecimal` and static methods taking `Float16`. (Note we need to
explicitly exclude `equals`)
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line
865:
> 863: public static Float16 min(Float16 a, Float16 b) {
> 864: return shortBitsToFloat16(floatToFloat16(Math.min(a.floatValue(),
> 865:
> b.floatValue()) ));
I assume we will optimize these min/max implementations in the future.
Otherwise, the extra space should be removed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1811609773
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1811611334
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1811614031
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1811615209
