RFR: 8371536: C2: VerifyIterativeGVN should assert on first detected failure [v9]

[Emanuel Peter]

On Wed, 28 Jan 2026 08:27:01 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...
>
> Benoît Maillard has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove unused declaration

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

> 1204:   } else if (strcmp(phase, "PhaseCCP") == 0) {
> 1205:     assert(false, "PhaseCCP not at fixpoint: analysis result may be unsound for %s", n->Name());
> 1206:   }

The `strcmp` is a little nasty, but I don't have a better solution right now.
But I think we should convert the `else if` condition into an `assert` in the `else` branch.
Imagine someone calls the method with a string we don't match here: would we just silently pass?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28295#discussion_r2735464563