Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v4]

2023-12-03 Thread Xiaohong Gong
On Wed, 22 Nov 2023 07:05:21 GMT, Eric Liu  wrote:

>> Vector API defines zero-extend operations [1], which are going to be 
>> intrinsified and generated to `VectorUCastNode` by C2. This patch adds 
>> backend implementation for `VectorUCastNode` on AArch64.
>> 
>> The micro benchmark shows significant performance improvement. In my test 
>> machine (SVE, 256-bit), the result is shown as below:
>> 
>> 
>> 
>>   Benchmark Before After   Units   Gain
>>   VectorZeroExtend.byte2Int 3168.251   243012.399  ops/ms  75.70
>>   VectorZeroExtend.byte2Long3212.201   216291.588  ops/ms  66.33
>>   VectorZeroExtend.byte2Short   3391.968   182655.365  ops/ms  52.85
>>   VectorZeroExtend.int2Long 1012.19780448.553  ops/ms  78.48
>>   VectorZeroExtend.short2Int1812.471   153416.828  ops/ms  83.65
>>   VectorZeroExtend.short2Long   1788.382   129794.814  ops/ms  71.58
>> 
>> 
>> On other Neon systems, we can get similar performance boost as a result of 
>> intrinsification success.
>> 
>> Since `VectorUCastNode` only used in Vector API's zero extension currently, 
>> this patch also adds assertion on nodes' definitions to clarify their usages.
>> 
>> [TEST]
>> compiler/vectorapi and jdk/incubator/vector passed on NEON and SVE machines.
>> 
>> [1] 
>> https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java#L726
>
> Eric Liu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   small fix
>   
>   Change-Id: Icfe9619af1c9e7d5ea8cac457ccebb4eec5c34ad

LGTM!

-

Marked as reviewed by xgong (Committer).

PR Review: https://git.openjdk.org/jdk/pull/16670#pullrequestreview-1761741783


Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v4]

2023-11-30 Thread Andrew Haley
On Wed, 22 Nov 2023 07:05:21 GMT, Eric Liu  wrote:

>> Vector API defines zero-extend operations [1], which are going to be 
>> intrinsified and generated to `VectorUCastNode` by C2. This patch adds 
>> backend implementation for `VectorUCastNode` on AArch64.
>> 
>> The micro benchmark shows significant performance improvement. In my test 
>> machine (SVE, 256-bit), the result is shown as below:
>> 
>> 
>> 
>>   Benchmark Before After   Units   Gain
>>   VectorZeroExtend.byte2Int 3168.251   243012.399  ops/ms  75.70
>>   VectorZeroExtend.byte2Long3212.201   216291.588  ops/ms  66.33
>>   VectorZeroExtend.byte2Short   3391.968   182655.365  ops/ms  52.85
>>   VectorZeroExtend.int2Long 1012.19780448.553  ops/ms  78.48
>>   VectorZeroExtend.short2Int1812.471   153416.828  ops/ms  83.65
>>   VectorZeroExtend.short2Long   1788.382   129794.814  ops/ms  71.58
>> 
>> 
>> On other Neon systems, we can get similar performance boost as a result of 
>> intrinsification success.
>> 
>> Since `VectorUCastNode` only used in Vector API's zero extension currently, 
>> this patch also adds assertion on nodes' definitions to clarify their usages.
>> 
>> [TEST]
>> compiler/vectorapi and jdk/incubator/vector passed on NEON and SVE machines.
>> 
>> [1] 
>> https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java#L726
>
> Eric Liu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   small fix
>   
>   Change-Id: Icfe9619af1c9e7d5ea8cac457ccebb4eec5c34ad

That looks nice. Thanks.

-

Marked as reviewed by aph (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16670#pullrequestreview-1757770099


Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v4]

2023-11-29 Thread Eric Liu
On Wed, 22 Nov 2023 07:05:21 GMT, Eric Liu  wrote:

>> Vector API defines zero-extend operations [1], which are going to be 
>> intrinsified and generated to `VectorUCastNode` by C2. This patch adds 
>> backend implementation for `VectorUCastNode` on AArch64.
>> 
>> The micro benchmark shows significant performance improvement. In my test 
>> machine (SVE, 256-bit), the result is shown as below:
>> 
>> 
>> 
>>   Benchmark Before After   Units   Gain
>>   VectorZeroExtend.byte2Int 3168.251   243012.399  ops/ms  75.70
>>   VectorZeroExtend.byte2Long3212.201   216291.588  ops/ms  66.33
>>   VectorZeroExtend.byte2Short   3391.968   182655.365  ops/ms  52.85
>>   VectorZeroExtend.int2Long 1012.19780448.553  ops/ms  78.48
>>   VectorZeroExtend.short2Int1812.471   153416.828  ops/ms  83.65
>>   VectorZeroExtend.short2Long   1788.382   129794.814  ops/ms  71.58
>> 
>> 
>> On other Neon systems, we can get similar performance boost as a result of 
>> intrinsification success.
>> 
>> Since `VectorUCastNode` only used in Vector API's zero extension currently, 
>> this patch also adds assertion on nodes' definitions to clarify their usages.
>> 
>> [TEST]
>> compiler/vectorapi and jdk/incubator/vector passed on NEON and SVE machines.
>> 
>> [1] 
>> https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java#L726
>
> Eric Liu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   small fix
>   
>   Change-Id: Icfe9619af1c9e7d5ea8cac457ccebb4eec5c34ad

@theRealAph Could you help to take a look? Thanks.

-

PR Comment: https://git.openjdk.org/jdk/pull/16670#issuecomment-1832934884


Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v4]

2023-11-21 Thread Eric Liu
> Vector API defines zero-extend operations [1], which are going to be 
> intrinsified and generated to `VectorUCastNode` by C2. This patch adds 
> backend implementation for `VectorUCastNode` on AArch64.
> 
> The micro benchmark shows significant performance improvement. In my test 
> machine (SVE, 256-bit), the result is shown as below:
> 
> 
> 
>   Benchmark Before After   Units   Gain
>   VectorZeroExtend.byte2Int 3168.251   243012.399  ops/ms  75.70
>   VectorZeroExtend.byte2Long3212.201   216291.588  ops/ms  66.33
>   VectorZeroExtend.byte2Short   3391.968   182655.365  ops/ms  52.85
>   VectorZeroExtend.int2Long 1012.19780448.553  ops/ms  78.48
>   VectorZeroExtend.short2Int1812.471   153416.828  ops/ms  83.65
>   VectorZeroExtend.short2Long   1788.382   129794.814  ops/ms  71.58
> 
> 
> On other Neon systems, we can get similar performance boost as a result of 
> intrinsification success.
> 
> Since `VectorUCastNode` only used in Vector API's zero extension currently, 
> this patch also adds assertion on nodes' definitions to clarify their usages.
> 
> [TEST]
> compiler/vectorapi and jdk/incubator/vector passed on NEON and SVE machines.
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java#L726

Eric Liu has updated the pull request incrementally with one additional commit 
since the last revision:

  small fix
  
  Change-Id: Icfe9619af1c9e7d5ea8cac457ccebb4eec5c34ad

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16670/files
  - new: https://git.openjdk.org/jdk/pull/16670/files/68748e7f..268b71db

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

  Stats: 17 lines in 1 file changed: 0 ins; 9 del; 8 mod
  Patch: https://git.openjdk.org/jdk/pull/16670.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16670/head:pull/16670

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