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