RFR: 8350864: C2: verify structural invariants of the Ideal graph [v3]

[Marc Chevalier]

On Mon, 25 Aug 2025 14:27:27 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Benoît's comments
>
> src/hotspot/share/opto/graphInvariants.cpp line 338:
> 
>> 336:       if (out->is_CFG()) {
>> 337:         cfg_out++;
>> 338:         ctrl_succ.push(out);
> 
> Seems you do these in a pair. So why do you need `cfg_out` at all? Can you not take the length/size of `ctrl_succ`? After all, it counts duplicates too (hope that is intended).

True. And yes, duplicated input must still be counted!

> src/hotspot/share/opto/graphInvariants.cpp line 413:
> 
>> 411:       ss.print_cr("%s nodes' 0-th input must be itself or nullptr (for a copy Region).", center->Name());
>> 412:       return CheckResult::FAILED;
>> 413:     }
> 
> Absolutely subjective: checking `self != center` is more about `self`, checking `center != self` is more about `center`. So I would use `self != center` :rofl: 
> Suggestion:
> 
>     if (self != center || (center->is_Region() && self == nullptr)) {
>       ss.print_cr("%s nodes' 0-th input must be itself or nullptr (for a copy Region).", center->Name());
>       return CheckResult::FAILED;
>     }

yes

> src/hotspot/share/opto/graphInvariants.cpp line 447:
> 
>> 445:                     And::make(
>> 446:                         new NodeClass(&Node::is_IfTrue),
>> 447:                         new HasAtLeastNInputs(1),
> 
> Can an `IfTrue` have more than 1 input?

I surely hope not!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26362#discussion_r2321724188
PR Review Comment: https://git.openjdk.org/jdk/pull/26362#discussion_r2321730065
PR Review Comment: https://git.openjdk.org/jdk/pull/26362#discussion_r2321743991