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

[Benoît Maillard]

On Thu, 18 Dec 2025 13:53:36 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Benoît Maillard has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
>> 
>>  - Remove trailing whitespace
>>  - Merge branch 'master' into JDK-8371536
>>  - More style
>>  - Style and comments
>>  - Merge branch 'master' into JDK-8371536
>>  - Assert directly in the verify methods
>>  - Add comment for _table.hash_delete(n)
>>  - Change assert to print only the cause and the node name
>>    
>>    Bring back old comment
>>    
>>    Wording
>>  - Assert at first failure
>>  - Remove node from hash table before calling Ideal in verification
>
> src/hotspot/share/opto/phaseX.cpp line 1202:
> 
>> 1200:   tty->print_cr("%s", ss.as_string());
>> 1201: 
>> 1202:   assert(false, "Missed Value optimization opportunity in PhaseIterGVN for %s", n->Name());
> 
> What if it gets called during CCP? Then it is not just a missed opportunity, but possibly a correctness problem.
> 
> I wonder if we should have different assert messages here. We could even just pass a string into the method, either `IGVN` or `CCP`.
> 
> What do you think?

Good point, I didn't think of that. Passing a string into the method would be one solution. Another one would be to keep the `bool` return type for `verify_Value_for` and assert at the call site (just as it was before). I think this feels a bit more natural that passing an assert message as parameter. What do you think?

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

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