8319889: Vector API tests trigger VM crashes with -XX:+StressIncrementalInlining

[jatin-bhateja]

This bug fix patch fixes a crash occurring due to combined effect of bi-morphic inlining, exception handling, randomized incremental inlining. In this case top level slice API is invoked using concrete 256 bit vector, some of the intermediate APIs within sliceTemplate are marked for lazy inlining due to randomized IncrementalInlining, these APIs returns an abstract vector which when used for virtual dispatch of subsequent APIs results into bi-morphic inlining on account of multiple profile based receiver types. Consider following code snippet.

    ByteVector sliceTemplate(int origin, Vector<Byte> v1) {
        ByteVector that = (ByteVector) v1;
        that.check(this);
        Objects.checkIndex(origin, length() + 1);
        VectorShuffle<Byte> iota = iotaShuffle();
        VectorMask<Byte> blendMask = iota.toVector().compare(VectorOperators.LT, (broadcast((byte)(length() - origin))));  [A]
        iota = iotaShuffle(origin, 1, true);     [B]    
        return that.rearrange(iota).blend(this.rearrange(iota), blendMask); [C]
    }

Receiver for sliceTemplate is a 256 bit vector, parser defers inlining of toVector() API (see code at line A) and generates a Call IR returning an abstract vector. This abstract vector then virtually dispatches compare API. Compiler observes multiple profile based receiver types (128 and 256 bit byte vectors) for compare API and parser generates a chain of PredictedCallGenerators for bi-morphically inlining it.

     PredictedCallGenerators (Vector.compare)
          PredictedCallGenerators (Byte256Vector.compare)
             ParseGenerator (Byte256Vector.compare)               [D]
             UncommonTrap (receiver other than Byte256Vector)
          PredictedCallGenerators (Byte128Vector.compare)
             ParseGenerator (Byte128Vector.compare)               [E]
             UncommonTrap (receiver other than Byte128Vector)     [F]
          PredictedCallGenerators (UncommonTrap)
     [converged state] = Merge JVM State orginating from C and E  [G]

Since top level receiver of sliceTemplate is Byte256Vector hence while executing the call generator for Byte128Vector.compare (see code at line E) compiler observes a mismatch b/w incoming argument species i.e. one argument is a 256 bit vector while other is 128 bit vector and throws an exception.

At state convergence point (see code at line G), since one of the control path resulted into an exception, compiler propagates the JVM state of other control path comprising of Byte256Mask to downstream graph after bookkeeping the pending exception state.

Similar to toVector API, iotaShuffle (see code at line B) is also lazily inlined and returns an abstract vector which results into bi-morphic inlining of rearrange. State convergence due to bi-morphic inlining of rearrange results into generation of an abstract ByteVector (Phi Byte128Vector Byte256Vector) which further causes bi-morphic inlining of blend API due to multiple profile based receiver types.

Byte128Vector.blend Java implementation explicitly cast incoming mask (Byte256Mask) by Byte128Mask type which results into a null reference, this causes a crash while unboxing the mask during inline expansion of blend. To be safe, relaxing the null checking constraint during unboxing to disable intrinsification.

All existing Vector API JTREG tests are passing with -XX:+StressIncrementalInlining at various AVX levels.

Please review and share your feedback.

Best Regards, Jatin

---

Progress

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

Issue

• JDK-8319889: Vector API tests trigger VM crashes with -XX:+StressIncrementalInlining (Bug - P4)

Reviewers

• Vladimir Ivanov (@iwanowww - Reviewer) ⚠️ Review applies to 43c1e399

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18282

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: 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[bot]]

@jatin-bhateja 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[bot]]

Webrevs

• 01: Full - Incremental (39edea6f)
• 00: Full (43c1e399)

[openjdk[bot]]

@jatin-bhateja 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:

8319889: Vector API tests trigger VM crashes with -XX:+StressIncrementalInlining

Reviewed-by: vlivanov, sviswanathan

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 102 new commits pushed to the master branch:

• 3f2e849c54c2a9c55e3b5c9f5a6d3478b83144e3: 8280392: java/awt/Focus/NonFocusableWindowTest/NonfocusableOwnerTest.java failed with "RuntimeException: Test failed."
• c901da48e30d53cb8e4e3c1f0584c5f2d3d095f1: 8327098: GTest needs larger combination limit
• 9e32db266e4c3cc9be273fa6b77112832a43ba4a: 8328309: Remove malformed masked shift instruction selection patterns
• fc0472baf9bad298e853bf2ca3d10dc9415272cd: 8328000: Convert /java/awt/im/8154816/bug8154816.java applet test to main
• 85fc47c81af81a595dc88e61454d8ba2d860f301: 8327180: Failed: java/io/ObjectStreamClass/ObjectStreamClassCaching.java#G1
• 569b05addf69698fc93026b4dca69bc6ca0920b6: 8327818: Implement Kerberos debug with sun.security.util.Debug
• dec68d7e36a9436468594416272c44a2afbece8d: 8328234: Remove unused nativeUtils files
• 7734466c4627c51b2c24b4b4dbf6db4184607aa3: 8325871: Move StringTable and SymbolTable rehashing calls
• 9e98118f289e98ca9f3be2a274e0ddf8822aaa7c: 8328177: Move LDFLAGS_JDK[LIB/EXE] to JdkNativeCompilation.gmk
• f3af91815a662b195938b26962a8670a4a692220: 8327945: Inline HasScavengableOops
• ... and 92 more: https://git.openjdk.org/jdk/compare/5d4bfad12b650b9f7c512a071830c58b8f1d020b...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.

[jatin-bhateja]

Overall, both fixes look good.

I suggest to handle the bugs separately (as 2 bug fixes).

Removed shift pattern related changes and created a separate JBS entry for it https://bugs.openjdk.org/browse/JDK-8328309

[jatin-bhateja]

/integrate

[openjdk[bot]]

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

• 3f2e849c54c2a9c55e3b5c9f5a6d3478b83144e3: 8280392: java/awt/Focus/NonFocusableWindowTest/NonfocusableOwnerTest.java failed with "RuntimeException: Test failed."
• c901da48e30d53cb8e4e3c1f0584c5f2d3d095f1: 8327098: GTest needs larger combination limit
• 9e32db266e4c3cc9be273fa6b77112832a43ba4a: 8328309: Remove malformed masked shift instruction selection patterns
• fc0472baf9bad298e853bf2ca3d10dc9415272cd: 8328000: Convert /java/awt/im/8154816/bug8154816.java applet test to main
• 85fc47c81af81a595dc88e61454d8ba2d860f301: 8327180: Failed: java/io/ObjectStreamClass/ObjectStreamClassCaching.java#G1
• 569b05addf69698fc93026b4dca69bc6ca0920b6: 8327818: Implement Kerberos debug with sun.security.util.Debug
• dec68d7e36a9436468594416272c44a2afbece8d: 8328234: Remove unused nativeUtils files
• 7734466c4627c51b2c24b4b4dbf6db4184607aa3: 8325871: Move StringTable and SymbolTable rehashing calls
• 9e98118f289e98ca9f3be2a274e0ddf8822aaa7c: 8328177: Move LDFLAGS_JDK[LIB/EXE] to JdkNativeCompilation.gmk
• f3af91815a662b195938b26962a8670a4a692220: 8327945: Inline HasScavengableOops
• ... and 92 more: https://git.openjdk.org/jdk/compare/5d4bfad12b650b9f7c512a071830c58b8f1d020b...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@jatin-bhateja Pushed as commit 2dd5fba3bd37c577b8442b67a67dbcb22b9a530e.

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

[TobiHartmann]

I'm late here but is there a reason no regression test was added?