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