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

[Emanuel Peter]

On Wed, 19 Nov 2025 08:44:16 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:

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

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

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