8371536: C2: VerifyIterativeGVN should assert on first detected failure #28295

benoitmaillard · 2025-11-13T14:25:21Z

This PR introduces changes in the detection of missing IGVN optimizations. As explained in the JBS issue description, when -XX:VerifyIterativeGVN was introduced, it was helpful to list all the missing optimizations. Such failures occur less frequently now, and the focus has changed to being able to debug such failure quickly and identifying similar or related failures during bug triaging.

In summary, this PR brings the following changes:

Assert at the first verification failure in verify_Optimize instead of attemtping to process all the nodes in the graph. This makes the output easier to parse, and also decreases the overhead of getting to the actual optimization site with a debugger.
Avoid confusing Need to remove from hash before changing edges assert messages by removing the verified node from the hash table before attempting to optimize the node in question.
Provide the failure reason (Ideal, Identity or Value) and the node name in the assert message itself to facilitate identifying related failures in the testing infrastructure during bug triaging.

Example outputs

JDK-8371534: C2: Missed Ideal optimization opportunity with AndL and URShiftL

Before the change, we would get two missed optimizations (the second one is only a consequence of the first one). After the change, we only get the first one, which is the one that actually needs to be fixed. We also get the name of the node in the assert message.

Before

Missed Ideal optimization (can_reshape=false):
The node was replaced by Ideal.
Old node:
dist dump
---------------------------------------------
   1   22  ConI  === 0  [[ 70 81 70 290 81 76 32 37 37 43 48 48 54 59 59 65 336 ]]  #int:1
   1  297  AndL  === _ 298 21  [[ 290 ]]  !orig=[236],[193] !jvms: TestMaskAndRShiftReorder::testURShiftL @ bci:42 (line 81)
   0  290  URShiftL  === _ 297 22  [[ 299 ]]  !orig=[231],[194] !jvms: TestMaskAndRShiftReorder::testURShiftL @ bci:46 (line 82)
The result after Ideal:
dist dump
---------------------------------------------
   1  337  ConL  === 0  [[ 338 ]]  #long:-9
   1  336  URShiftL  === _ 298 22  [[ 338 ]] 
   0  338  AndL  === _ 336 337  [[ ]] 


Missed Ideal optimization (can_reshape=true):
The node was replaced by Ideal.
Old node:
dist dump
---------------------------------------------
   1   22  ConI  === 0  [[ 70 81 70 290 81 76 32 37 37 43 48 48 54 59 59 65 336 ]]  #int:1
   1  297  AndL  === _ 298 21  [[ 290 ]]  !orig=[236],[193] !jvms: TestMaskAndRShiftReorder::testURShiftL @ bci:42 (line 81)
   0  290  URShiftL  === _ 297 22  [[ 299 ]]  !orig=[231],[194] !jvms: TestMaskAndRShiftReorder::testURShiftL @ bci:46 (line 82)
The result after Ideal:
dist dump
---------------------------------------------
   1  337  ConL  === 0  [[ 338 340 ]]  #long:-9
   1  336  URShiftL  === _ 298 22  [[ 338 340 ]]  !orig=[339]
   0  340  AndL  === _ 336 337  [[ ]] 

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/bmaillar/src/jdk/main/open/src/hotspot/share/opto/phaseX.cpp:1105), pid=1949581, tid=1949599
#  assert(!failure) failed: Missed optimization opportunity in PhaseIterGVN
#
# JRE version: Java(TM) SE Runtime Environment (26.0) (fastdebug build 26-internal-bmaillar.open)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 26-internal-bmaillar.open, compiled mode, tiered, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x18897f9]  PhaseIterGVN::verify_optimize()+0xd19
#

After

Missed Ideal optimization (can_reshape=false):
The node was replaced by Ideal.
Old node:
dist dump
---------------------------------------------
   1   22  ConI  === 0  [[ 70 81 70 290 81 76 32 37 37 43 48 48 54 59 59 65 336 ]]  #int:1
   1  297  AndL  === _ 298 21  [[ 290 ]]  !orig=[236],[193] !jvms: TestMaskAndRShiftReorder::testURShiftL @ bci:42 (line 81)
   0  290  URShiftL  === _ 297 22  [[ 299 ]]  !orig=[231],[194] !jvms: TestMaskAndRShiftReorder::testURShiftL @ bci:46 (line 82)
The result after Ideal:
dist dump
---------------------------------------------
   1  337  ConL  === 0  [[ 338 ]]  #long:-9
   1  336  URShiftL  === _ 298 22  [[ 338 ]] 
   0  338  AndL  === _ 336 337  [[ ]] 

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/bmaillar/src/jdk/JDK-8371536/open/src/hotspot/share/opto/phaseX.cpp:1096), pid=1950252, tid=1950270
#  assert(!failure) failed: Missed Ideal optimization opportunity in PhaseIterGVN for URShiftL
#
# JRE version: Java(TM) SE Runtime Environment (26.0) (fastdebug build 26-internal-bmaillar.open)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 26-internal-bmaillar.open, compiled mode, tiered, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x1889a12]  PhaseIterGVN::verify_optimize()+0x8b2
#

JDK-8371558: C2: Missing optimization opportunity in AbsNode::Ideal

Before, we get a confusing Need to remove from hash before changing edges assert.
After, we get the actual cause of the failure as well as the name of the node in the assert itself.

Before

Need to remove from hash before changing edges
 435  SubI  === _ 101 103  [[ 433 434 106 ]]  !orig=[104] !jvms: TestMissingOptAbsZeroMinusX::testAbsI @ bci:16 (line 53)
 434  AbsI  === _ 435  [[ 433 ]]  !orig=105 !jvms: TestMissingOptAbsZeroMinusX::testAbsI @ bci:20 (line 54)
Set at i = 1
 103  LoadI  === _ 7 102  [[ 503 435 105 ]]  @java/lang/Class (java/io/Serializable,java/lang/constant/Constable,java/lang/reflect/AnnotatedElement,java/lang/invoke/TypeDescriptor,java/lang/reflect/GenericDeclaration,java/lang/reflect/Type,java/lang/invoke/TypeDescriptor$OfField):exact+120 *, name=a, idx=4; #int !jvms: TestMissingOptAbsZeroMinusX::testAbsI @ bci:13 (line 53)
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/bmaillar/src/jdk/main/open/src/hotspot/share/opto/phaseX.cpp:3357), pid=1956852, tid=1956870
#  assert(false) failed: Need to remove from hash before changing edges
#
# JRE version: Java(TM) SE Runtime Environment (26.0) (fastdebug build 26-internal-bmaillar.open)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 26-internal-bmaillar.open, compiled mode, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x1881b85]  Node::set_req_X(unsigned int, Node*, PhaseIterGVN*)+0x1e5
#

After

Missed Ideal optimization (can_reshape=false):
The node was reshaped by Ideal.
The result after Ideal:
dist dump
---------------------------------------------
   1  103  LoadI  === _ 7 102  [[ 503 435 105 434 ]]  @java/lang/Class (java/io/Serializable,java/lang/constant/Constable,java/lang/reflect/AnnotatedElement,java/lang/invoke/TypeDescriptor,java/lang/reflect/GenericDeclaration,java/lang/reflect/Type,java/lang/invoke/TypeDescriptor$OfField):exact+120 *, name=a, idx=4; #int !jvms: TestMissingOptAbsZeroMinusX::testAbsI @ bci:13 (line 53)
   0  434  AbsI  === _ 103  [[ 433 ]]  !orig=105 !jvms: TestMissingOptAbsZeroMinusX::testAbsI @ bci:20 (line 54)

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/bmaillar/src/jdk/JDK-8371536/open/src/hotspot/share/opto/phaseX.cpp:1096), pid=1958068, tid=1958086
#  assert(!failure) failed: Missed Ideal optimization opportunity in PhaseIterGVN for AbsI
#
# JRE version: Java(TM) SE Runtime Environment (26.0) (fastdebug build 26-internal-bmaillar.open)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 26-internal-bmaillar.open, compiled mode, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x1889a12]  PhaseIterGVN::verify_optimize()+0x8b2
#

Testing

GitHub Actions
tier1-4, plus some internal testing
Manual testing on recent missed optimizations reproducers, including JDK-8371558, JDK-8371534 and JDK-8371674

Thank you for reviewing!

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-8371536: C2: VerifyIterativeGVN should assert on first detected failure (Enhancement - P4)

Reviewers

Christian Hagedorn (@chhagedorn - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28295

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

Using diff file

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

Using Webrev

Link to Webrev Comment

Bring back old comment Wording

bridgekeeper · 2025-11-13T14:26:32Z

👋 Welcome back bmaillard! 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-11-13T14:27:04Z

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

8371536: C2: VerifyIterativeGVN should assert on first detected failure

Reviewed-by: chagedorn

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

c55287d: 8370890: Start of release updates for JDK 27
45dcc0e: 8371914: PNG defines in CFLAGS can cause compilation errors with external libpng
2735140: 8370939: C2: SIGSEGV in SafePointNode::verify_input when processing MH call from Compile::process_late_inline_calls_no_inline()
... and 391 more: https://git.openjdk.org/jdk/compare/405d5f7a6892426d69409c3975d0c808304b8438...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-11-13T14:28:42Z

@benoitmaillard 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 · 2025-11-13T16:25:53Z

Webrevs

00: Full (ad20446d)

chhagedorn

Good idea, looks good to me!

eme64

@benoitmaillard Thanks for working on this, it will be really helpful for triaging :)

eme64 · 2025-11-14T07:18:02Z

src/hotspot/share/opto/phaseX.cpp

+      if (is_verify_Value()) {
+        bool failure = verify_Value_for(n);
+        assert(!failure, "Missed Value optimization opportunity in PhaseIterGVN for %s", n->Name());
+      }
+      if (is_verify_Ideal()) {
+        bool failure = verify_Ideal_for(n, false) || verify_Ideal_for(n, true);
+        assert(!failure, "Missed Ideal optimization opportunity in PhaseIterGVN for %s", n->Name());
+      }
+      if (is_verify_Identity()) {
+        bool failure = verify_Identity_for(n);
+        assert(!failure, "Missed Identity optimization opportunity in PhaseIterGVN for %s", n->Name());
+      }


The alternative would be to directly assert in the verify methods, but I suppose that would be a bigger code change.

Hmm, I did see some cases in the verify methods that are maybe not directly "missed optimization opportunity" but some other kind of issue. Maybe we should assert directly for those, rather than returning and ending up at this assert.

Look at:
Ideal optimization did not make progress but created new unused nodes.
And
Ideal optimization did not make progress but node hash changed.

That's all I could find now, but you should double check ;)

The alternative would be to directly assert in the verify methods, but I suppose that would be a bigger code change.

Yes, I also considered it. I don't really have a strong opinion, but maybe you do. Asserting directly in the verify methods would allow us to have more targeted asserts, and more accurate reports for triaging. On the other side, as you mentioned, this would be more code changes.

Hmm, I did see some cases in the verify methods that are maybe not directly "missed optimization opportunity" but some other kind of issue. Maybe we should assert directly for those, rather than returning and ending up at this assert.

These are not labelled as such in the printing, but I would argue these are still missed optimization opportunities, aren't they? I mean if things are still moving when calling Ideal, it means that this could have been done earlier.

Honestly, I would do the changes, and just assert in the specific methods. That also helps us with more precise stack traces. The required changes are not that large and surely not that complicated, and we may actually end up with less code over all.

Ideal optimization did not make progress but created new unused nodes.

This one could get triggered even if we don't make progress at all. It may be that some Ideal optimization always generates nodes but then does not actually insert them into the graph. That could be considered wasteful, and that is really all that assert tells us. What do you think?

I agree, doing the changes sounds quite reasonable and is definitely worth it if it gives us more information.

This one could get triggered even if we don't make progress at all. It may be that some Ideal optimization always generates nodes but then does not actually insert them into the graph. That could be considered wasteful, and that is really all that assert tells us. What do you think?

Mmh, I never saw that assert actually getting triggered, interesting. I agree as well, in this case it's a bit different. I will do the changes, thanks for the suggestions!

benoitmaillard added 3 commits November 11, 2025 16:37

Remove node from hash table before calling Ideal in verification

b5c8a23

Assert at first failure

d785fe9

Change assert to print only the cause and the node name

f91ab65

Bring back old comment Wording

openjdk bot changed the title ~~8371536~~ 8371536: C2: VerifyIterativeGVN should assert on first detected failure Nov 13, 2025

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Nov 13, 2025

Add comment for _table.hash_delete(n)

ad20446

benoitmaillard marked this pull request as ready for review November 13, 2025 16:20

openjdk bot added the rfr Pull request is ready for review label Nov 13, 2025

chhagedorn approved these changes Nov 14, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 14, 2025

eme64 suggested changes Nov 14, 2025

View reviewed changes

eme64 mentioned this pull request Nov 24, 2025

8351889: C2 crash: assertion failed: Base pointers must match (addp 344) #25386

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

8371536: C2: VerifyIterativeGVN should assert on first detected failure #28295

8371536: C2: VerifyIterativeGVN should assert on first detected failure #28295

Uh oh!

benoitmaillard commented Nov 13, 2025 •

edited by openjdk bot

Loading

Uh oh!

bridgekeeper bot commented Nov 13, 2025

Uh oh!

openjdk bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

openjdk bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

mlbridge bot commented Nov 13, 2025

Uh oh!

chhagedorn left a comment

Uh oh!

eme64 left a comment

Uh oh!

eme64 Nov 14, 2025

Uh oh!

eme64 Nov 14, 2025

Uh oh!

benoitmaillard Nov 19, 2025

Uh oh!

eme64 Nov 19, 2025

Uh oh!

benoitmaillard Nov 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

8371536: C2: VerifyIterativeGVN should assert on first detected failure #28295

Are you sure you want to change the base?

8371536: C2: VerifyIterativeGVN should assert on first detected failure #28295

Uh oh!

Conversation

benoitmaillard commented Nov 13, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example outputs

JDK-8371534: C2: Missed Ideal optimization opportunity with AndL and URShiftL

JDK-8371558: C2: Missing optimization opportunity in AbsNode::Ideal

Testing

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Nov 13, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

mlbridge bot commented Nov 13, 2025

Webrevs

Uh oh!

chhagedorn left a comment

Choose a reason for hiding this comment

Uh oh!

eme64 left a comment

Choose a reason for hiding this comment

Uh oh!

eme64 Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

eme64 Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

benoitmaillard Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

eme64 Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

benoitmaillard Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

benoitmaillard commented Nov 13, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Nov 13, 2025 •

edited

Loading

openjdk bot commented Nov 13, 2025 •

edited

Loading