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

[Benoît Maillard]

On Fri, 14 Nov 2025 07:19:12 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> 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.
>
> 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.

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

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