RFR: 8350864: C2: verify structural invariants of the Ideal graph [v2]
Marc Chevalier
mchevalier at openjdk.org
Thu Jul 31 13:30:55 UTC 2025
On Thu, 31 Jul 2025 12:44:02 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:
>> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Rename flag as suggested
>
> src/hotspot/share/opto/graphInvariants.cpp line 197:
>
>> 195: };
>> 196:
>> 197: struct HasType : Pattern {
>
> Could we make it slightly more general and accept any predicate on the type? From a previous PR that I worked on I remember that for example for `ModINode` if it has no control input then its divisor input should never be `0`. Maybe this is the kind of properties we could check in the future. This is just a random idea, feel free to ignore.
I think there is a misunderstanding here. I'm talking about node type, as in which C++ class is it, not type as abstract values for nodes. I could rename this struct then. Maybe HasNodeType? `NodeClass` one could see `NodeClass(&Node::is_Region)` that reads almost as "node class is Region". Open to ideas...
Also, in theory, it accepts any method of `Node` of type `bool()`. This could be used for something else. The idea was to make easy to say "I want a Node of type `IfNode` here". It's not that great to do with Opcode because of derived classes. I also considered something that would take any `Node -> bool` function, but that made the simple case harder. Instead of `HasType(&Node::is_If)`, I would have had to write something like `HasType([](const Node& n) { return n.is_If(); })`. Functional programming is possible in C++, but not quite syntactically elegant, and I think readability here is important. If such a need arises, I suggest to add a `UnaryPredicate` (or `NodePredicate` etc.) to do that. If the predicates are complicated enough, the bit of symbols needed for making a lambda doesn't matter so much.
As for your case, yes, we can add that in the future. It could be done with the UnaryPredicate I describe above, or with a more specific pattern that would work on types, and take a method `bool(Type::))()` or a function `bool(const Type&)` and the pattern would take care of finding the type and submitting it to the predicate. Not that it's a lot of work, but it allows to communicate more clearly the intention, in my opinion.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26362#discussion_r2245405917
More information about the hotspot-compiler-dev
mailing list