8284960: Integration of JEP 426: Vector API (Fourth Incubator) #8425

jatin-bhateja · 2022-04-27T11:03:48Z

Hi All,

Patch adds the planned support for new vector operations and APIs targeted for JEP 426: Vector API (Fourth Incubator).

Following is the brief summary of changes:-

Extends the scope of existing lanewise API for following new vector operations.

VectorOperations.BIT_COUNT: counts the number of one-bits
VectorOperations.LEADING_ZEROS_COUNT: counts the number of leading zero bits
VectorOperations.TRAILING_ZEROS_COUNT: counts the number of trailing zero bits
VectorOperations.REVERSE: reversing the order of bits
VectorOperations.REVERSE_BYTES: reversing the order of bytes
compress and expand bits: Semantics are based on Hacker's Delight section 7-4 Compress, or Generalized Extract.

Adds following new APIs to perform cross lane vector compress and expansion operations under the influence of a mask.

Vector.compress
Vector.expand
VectorMask.compress

Adds predicated and non-predicated versions of following new APIs to load and store the contents of vector from foreign MemorySegments.

Vector.fromMemorySegment
Vector.intoMemorySegment

C2 Compiler IR enhancements and optimized X86 and AARCH64 backend support for each newly added operation.

Patch has been regressed over AARCH64 and X86 targets different AVX levels.

Kindly review and share your feedback.

Best Regards,
Jatin

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
Change requires a JEP request to be targeted
Change requires a CSR request to be approved

Issues

JDK-8284960: Integration of JEP 426: Vector API (Fourth Incubator)
JDK-8284972: Integration of JEP 426: Vector API (Fourth Incubator) (CSR)
JDK-8280173: JEP 426: Vector API (Fourth Incubator) (JEP)

Reviewers

Nick Gasson (@nick-arm - Reviewer) ⚠️ Review applies to 731787ef
Vladimir Ivanov (@iwanowww - Reviewer) ⚠️ Review applies to 17a0e38c
Maurizio Cimadamore (@mcimadamore - Reviewer) ⚠️ Review applies to 17a0e38c
Jan Lahoda (@lahodaj - Reviewer) ⚠️ Review applies to 17a0e38c
Vladimir Kozlov (@vnkozlov - Reviewer)

Contributors

Jatin Bhateja <jbhateja@openjdk.org>
Paul Sandoz <psandoz@openjdk.org>
Sandhya Viswanathan <sviswanathan@openjdk.org>
Smita Kamath <svkamath@openjdk.org>
Joshua Zhu <jzhu@openjdk.org>
Xiaohong Gong <xgong@openjdk.org>
John R Rose <jrose@openjdk.org>
Eric Liu <eliu@openjdk.org>
Ningsheng Jian <njian@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8425/head:pull/8425
$ git checkout pull/8425

Update a local copy of the PR:
$ git checkout pull/8425
$ git pull https://git.openjdk.java.net/jdk pull/8425/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8425

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8425.diff

bridgekeeper · 2022-04-27T11:05:45Z

👋 Welcome back jbhateja! 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 · 2022-04-27T11:07:28Z

@jatin-bhateja The following labels will be automatically applied to this pull request:

core-libs
hotspot

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

jatin-bhateja · 2022-04-27T16:34:52Z

/contributor add @jatin-bhateja

jatin-bhateja · 2022-04-27T16:36:04Z

/contributor add @PaulSandoz

openjdk · 2022-04-27T16:36:06Z

@jatin-bhateja
Contributor Jatin Bhateja <jbhateja@openjdk.org> successfully added.

openjdk · 2022-04-27T16:37:19Z

@jatin-bhateja
Contributor Paul Sandoz <psandoz@openjdk.org> successfully added.

jatin-bhateja · 2022-04-27T16:39:19Z

/contributor add @sviswa7

openjdk · 2022-04-27T16:39:59Z

@jatin-bhateja
Contributor Sandhya Viswanathan <sviswanathan@openjdk.org> successfully added.

jatin-bhateja · 2022-04-27T16:42:07Z

/contributor add @smita-kamath

openjdk · 2022-04-27T16:42:55Z

@jatin-bhateja
Contributor Smita Kamath <svkamath@openjdk.org> successfully added.

jatin-bhateja · 2022-04-27T16:44:24Z

/contributor add @JoshuaZhuwj

openjdk · 2022-04-27T16:44:29Z

@jatin-bhateja
Contributor Joshua Zhu <jzhu@openjdk.org> successfully added.

openjdk · 2022-04-27T16:45:52Z

@jatin-bhateja Could not parse @swati-sha as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

openjdk · 2022-04-27T16:56:34Z

@jatin-bhateja Unknown command contributors - for a list of valid commands use /help.

jatin-bhateja · 2022-04-27T16:58:41Z

/contributor add @XiaohongGong

openjdk · 2022-04-27T16:59:24Z

@jatin-bhateja
Contributor Xiaohong Gong <xgong@openjdk.org> successfully added.

jatin-bhateja · 2022-04-27T17:41:09Z

/contributor add @rose00

openjdk · 2022-04-27T17:41:55Z

@jatin-bhateja
Contributor John R Rose <jrose@openjdk.org> successfully added.

jatin-bhateja · 2022-04-28T13:09:30Z

/contributor add @theRealELiu

jatin-bhateja · 2022-04-28T13:10:36Z

/contributor add @nsjian

openjdk · 2022-04-28T13:11:07Z

@jatin-bhateja
Contributor Eric Liu <eliu@openjdk.org> successfully added.

…iew participant. Code re-organization related to Reverse/ReverseByte IR transforms.

PaulSandoz · 2022-05-19T21:19:49Z

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java

 import java.util.Map;
 import java.util.Set;

+import static com.sun.tools.javac.code.Flags.PREVIEW_API;


Suggested change

import static com.sun.tools.javac.code.Flags.PREVIEW_API;

Redundant import (sorry i should have checked before i sent you updates to this area)

PaulSandoz · 2022-05-19T23:48:55Z

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java

+    /**
+     * Returns true if {@code s} is participating in the preview of {@code previewSymbol}, and
+     * therefore no warnings or errors will be produced.
+     *
+     * @param s the symbol depending on the preview symbol
+     * @param previewSymbol the preview symbol marked with @Preview
+     * @return true if {@code s} is participating in the preview of {@code previewSymbol}
+     */
+    public boolean isPreviewParticipating(Symbol s, Symbol previewSymbol) {


Some feedback from a colleague:

Suggested change

/**

* Returns true if {@code s} is participating in the preview of {@code previewSymbol}, and

* therefore no warnings or errors will be produced.

*

* @param s the symbol depending on the preview symbol

* @param previewSymbol the preview symbol marked with @Preview

* @return true if {@code s} is participating in the preview of {@code previewSymbol}

*/

public boolean isPreviewParticipating(Symbol s, Symbol previewSymbol) {

/**

* Returns true if {@code s} is deemed to participate in the preview of {@code previewSymbol}, and

* therefore no warnings or errors will be produced.

*

* @param s the symbol depending on the preview symbol

* @param previewSymbol the preview symbol marked with @Preview

* @return true if {@code s} is participating in the preview of {@code previewSymbol}

*/

public boolean participatesInPreview(Symbol s, Symbol previewSymbol) {

mcimadamore

Javac changes look good

lahodaj

The javac changes look OK to me.

jatin-bhateja · 2022-05-23T16:26:06Z

Hi @iwanowww , your comments have been addressed. kindly let me know if you have other comments on x86 side changes.

iwanowww

Looks good!

vnkozlov · 2022-05-23T22:12:00Z

src/hotspot/cpu/x86/assembler_x86.cpp

 }

+void Assembler::evplzcntd(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len) {
+  assert(VM_Version::supports_avx512cd() && (vector_len == AVX_512bit || VM_Version::supports_avx512vl()), "");


Please, split assert as in other instructions - it will help to understand failure better.

vnkozlov · 2022-05-23T22:12:18Z

src/hotspot/cpu/x86/assembler_x86.cpp

+}
+
+void Assembler::evplzcntq(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len) {
+  assert(VM_Version::supports_avx512cd() && (vector_len == AVX_512bit || VM_Version::supports_avx512vl()), "");


Split assert.

vnkozlov · 2022-05-23T22:14:01Z

src/hotspot/cpu/x86/assembler_x86.cpp

 }
+
+void Assembler::evpcompressb(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len) {
+  assert(VM_Version::supports_avx512_vbmi2() && (vector_len == AVX_512bit || VM_Version::supports_avx512vl()), "");


Split assert in this and following new instructions.

vnkozlov · 2022-05-23T22:17:40Z

src/hotspot/cpu/x86/assembler_x86.cpp

-  assert(VM_Version::supports_avx2(), "");
+  assert(VM_Version::supports_evex(), "");


Hmm, did we never trigger this wrong assert because the use was guarded by correct check?

@jatin-bhateja something wrong with merge. vpadd() is removed. It was added by #8778 and still is used in x86.ad.

Hi @vnkozlov , after integration of PR 8778 there were there were two copies of vpadd with same signature, so removed one of them.

Okay. Got it.

vnkozlov · 2022-05-23T22:23:41Z

src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp

+      evcompresspd(dst, mask, src, merge, vec_enc);
+      break;
+    default:
+      fatal("Unsupported type");


Print wrong type: fatal("Unsupported type : %s", type2name(type));
Below too.

vnkozlov · 2022-05-23T22:45:04Z

src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp

+      vpaddq(dst, src1, src2, vec_enc);
+      break;
+    default:
+      ShouldNotReachHere();


Use fatal and print type.

vnkozlov · 2022-05-23T22:49:30Z

src/hotspot/cpu/x86/x86.ad

+    // should be succeeded by its corresponding vector IR and following
+    // special handling should be removed.
+    if (bt == T_LONG && rbt == T_INT) {
+      if (VM_Version::supports_avx512vl()) {


Predicate say !VM_Version::supports_avx512vl()

vnkozlov · 2022-05-23T22:57:59Z

src/hotspot/share/opto/node.hpp

+  // The node is a CountedLoopEnd with a mask annotation so as to emit a restore context
+  bool has_vector_mask_set() const { return (_flags & Flag_has_vector_mask_set) != 0; }


I don't see use of this flag.

vnkozlov · 2022-05-23T23:00:56Z

src/hotspot/share/opto/vectorIntrinsics.cpp

    if ((mask_use_type & VecMaskUseLoad) != 0) {
-      if (!Matcher::match_rule_supported_vector(Op_VectorLoadMask, num_elem, elem_bt)) {
+      if (!Matcher::match_rule_supported_vector(Op_VectorLoadMask, num_elem, elem_bt) ||
+          !Matcher::match_rule_supported_vector(Op_LoadVector, num_elem, T_BOOLEAN)) {


Add comment explaining new check. In follow ing places too.

vnkozlov · 2022-05-23T23:05:14Z

src/hotspot/share/runtime/vmStructs.cpp

  declare_c2_type(CMoveVFNode, VectorNode)                                \
  declare_c2_type(CMoveVDNode, VectorNode)                                \
+  declare_c2_type(CompressVNode, VectorNode)                              \
+  declare_c2_type(ExpandVNode, VectorNode)                                \


Not all new nodes listed.

jatin-bhateja · 2022-05-25T06:29:06Z

Hi @vnkozlov , Your comments have been addressed.

vnkozlov · 2022-05-25T16:25:59Z

@jatin-bhateja something wrong with merge. vpadd() is removed. It was added by #8778 and still is used in x86.ad.

vnkozlov

Good.

jatin-bhateja · 2022-05-31T16:01:05Z

/integrate

jatin-bhateja · 2022-05-31T16:01:54Z

Thanks reviewers for your comments.

openjdk · 2022-05-31T16:02:25Z

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

171a7cd: 8286895: InternalError: Exception during analyze
1b44f6c: 8287526: java/nio/channels/FileChannel/LargeMapTest.java fails on 32-bit systems
7ef6993: 8276836: Error in javac caused by switch expression without result expressions: Internal error: stack sim error
c6e0416: 8282863: java/awt/FullScreen/FullscreenWindowProps/FullscreenWindowProps.java fails on Windows 10 with HiDPI screen
6e55a72: 8287438: IGV: scheduling crashes on non-block-start Region with multiple predecessors
8f59422: 8285558: IGV: scheduling crashes on control-unreachable CFG nodes
8a9aeff: 8287497: Use String.contains() instead of String.indexOf() in java.naming
b2ba9fc: 8286180: Enable construction of LogStreamImpl from LogMessageImpl
0c420e0: 8287430: MemorySessionImpl::addOrCleanupIfFail does not rethrow exceptions
1606d55: 8286855: javac error on invalid jar should only print filename
... and 63 more: https://git.openjdk.java.net/jdk/compare/742644e291039e4f2f30cf80718669c06da5809c...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-05-31T16:02:40Z

@jatin-bhateja Pushed as commit 6f6486e.

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

Jatin Bhateja added 2 commits April 26, 2022 22:13

8284960: Integration of JEP 426: Vector API (Fourth Incubator)

c7f0e28

Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960

fe4a064

openjdk bot added hotspot hotspot-dev@openjdk.org core-libs core-libs-dev@openjdk.org csr Pull request needs approved CSR before integration labels Apr 27, 2022

Jatin Bhateja added 4 commits April 28, 2022 05:47

Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960

4a5bea7

Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960

c87593e

8284960: AARCH64 backend changes.

fe74c7f

Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960

39f8ef3

openjdk bot removed the ready Pull request is ready to be integrated label May 13, 2022

Jatin Bhateja added 2 commits May 17, 2022 01:16

Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960

df7eb90

8284960: Adding --enable-preview in vectorAPI benchmarks.

0b7f84b

openjdk bot added ready Pull request is ready to be integrated and removed merge-conflict Pull request has merge conflict with target branch labels May 17, 2022

Jatin Bhateja added 2 commits May 19, 2022 14:10

8284960: Changes to enable jdk.incubator.vector to be treated as prev…

823ba8c

…iew participant. Code re-organization related to Reverse/ReverseByte IR transforms.

Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960

311f323

PaulSandoz reviewed May 19, 2022

View reviewed changes

8284960: Integrating incremental patches.

17a0e38

PaulSandoz reviewed May 19, 2022

View reviewed changes

mcimadamore approved these changes May 20, 2022

View reviewed changes

lahodaj approved these changes May 20, 2022

View reviewed changes

iwanowww approved these changes May 23, 2022

View reviewed changes

vnkozlov reviewed May 23, 2022

View reviewed changes

Jatin Bhateja added 3 commits May 24, 2022 22:51

8284960: Review comments resolved.

a2c9673

Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960

d17f99c

8284960: Post merge cleanups.

0f6e158

vnkozlov approved these changes May 26, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label May 31, 2022

openjdk bot closed this May 31, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 31, 2022

jatin-bhateja deleted the JDK-8284960 branch January 20, 2023 21:25

		assert(VM_Version::supports_avx2(), "");
		assert(VM_Version::supports_evex(), "");

		// The node is a CountedLoopEnd with a mask annotation so as to emit a restore context
		bool has_vector_mask_set() const { return (_flags & Flag_has_vector_mask_set) != 0; }

8284960: Integration of JEP 426: Vector API (Fourth Incubator) #8425

8284960: Integration of JEP 426: Vector API (Fourth Incubator) #8425

Uh oh!

Conversation

jatin-bhateja commented Apr 27, 2022 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Contributors

Reviewing

Uh oh!

bridgekeeper bot commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

jatin-bhateja commented Apr 27, 2022

Uh oh!

jatin-bhateja commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

jatin-bhateja commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

jatin-bhateja commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

jatin-bhateja commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

jatin-bhateja commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

jatin-bhateja commented Apr 27, 2022

Uh oh!

openjdk bot commented Apr 27, 2022

Uh oh!

jatin-bhateja commented Apr 28, 2022

Uh oh!

jatin-bhateja commented Apr 28, 2022

Uh oh!

openjdk bot commented Apr 28, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcimadamore left a comment

Choose a reason for hiding this comment

Uh oh!

lahodaj left a comment

Choose a reason for hiding this comment

Uh oh!

jatin-bhateja commented May 23, 2022

Uh oh!

iwanowww left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jatin-bhateja commented Apr 27, 2022 •

edited by openjdk bot

Loading