On Thu, 29 Jan 2026 12:58:57 GMT, Jatin Bhateja <[email protected]> wrote:
>> As per [discussions >> ](https://github.com/openjdk/jdk/pull/28002#issuecomment-3789507594) on >> JDK-8370691 pull request, splitting out portion of PR#28002 into a separate >> patch in preparation of Float16 vector API support. >> >> Patch add new lane type constants and pass them to vector intrinsic entry >> points. >> >> All existing Vector API jtreg test are passing with the patch. >> >> Kindly review and share your feedback. >> >> Best Regards, >> Jatin > > Jatin Bhateja has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments resolutions src/hotspot/share/prims/vectorSupport.hpp line 146: > 144: LT_SHORT = 9, > 145: LT_INT = 10, > 146: LT_LONG = 11 Are the values designed to be in sync with the `BasicType` values where the lane type and the basic type are the same? If so we should call this out via explicit assignment. Otherwise, i think we should adjust the values (which may require some adjustment elsewhere e.g., VectorOperators.java). src/java.base/share/classes/jdk/internal/vm/vector/VectorSupport.java line 152: > 150: public static final int MODE_BITS_COERCED_LONG_TO_MASK = 1; > 151: > 152: // BasicType codes, for primitives only: This comment needs to be updated. src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java line 544: > 542: public byte laneHelper(int i) { > 543: return (byte) VectorSupport.extract( > 544: VCLASS, LT_BYTE, VLENGTH, Can we declare a static final field `LANE_TYPE_ORDINAL` (or `LANE_TYPE_ID`, see comment on `LaneType` as the naming is important) and use that consistently like we already do for `ETYPE`? src/jdk.incubator.vector/share/classes/jdk/incubator/vector/LaneType.java line 88: > 86: final String printName; > 87: final char typeChar; // one of "BSILFD" > 88: final int laneType; // lg(size/8) | (kind=='F'?4:kind=='I'?8) We need to change the name of this field to more clearly distinguish between it and the class name. If we can change the values of `LT_*` and align them with the enum ordinal values then we can call it `laneTypeOrdinal` and consistently use that, then we don't likely need the `LT_*` constants. If the values need to align with `BasicType` values then it might be better called `laneTypeIdentifier` or `laneTypeId`. src/jdk.incubator.vector/share/classes/jdk/incubator/vector/LaneType.java line 197: > 195: /*package-private*/ > 196: @ForceInline > 197: static LaneType ofBasicType(int bt) { The method name and argument need updating. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2742857369 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2742850134 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2742840498 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2742894994 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2742873650
