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