8367333: C2: Vector math operation intrinsification failure #27263

iwanowww · 2025-09-12T19:14:18Z

As part of JDK-8353786, C2 support for operations backed by the vector math library was completely removed. On JDK side, there is a special dispatching logic added to avoid intrinsic calls in jdk.internal.vm.vector.VectorSupport. But it's still possible to observe such paradoxical situations (intrinsic calls with obsolete operation IDs) when processing effectively dead code.

Consider FloatVector::lanewiseTemplate:

    FloatVector lanewiseTemplate(VectorOperators.Unary op) {
        if (opKind(op, VO_SPECIAL)) {
            ...                             
            else if (opKind(op, VO_MATHLIB)) {
                return unaryMathOp(op);
            }
        }
        int opc = opCode(op);
        return VectorSupport.unaryOp(opc, ...);
    }

At runtime, unaryMathOp is unconditionally invoked, but during compilation it's possible to end up with an intrinsification attempt of VectorSupport.unaryOp() before opKind(op, VO_SPECIAL) is inlined.

It can be reliably reproduced -XX:+StressIncrementalInlining flag.

The fix is to fail-fast intrinsification rather than crashing the VM.

Testing: tier1 - tier4

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-8367333: C2: Vector math operation intrinsification failure (Bug - P3)

Reviewers

Aleksey Shipilev (@shipilev - Reviewer) Review applies to 66892f1d
Jatin Bhateja (@jatin-bhateja - Reviewer) Review applies to 66892f1d
Emanuel Peter (@eme64 - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27263

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-09-12T19:16:00Z

👋 Welcome back vlivanov! 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 · 2025-09-12T19:16:26Z

@iwanowww 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:

8367333: C2: Vector math operation intrinsification failure

Reviewed-by: epeter, shade, jbhateja

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

c2c44a0: 8367724: Remove Trailing Return Types from undecided list
e107179: 8367017: Remove legacy checks from WrappedToolkitTest and convert from bash
b75e35c: 8365858: FilteredJavaFieldStream is unnecessary
... and 39 more: https://git.openjdk.org/jdk/compare/84aa295227749f5daf73100550355ac6d56a6eca...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.

openjdk · 2025-09-12T19:17:16Z

@iwanowww
The hotspot-compiler label was successfully added.

mlbridge · 2025-09-12T20:53:29Z

Webrevs

01: Full - Incremental (f63e76ce)
00: Full (66892f1d)

shipilev

Looks reasonable! Thanks.

jatin-bhateja

LGTM

Best Regards

jatin-bhateja · 2025-09-15T15:24:58Z

test/hotspot/jtreg/compiler/vectorapi/TestVectorMathLib.java

+ *                   -XX:+StressIncrementalInlining
+ *                       compiler.vectorapi.TestVectorMathLib


Suggested change

* -XX:+StressIncrementalInlining

* compiler.vectorapi.TestVectorMathLib

* -XX:+StressIncrementalInlining compiler.vectorapi.TestVectorMathLib

Ok, fixed. I prefer to keep test class name on a separate line.

eme64 · 2025-09-16T06:44:49Z

test/hotspot/jtreg/compiler/vectorapi/TestVectorMathLib.java

+/*
+ * @test
+ * @bug 8367333
+ * @requires vm.compiler2.enabled


Do you need this @requires? It might be nice to be able to run this with other compilers too.

It's intended as C2-specific regression test and it relies on C2-specific VM flags. Vector API unit tests (under test/jdk/jdk/incubator/vector/) exercise the very same functionality, but don't specify flags required to trigger the bug.

I leave it up to you. You can always ignore unrecognized flags. And tests tend to diverge over time, so maybe a little duplication would not hurt. But up to you.

And if it is a duplication, you should probably leave a comment linking it to the other one.
Also: why not just add the extra run over at the original test?

Also: why not just add the extra run over at the original test?

test/jdk/jdk/incubator/vector/*VectorTests.java are huge and already override default timeout setting. But running them with -XX:+StressIncrementalInlining does make some sense. (Maybe not by default, but as part of some stress testing configuration we have.)

eme64 · 2025-09-16T06:46:27Z

test/hotspot/jtreg/compiler/vectorapi/TestVectorMathLib.java

+ * @run main/othervm -Xbatch -XX:-TieredCompilation -XX:CompileCommand=quiet
+ *                   -XX:CompileCommand=compileonly,compiler.vectorapi.TestVectorMathLib::test*
+ *                   -XX:+StressIncrementalInlining
+ *                       compiler.vectorapi.TestVectorMathLib


Like @jatin-bhateja mentioned: alignment is off.
I'd also like to see a run without flags, maybe with only -XX:CompileCommand=compileonly,compiler.vectorapi.TestVectorMathLib::test*

Again, IMO it doesn't make sense to run the regression test without stressing incremental inlining. Otherwise, it duplicates existing tests.

iwanowww

Thanks for the reviews.

iwanowww · 2025-09-16T20:00:10Z

test/hotspot/jtreg/compiler/vectorapi/TestVectorMathLib.java

+/*
+ * @test
+ * @bug 8367333
+ * @requires vm.compiler2.enabled


It's intended as C2-specific regression test and it relies on C2-specific VM flags. Vector API unit tests (under test/jdk/jdk/incubator/vector/) exercise the very same functionality, but don't specify flags required to trigger the bug.

iwanowww · 2025-09-16T20:00:57Z

test/hotspot/jtreg/compiler/vectorapi/TestVectorMathLib.java

+ * @run main/othervm -Xbatch -XX:-TieredCompilation -XX:CompileCommand=quiet
+ *                   -XX:CompileCommand=compileonly,compiler.vectorapi.TestVectorMathLib::test*
+ *                   -XX:+StressIncrementalInlining
+ *                       compiler.vectorapi.TestVectorMathLib


Again, IMO it doesn't make sense to run the regression test without stressing incremental inlining. Otherwise, it duplicates existing tests.

iwanowww · 2025-09-16T20:04:04Z

test/hotspot/jtreg/compiler/vectorapi/TestVectorMathLib.java

+ *                   -XX:+StressIncrementalInlining
+ *                       compiler.vectorapi.TestVectorMathLib


Ok, fixed. I prefer to keep test class name on a separate line.

eme64

Looks reasonable.

I leave it up to you with the @requires. I was wondering why not just add the extra run with special flags to the original test? But I don't want to hold up the PR, so up to you ;)

iwanowww · 2025-09-17T21:33:43Z

Thanks for the reviews, Aleksey, Jatin, and Emanuel.

/integrate

openjdk · 2025-09-17T21:34:16Z

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

919f5fa: 8367787: Expand use of representation equivalence terminology in Float16
f682f07: 8367796: Rename AtomicAccess gtests
0756ecb: 8367031: [backout] Change java.time month/day field types to 'byte'
... and 62 more: https://git.openjdk.org/jdk/compare/84aa295227749f5daf73100550355ac6d56a6eca...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-09-17T21:34:25Z

@iwanowww Pushed as commit aa36799.

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

fix

66892f1

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Sep 12, 2025

iwanowww marked this pull request as ready for review September 12, 2025 20:49

openjdk bot added the rfr Pull request is ready for review label Sep 12, 2025

shipilev approved these changes Sep 15, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 15, 2025

jatin-bhateja approved these changes Sep 15, 2025

View reviewed changes

eme64 suggested changes Sep 16, 2025

View reviewed changes

review feedback

f63e76c

iwanowww commented Sep 16, 2025

View reviewed changes

openjdk bot removed the ready Pull request is ready to be integrated label Sep 16, 2025

eme64 approved these changes Sep 17, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 17, 2025

openjdk bot added the integrated Pull request has been integrated label Sep 17, 2025

openjdk bot closed this Sep 17, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 17, 2025

eme64 mentioned this pull request Sep 18, 2025

8367969: C2: compiler/vectorapi/TestVectorMathLib.java fails without UnlockDiagnosticVMOptions #27359

Closed

3 tasks

iwanowww deleted the 8367333.vector_math branch November 18, 2025 22:41

		* -XX:+StressIncrementalInlining
		* compiler.vectorapi.TestVectorMathLib

	* -XX:+StressIncrementalInlining
	* compiler.vectorapi.TestVectorMathLib
	* -XX:+StressIncrementalInlining compiler.vectorapi.TestVectorMathLib

8367333: C2: Vector math operation intrinsification failure #27263

8367333: C2: Vector math operation intrinsification failure #27263

Uh oh!

Conversation

iwanowww commented Sep 12, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Sep 12, 2025

Uh oh!

openjdk bot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Sep 12, 2025

Uh oh!

mlbridge bot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

shipilev left a comment

Choose a reason for hiding this comment

Uh oh!

jatin-bhateja left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

iwanowww Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

eme64 left a comment

Choose a reason for hiding this comment

Uh oh!

iwanowww commented Sep 17, 2025

Uh oh!

openjdk bot commented Sep 17, 2025

Uh oh!

openjdk bot commented Sep 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

iwanowww commented Sep 12, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Sep 12, 2025 •

edited

Loading

mlbridge bot commented Sep 12, 2025 •

edited

Loading

jatin-bhateja left a comment •

edited

Loading

iwanowww Sep 17, 2025 •

edited

Loading