RFR: 8371536: C2: VerifyIterativeGVN should assert on first detected failure

[Emanuel Peter]

On Thu, 13 Nov 2025 14:25:21 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:

> 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 ](https://bugs.openjdk.org/browse/JDK-8371534)
> 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.
> <details>
> <summary>Before</summary>
> 
> 
> 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...

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

src/hotspot/share/opto/phaseX.cpp line 1101:

> 1099:         bool failure = verify_Identity_for(n);
> 1100:         assert(!failure, "Missed Identity optimization opportunity in PhaseIterGVN for %s", n->Name());
> 1101:       }

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.

-------------

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28295#pullrequestreview-3463193618
PR Review Comment: https://git.openjdk.org/jdk/pull/28295#discussion_r2526093232