8290485: [vector] REVERSE_BYTES for byte type should not emit any instructions

[XiaohongGong]

The Vector API unary operation "REVERSE_BYTES" should not emit any instructions for byte vectors. The same to the relative masked operation. Currently it emits "mov dst, src" on aarch64 when the "dst" and "src" are not the same register. But for the masked "REVERSE_BYTES", the compiler will always generate a "VectorBlend" which I think is redundant, since the first and second vector input is the same one. Please see the generated codes for the masked "REVERSE_BYTES" for byte type with NEON:

  ldr q16, [x15, #16]            ; load the "src" vector
  mov v17.16b, v16.16b           ; reverse bytes "src"
  ldr q18, [x13, #16]
  neg v18.16b, v18.16b           ; load the vector mask
  bsl v18.16b, v17.16b, v16.16b  ; vector blend

The elements in register "v17" and "v16" are the same to each other, so the elements in result of "bsl" is the same to the original loaded values in "v16", no matter what the values in the vector mask are.

To improve this, we can add the igvn transformations for "ReverseBytesV" and "VectorBlend" in compiler. For "ReverseBytesV", it can return the vector input if the basic element type is T_BYTE. And for "VectorBlend", it can return the first input if the first and the second input are the same one.

Here is the performance data for the jmh benchmark [1] on ARM NEON:

Benchmark                         (size)  Mode  Cnt  Before    After    Units
ByteMaxVector.REVERSE_BYTES        1024  thrpt  15  19457.641 19516.124 ops/ms
ByteMaxVector.REVERSE_BYTESMasked  1024  thrpt  15  12498.416 20528.004 ops/ms

This patch may not have any influence to the non-masked "REVERSE_BYTES" on ARM NEON, because the backend may not emit any instruction for it before.

And here is the performance data on an x86 system:

Benchmark                         (size)  Mode  Cnt  Before    After    Units
ByteMaxVector.REVERSE_BYTES        1024  thrpt  15  19358.941 20012.047 ops/ms
ByteMaxVector.REVERSE_BYTESMasked  1024  thrpt  15  15759.788 20389.996 ops/ms

[1] https://github.com/openjdk/panama-vector/blob/vectorIntrinsics/test/micro/org/openjdk/bench/jdk/incubator/vector/operation/ByteMaxVector.java#L2201

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8290485: [vector] REVERSE_BYTES for byte type should not emit any instructions

Reviewers

• Tobias Hartmann (@TobiHartmann - Reviewer)
• Vladimir Kozlov (@vnkozlov - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9565/head:pull/9565
$ git checkout pull/9565

Update a local copy of the PR:
$ git checkout pull/9565
$ git pull https://git.openjdk.org/jdk pull/9565/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9565

View PR using the GUI difftool:
$ git pr show -t 9565

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9565.diff

[bridgekeeper]

👋 Welcome back xgong! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@XiaohongGong The following label will be automatically applied to this pull request:

• hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (e0bd9dfc)

[XiaohongGong]

Hi, could anyone please help to take a look at this simple patch? Thanks a lot for your time!

[TobiHartmann]

Looks good to me. I'll run some testing and report back once it passed.

[openjdk]

@XiaohongGong This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8290485: [vector] REVERSE_BYTES for byte type should not emit any instructions

Reviewed-by: thartmann, kvn

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 113 new commits pushed to the master branch:

• 0ca5cb1: 8290834: Improve potentially confusing documentation on collection of profiling information
• 7c3cfd1: 8290557: tools/jpackage/share/AddLauncherTest.java#id1 failed with "ERROR: Failed: Check icon file"
• 0ca74f5: 8289659: Refactor I/O stream copying to use InputStream.readAllBytes in X509CertPath
• 1e270ea: 8290780: AArch64: Crash in c2 nmethod running RunThese30M.java
• b17269a: 8290959: Consistently use "grey" instead of "gray" in GC code
• 350808a: 8290459: Remove unused GCCause enums
• d0c365a: 8170794: [macosx] Error when using setDesktopManager on a JDesktopPane on MacOS X with Look and Feel Aqua
• 80dc6ce: 8290730: compiler/vectorization/TestAutoVecIntMinMax.java failed with "IRViolationException: There were one or multiple IR rule failures."
• 228e8e9: 8290700: Optimize AArch64 nmethod entry barriers
• 852e71d: 8290870: NMT: Increase MallocSiteTable size and allocate it only when needed
• ... and 103 more: https://git.openjdk.org/jdk/compare/522b65743ca10fcba0a27d25b8fa11319999e228...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[XiaohongGong]

Thanks for the review and testing @TobiHartmann !

[vnkozlov]

Good.

[XiaohongGong]

Thanks for the review @vnkozlov !

[XiaohongGong]

/integrate

[openjdk]

Going to push as commit a6faf5d.
Since your change was applied there have been 113 commits pushed to the master branch:

• 0ca5cb1: 8290834: Improve potentially confusing documentation on collection of profiling information
• 7c3cfd1: 8290557: tools/jpackage/share/AddLauncherTest.java#id1 failed with "ERROR: Failed: Check icon file"
• 0ca74f5: 8289659: Refactor I/O stream copying to use InputStream.readAllBytes in X509CertPath
• 1e270ea: 8290780: AArch64: Crash in c2 nmethod running RunThese30M.java
• b17269a: 8290959: Consistently use "grey" instead of "gray" in GC code
• 350808a: 8290459: Remove unused GCCause enums
• d0c365a: 8170794: [macosx] Error when using setDesktopManager on a JDesktopPane on MacOS X with Look and Feel Aqua
• 80dc6ce: 8290730: compiler/vectorization/TestAutoVecIntMinMax.java failed with "IRViolationException: There were one or multiple IR rule failures."
• 228e8e9: 8290700: Optimize AArch64 nmethod entry barriers
• 852e71d: 8290870: NMT: Increase MallocSiteTable size and allocate it only when needed
• ... and 103 more: https://git.openjdk.org/jdk/compare/522b65743ca10fcba0a27d25b8fa11319999e228...master

Your commit was automatically rebased without conflicts.

[openjdk]

@XiaohongGong Pushed as commit a6faf5d.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[XiaohongGong]

I‘m sorry that I was in mistake for that the tests had passed, so I integrated this PR. If any tests fail I will revert this PR and fix it. Thanks!

[vnkozlov]

I‘m sorry that I was in mistake for that the tests had passed, so I integrated this PR. If any tests fail I will revert this PR and fix it. Thanks!

Testing passed and results are good. I looked.

[XiaohongGong]

I‘m sorry that I was in mistake for that the tests had passed, so I integrated this PR. If any tests fail I will revert this PR and fix it. Thanks!

Testing passed and results are good. I looked.

That's great! Thanks so much for that!