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

[Emanuel Peter]

On Thu, 4 Sep 2025 11:08:20 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

>> src/hotspot/share/opto/graphInvariants.cpp line 279:
>> 
>>> 277:       return CheckResult::NOT_APPLICABLE;
>>> 278:     }
>>> 279:     CheckResult r = PatternBasedCheck::check(center, reachable_cfg_nodes, steps, path, ss);
>> 
>> Could this not be solved with a `OrPattern`?
>> 
>> Or::make( <not is_If ...,
>>   <the And::make from above>
>> )
>> 
>> Not sure that's worth it...
>
> I understand that OrPatterns are tempting! I also thought about it, it's naturally the dual of `And`. At this point, they are not actually a good idea.
> 
> First, they cannot provide good reporting. When an `And` is failing, we can at least blame the first thing that fails: "I followed this path, I expected to find 5 inputs (for instance), there are only 2!". With `Or` we would get that and... maybe it's fine? Maybe not? Depends on the next branches, and if it ends up failing, how to provide a good message?
> 
> Also, they cause a mess with binding. If a branch contains a `Bind`, one cannot know which branch matched and whether the content of the `Node` pointer given to `Bind` is trustworthy. We can't even rely on a test whether the pointer was set because the execution of a branch might find a `Bind` first, run it, assign the pointer and later fail, and then the `Bind` is not to use. This is a common problem with pattern matching in functional programming: the same bindings must appear (with same types) on each branch of or-patterns. But we have no such mechanisms to enforce that yet, and it seems like setting a trap for future us.
> 
> There is also relatively few use cases, and that would not profit a lot from a `Or` pattern. Maybe in the future, we will have more interesting usecases and we will see how to address these issues. But for now, I think we should not include it for now rather than making a bad choice.
> 
> By the way, I think something that has more future than `Or` is rather a case analysis: `IfThenElse(CondtionPattern, TrueBranchPattern, FalseBranchPattern)` if CondtionPattern is true, then we try to match TrueBranchPattern, otherwise FalseBranchPattern. This is better for reporting since we know which branch to we expect to be true, and so to blame (assuming we don't blame CondtionPattern, but we can include that in the message possibly). This still has the binding consistency issue, but more boilerplate could help (querying the set of pointers that would be set in each branch with helping methods...). Yet, let's wait and see.

Hmm, yes. I did later on think about binding.
If we ever use pattern matching for IGVN optimizations, we need to be able to do or-like patterns, maybe even over 2, 3, ...n many branches. And then bind to something.

And you are right, reporting could also be an issue.
Maybe there could be some kind of reporting still though: we could evaluate both branches and report where each fails.

I saw multiple uses, so maybe at some point an Or could be justified. But maybe not yet.

I think this is an interesting thread, so a shame you closed it as "resolved" ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26362#discussion_r2324343167