On Fri, 30 Jan 2026 07:35:43 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 This is looking good. Just one last comment on the location of `LANE_TYPE_ORDINAL`. 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: // Lane type codes for vector: Suggest you change the comment to indicate the values correspond to `jdk.incubator.vector.LaneType` ordinals e.g., jdk.incubator.vector.LaneType.FLOAT.ordinal() == LT_FLOAT etc. src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractSpecies.java line 152: > 150: int laneTypeOrdinal() { > 151: return laneType.ordinal(); > 152: } Is this needed? Won't all concrete sub types override this? src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java line 60: > 58: > 59: static final int LANE_TYPE_ORDINAL = LT_BYTE; > 60: You can move this up to `ByteVector` and then reuse it to replace `byte.class`, so it is used consistently. src/jdk.incubator.vector/share/classes/jdk/incubator/vector/LaneType.java line 270: > 268: > 269: static { > 270: assert(ofLaneTypeOrdinal(LT_FLOAT) == FLOAT); Very good! src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java line 821: > 819: convert(String name, char kind, Class<E> dom, Class<F> ran, int > opCode, int flags) { > 820: int domran = ((LaneType.of(dom).ordinal() << VO_DOM_SHIFT) + > 821: (LaneType.of(ran).ordinal() << VO_RAN_SHIFT)); As i understand this is still correct because the maximum ordinal value is less than 16 (as was already the case for the basic type). ------------- PR Review: https://git.openjdk.org/jdk/pull/29481#pullrequestreview-3730928806 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2748410039 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2748387527 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2748483577 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2748392412 PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2748427970
