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