RFR: 8341260: Add Float16 to jdk.incubator.vector [v6]
Chen Liang
liach at openjdk.org
Wed Oct 23 01:12:09 UTC 2024
On Tue, 22 Oct 2024 23:47:33 GMT, Joe Darcy <darcy at 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 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
More information about the core-libs-dev
mailing list