Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v8]

2026-02-05 Thread Paul Sandoz
On Thu, 5 Feb 2026 04:33:29 GMT, Jatin Bhateja  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 comment resolution

Marked as reviewed by psandoz (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/29481#pullrequestreview-3758240891


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v8]

2026-02-04 Thread Jatin Bhateja
On Thu, 5 Feb 2026 05:06:19 GMT, Quan Anh Mai  wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comment resolution
>
> Thanks, hotspot changes look good to me

Thanks @merykitty , Hi @PaulSandoz , please re-approve.

-

PR Comment: https://git.openjdk.org/jdk/pull/29481#issuecomment-3851499002


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v8]

2026-02-04 Thread Quan Anh Mai
On Thu, 5 Feb 2026 04:33:29 GMT, Jatin Bhateja  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 comment resolution

Thanks, hotspot changes look good to me

-

Marked as reviewed by qamai (Committer).

PR Review: https://git.openjdk.org/jdk/pull/29481#pullrequestreview-3754621036


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v7]

2026-02-04 Thread Jatin Bhateja
On Wed, 4 Feb 2026 18:13:08 GMT, Quan Anh Mai  wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comments resolution
>
> src/hotspot/share/prims/vectorSupport.hpp line 141:
> 
>> 139: 
>> 140:   // Values in this enum correspond to the 
>> jdk.incubator.vector.LaneType ordinal values.
>> 141:   enum LaneType {
> 
> Please specify the underlying type of this enum: `enum LaneType : int`. 
> Otherwise, this will not work:
> 
> VectorSupport::LaneType vltype = 
> static_cast(laneType->get_con());
> 
> It is because casting an integer to an unscoped enum is UB if 
> `laneType->get_con()` does not fit into the range of the enum values.

Thanks @merykitty , addressed.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2767032912


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v8]

2026-02-04 Thread Jatin Bhateja
> 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 comment resolution

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/29481/files
  - new: https://git.openjdk.org/jdk/pull/29481/files/c1935efc..4f7d671e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=07
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=06-07

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/29481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29481/head:pull/29481

PR: https://git.openjdk.org/jdk/pull/29481


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v7]

2026-02-04 Thread Quan Anh Mai
On Tue, 3 Feb 2026 03:31:52 GMT, Jatin Bhateja  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 resolution

src/hotspot/share/prims/vectorSupport.hpp line 141:

> 139: 
> 140:   // Values in this enum correspond to the jdk.incubator.vector.LaneType 
> ordinal values.
> 141:   enum LaneType {

Please specify the underlying type of this enum: `enum LaneType : int`. 
Otherwise, this will not work:

VectorSupport::LaneType vltype = 
static_cast(laneType->get_con());

It is because casting an integer to an unscoped enum is UB if 
`laneType->get_con()` does not fit into the range of the enum values.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2765349355


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v7]

2026-02-04 Thread Xueming Shen
On Tue, 3 Feb 2026 03:31:52 GMT, Jatin Bhateja  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 resolution

Testing tier1,tier2 & tier3: looks good.
bash jib.sh mach5 -- remote-build-and-test --job tier1,tier2,tier3 --id-tag 
pr-29481-with-hotspot
https://mach5.us.oracle.com/mdash/jobs/xuemingshen-pr-29481-with-hotspot-20260203-1712-40954590

-

PR Comment: https://git.openjdk.org/jdk/pull/29481#issuecomment-3848896208


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v7]

2026-02-03 Thread Jatin Bhateja
On Tue, 3 Feb 2026 03:31:52 GMT, Jatin Bhateja  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 resolution

Hi @merykitty , looking for your review approval to check-in this

-

PR Comment: https://git.openjdk.org/jdk/pull/29481#issuecomment-3845705837


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v6]

2026-02-02 Thread Jatin Bhateja
On Mon, 2 Feb 2026 20:22:46 GMT, Paul Sandoz  wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comment resolution
>
> Very good. Approved, there is just one comment related to adding a comment 
> for the LT_* values. Thank you for separating this out from the float16 PR. 
> Needs a HotSpot reviewer too. We will run it through tier 1 to 3 testing.

Thanks @PaulSandoz , @merykitty please let me know if this is good to land.

-

PR Comment: https://git.openjdk.org/jdk/pull/29481#issuecomment-3838835040


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v7]

2026-02-02 Thread Jatin Bhateja
> 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 resolution

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/29481/files
  - new: https://git.openjdk.org/jdk/pull/29481/files/23022d42..c1935efc

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=06
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=05-06

  Stats: 3 lines in 2 files changed: 1 ins; 2 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/29481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29481/head:pull/29481

PR: https://git.openjdk.org/jdk/pull/29481


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v6]

2026-02-02 Thread Paul Sandoz
On Mon, 2 Feb 2026 09:07:21 GMT, Jatin Bhateja  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 comment resolution

Very good. Approved, there is just one comment related to adding a comment for 
the LT_* values. Thank you for separating this out from the float16 PR. Needs a 
HotSpot reviewer too. We will run it through tier 1 to 3 testing.

src/hotspot/share/prims/vectorSupport.hpp line 140:

> 138:   };
> 139: 
> 140:   enum LaneType {

Please add a comment referencing `LaneType` and that the values in this enum 
correspond to the LaneType ordinal values.

-

Marked as reviewed by psandoz (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/29481#pullrequestreview-3741431390
PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2755893774


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v5]

2026-02-02 Thread Jatin Bhateja
On Sun, 1 Feb 2026 17:12:49 GMT, Paul Sandoz  wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comments resolution
>
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java 
> line 580:
> 
>> 578: public static ByteVector zero(VectorSpecies species) {
>> 579: ByteSpecies vsp = (ByteSpecies) species;
>> 580: return VectorSupport.fromBitsCoerced(vsp.vectorType(), 
>> vsp.laneTypeOrdinal(), species.length(),
> 
> You can now use `LANE_TYPE_ORDINAL` rather than `vsp.laneTypeOrdinal()`, 
> which better fits the prior pattern.

Done

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2753281411


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v6]

2026-02-02 Thread Jatin Bhateja
> 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 comment resolution

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/29481/files
  - new: https://git.openjdk.org/jdk/pull/29481/files/0c60016b..23022d42

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=04-05

  Stats: 115 lines in 7 files changed: 0 ins; 0 del; 115 mod
  Patch: https://git.openjdk.org/jdk/pull/29481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29481/head:pull/29481

PR: https://git.openjdk.org/jdk/pull/29481


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v5]

2026-02-01 Thread Paul Sandoz
On Sun, 1 Feb 2026 07:41:59 GMT, Jatin Bhateja  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 resolution

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java 
line 580:

> 578: public static ByteVector zero(VectorSpecies species) {
> 579: ByteSpecies vsp = (ByteSpecies) species;
> 580: return VectorSupport.fromBitsCoerced(vsp.vectorType(), 
> vsp.laneTypeOrdinal(), species.length(),

You can now use `LANE_TYPE_ORDINAL` rather than `vsp.laneTypeOrdinal()`, which 
better fits the prior pattern.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2751629721


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v4]

2026-02-01 Thread Paul Sandoz
On Sun, 1 Feb 2026 07:36:35 GMT, Jatin Bhateja  wrote:

>> 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?
>
> This interface provides access to lane type constant though species, its used 
> for consistency, please have a look at following line and other places around 
> it.
> https://github.com/jatin-bhateja/jdk/blob/ff73dc3d48a9435c4395556c8325fbce7610cba9/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/DoubleVector.java#L3374

Agreed that this method is required, but i was wondering why `AbstractSpecies` 
need to implement it. Ok, i see now you are copying the same pattern as some 
other methods such as `elementType`, so this is a more general issue we should 
not resolve in this PR.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2751614740


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v4]

2026-01-31 Thread Jatin Bhateja
On Fri, 30 Jan 2026 23:31:29 GMT, Paul Sandoz  wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comments resolutions
>
> 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?

This interface provides access to lane type constant though species, its used 
for consistency, please have a look at following line and other places around 
it.
https://github.com/jatin-bhateja/jdk/blob/ff73dc3d48a9435c4395556c8325fbce7610cba9/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/DoubleVector.java#L3374

> 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.

Done

> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java
>  line 821:
> 
>> 819: convert(String name, char kind, Class dom, Class 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).

Correct.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2750675259
PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2750675162
PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2750675209


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v5]

2026-01-31 Thread Jatin Bhateja
> 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 resolution

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/29481/files
  - new: https://git.openjdk.org/jdk/pull/29481/files/ff73dc3d..0c60016b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=03-04

  Stats: 401 lines in 39 files changed: 28 ins; 62 del; 311 mod
  Patch: https://git.openjdk.org/jdk/pull/29481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29481/head:pull/29481

PR: https://git.openjdk.org/jdk/pull/29481


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v4]

2026-01-30 Thread Paul Sandoz
On Fri, 30 Jan 2026 07:35:43 GMT, Jatin Bhateja  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 dom, Class 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


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v3]

2026-01-29 Thread Jatin Bhateja
On Thu, 29 Jan 2026 18:08:03 GMT, Paul Sandoz  wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comments resolutions
>
> 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`.

Thanks @PaulSandoz , I have incorporated your comments, it will still be useful 
to keep new LT_* constants as its better to pass named constants to intrinsic 
entries rather than numeric values.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2745013458


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v4]

2026-01-29 Thread Jatin Bhateja
> 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

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/29481/files
  - new: https://git.openjdk.org/jdk/pull/29481/files/d81035fd..ff73dc3d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=02-03

  Stats: 858 lines in 48 files changed: 290 ins; 13 del; 555 mod
  Patch: https://git.openjdk.org/jdk/pull/29481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29481/head:pull/29481

PR: https://git.openjdk.org/jdk/pull/29481


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v3]

2026-01-29 Thread Paul Sandoz
On Thu, 29 Jan 2026 12:58:57 GMT, Jatin Bhateja  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


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v3]

2026-01-29 Thread Jatin Bhateja
> 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

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/29481/files
  - new: https://git.openjdk.org/jdk/pull/29481/files/11021544..d81035fd

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=01-02

  Stats: 103 lines in 3 files changed: 23 ins; 0 del; 80 mod
  Patch: https://git.openjdk.org/jdk/pull/29481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29481/head:pull/29481

PR: https://git.openjdk.org/jdk/pull/29481


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v2]

2026-01-29 Thread Quan Anh Mai
On Thu, 29 Jan 2026 11:47:33 GMT, Jatin Bhateja  wrote:

>> Please use a separate named type instead of `int`.
>
> It is indeed an integral value which is passed from Java side which is casted 
> to BasicType.

So please cast it in the intrinsics functions in `vectorIntrinsics.cpp` and 
pass the `LaneType` into this function.

static bool is_primitive_lane_type(int laneType) {
  return laneType >= VectorSupport::LT_FLOAT && laneType <= 
VectorSupport::LT_LONG;
}

This function could return a `LaneType` for you. Also, when do we pass in 
something that is not a valid value? Should this be an `assert`?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2741273046


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v2]

2026-01-29 Thread Jatin Bhateja
On Thu, 29 Jan 2026 11:43:06 GMT, Quan Anh Mai  wrote:

>> Correct, we are passing an integer laneType from java side to intrinsic 
>> entry points.
>
> Please use a separate named type instead of `int`.

It is indeed an integral value which is passed from Java side which is casted 
to BasicType.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2741250148


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v2]

2026-01-29 Thread Quan Anh Mai
On Thu, 29 Jan 2026 11:34:51 GMT, Jatin Bhateja  wrote:

>> src/hotspot/share/prims/vectorSupport.cpp line 202:
>> 
>>> 200: }
>>> 201: 
>>> 202: int VectorSupport::vop2ideal(jint id, BasicType bt) {
>> 
>> Previously, this method accepts a `BasicType`, now it accepts an untyped 
>> `int`.
>
> Correct, we are passing an integer laneType from java side to intrinsic entry 
> points.

Please use a separate named type instead of `int`.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2741235103


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v2]

2026-01-29 Thread Jatin Bhateja
On Thu, 29 Jan 2026 11:19:18 GMT, Quan Anh Mai  wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comments resolution
>
> src/hotspot/share/prims/vectorSupport.cpp line 202:
> 
>> 200: }
>> 201: 
>> 202: int VectorSupport::vop2ideal(jint id, BasicType bt) {
> 
> Previously, this method accepts a `BasicType`, now it accepts an untyped 
> `int`.

Correct, we are passing an integer laneType from java side to intrinsic entry 
points.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2741207425


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v2]

2026-01-29 Thread Quan Anh Mai
On Thu, 29 Jan 2026 09:07:54 GMT, Quan Anh Mai  wrote:

>> Its contained in VectorSupport class which makes it implicitly scoped for 
>> external uses without being a named (scoped) enum
>
> I mean an `enum class`. With this we just pass `int` around which is not 
> recommended.

I don't see this gets resolved. My suggestion is to use a scoped enum so we 
have a strongly typed value instead of using an unscoped enum and passing `int` 
all over the places.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2741147870


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v2]

2026-01-29 Thread Quan Anh Mai
On Thu, 29 Jan 2026 09:43:30 GMT, Jatin Bhateja  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 resolution

src/hotspot/share/prims/vectorSupport.cpp line 202:

> 200: }
> 201: 
> 202: int VectorSupport::vop2ideal(jint id, BasicType bt) {

Previously, this method accepts a `BasicType`, now it accepts an untyped `int`.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2741153639


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries [v2]

2026-01-29 Thread Jatin Bhateja
> 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 resolution

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/29481/files
  - new: https://git.openjdk.org/jdk/pull/29481/files/ef96506d..11021544

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/29481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29481/head:pull/29481

PR: https://git.openjdk.org/jdk/pull/29481


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries

2026-01-29 Thread Quan Anh Mai
On Thu, 29 Jan 2026 09:02:29 GMT, Jatin Bhateja  wrote:

>> src/hotspot/share/prims/vectorSupport.hpp line 140:
>> 
>>> 138:   };
>>> 139: 
>>> 140:   enum {
>> 
>> Please use a scoped enum instead.
>
> Its contained in VectorSupport class which makes it implicitly scoped for 
> external uses without being a named (scoped) enum

I mean an `enum class`. With this we just pass `int` around which is not 
recommended.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2740638154


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries

2026-01-29 Thread Jatin Bhateja
On Thu, 29 Jan 2026 08:09:36 GMT, Quan Anh Mai  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
>
> src/hotspot/share/prims/vectorSupport.hpp line 140:
> 
>> 138:   };
>> 139: 
>> 140:   enum {
> 
> Please use a scoped enum instead.

Its contained in VectorSupport class which makes it implicitly scoped.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2740618259


Re: RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries

2026-01-29 Thread Quan Anh Mai
On Thu, 29 Jan 2026 07:16:35 GMT, Jatin Bhateja  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

src/hotspot/share/prims/vectorSupport.hpp line 140:

> 138:   };
> 139: 
> 140:   enum {

Please use a scoped enum instead.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29481#discussion_r2740433980


RFR: 8376187: [VectorAPI] Define new lane type constants and pass them to intrinsic entries

2026-01-28 Thread Jatin Bhateja
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

-

Commit messages:
 - [VectorAPI] Define new lane type constants and pass them to intrinsic entries

Changes: https://git.openjdk.org/jdk/pull/29481/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=29481&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8376187
  Stats: 1744 lines in 52 files changed: 192 ins; 79 del; 1473 mod
  Patch: https://git.openjdk.org/jdk/pull/29481.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29481/head:pull/29481

PR: https://git.openjdk.org/jdk/pull/29481