On Tue, 22 Oct 2024 19:41:57 GMT, Paul Sandoz <psan...@openjdk.org> wrote:

>> Joe Darcy has updated the pull request with a new target base due to a merge 
>> or a rebase. The incremental webrev excludes the unrelated changes brought 
>> in by the merge/rebase. The pull request contains seven additional commits 
>> since the last revision:
>> 
>>  - Add @since tag, respond to review feedback.
>>  - Merge branch 'master' into JDK-8341260
>>  - Remove comments for intrinsics per review feedback.
>>  - Update with changes from lworld+fp16 Float16.
>>  - Merge branch 'master' into JDK-8341260
>>  - Add support for BigDecimal -> Float16 conversion.
>>  - JDK-8341260: Add Float16 to jdk.incubator.vector
>
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line 
> 72:
> 
>> 70: // Enhanced Primitive Boxes described by JEP-402 
>> (https://openjdk.org/jeps/402)
>> 71: // @jdk.internal.MigratedValueClass
>> 72: //@jdk.internal.ValueBased
> 
> We can modify the module info in `java.base` to export `jdk.internal` to 
> `jdk.incubator.vector` then we can annotate `Float16` with `ValueBased`, and 
> then we can get some diagnostic checking e.g., if there is an attempt to 
> synchronize on it.

Will update to refer to JEP 401.

> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line 
> 78:
> 
>> 76:     implements Comparable<Float16> {
>> 77:     private final short value;
>> 78:     private static final long serialVersionUID = 16; // Not needed for a 
>> value class?
> 
> Unsure if this field needed. Perhaps a wider question to be thought and 
> answered about later is whether we should extend from `Number`.

For now, I think the svuid is fine; we can update to whatever the right answer 
will be under value classes at that time. Likewise, leaving the Number 
superclass is fine for now, but may change later.

> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line 
> 1198:
> 
>> 1196:      *
>> 1197:      * @param f16 the value to be negated
>> 1198:      * @jls 15.15.4 Unary Minus Operator {@code -}
> 
> In prior binary operations it is just stated `* @jls 15.4 Floating-point 
> Expressions` can we be consistent?

I'll add more precise tags as well.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1811383428
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1811388882
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1811381891

Reply via email to