RFR: 8305635: Replace Parse Predicate IfNode with new ParsePredicateNode and route predicate queries through dedicated classes

[Tobias Hartmann]

On Tue, 16 May 2023 15:27:26 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> This is the second PR towards fixing the issues with Assertion Predicates ([JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981)). This patch still does not change anything in the way the old Assertion Predicates work. The only observable change in the IR is the introduction of a new `ParsePredicateNode` instead of using an `IfNode` to better distinguish these dedicated Parse Predicates added during parsing (they still use the same inputs with `Opaque1Nodes` as before).
> 
> Changes include:
> - New `ParsePredicateNode` as subclass of `IfNode` and related code updates to make this work.
> - Moving predicate access code (skipping, matching etc.), including the called predicate methods found in `PhaseIdealLoop`, to dedicated `Predicates/ParsePredicates` classes. This is only a first step and these classes are further updated in the next PR. They can therefore be seen as an intermediate state to make the entire update to predicate classes easier to follow. As a consequence, I've tried to not clean the code up too much in these classes.
>   - Cleanup of touched code (dead code, variable renaming, code style)
>   -  Added comments (e.g. for some special case in Loop Predication)
> 
> For more background, have a look at the first PR: #13864
> 
> Thanks,
> Christian

Changes requested by thartmann (Reviewer).

src/hotspot/share/opto/cfgnode.hpp line 460:

> 458: // Loop Parse Predicate, Profiled Loop Parse Predicate (both used by Loop Predication), and Loop Limit Check Parse
> 459: // Predicate (used for integer overflow checks when creating a counted loop).
> 460: // More information about predicates can be found at loopPredicate.cpp.

Suggestion:

// More information about predicates can be found in loopPredicate.cpp.

src/hotspot/share/opto/cfgnode.hpp line 462:

> 460: // More information about predicates can be found at loopPredicate.cpp.
> 461: class ParsePredicateNode : public IfNode {
> 462:   Deoptimization::DeoptReason _deopt_reason;

Don't we need to override `Node::hash()` and `Node::cmp()` here to account for the `_deopt_reason` field?

src/hotspot/share/opto/node.hpp line 1026:

> 1024: 
> 1025:   // Is 'n' possibly a loop entry (i.e. a Parse Predicate projection)?
> 1026:   static bool is_maybe_loop_entry(Node* n) {

Suggestion:

  static bool may_be_loop_entry(Node* n) {

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

PR Review: https://git.openjdk.org/jdk/pull/14017#pullrequestreview-1430355381
PR Review Comment: https://git.openjdk.org/jdk/pull/14017#discussion_r1196301152
PR Review Comment: https://git.openjdk.org/jdk/pull/14017#discussion_r1196303119
PR Review Comment: https://git.openjdk.org/jdk/pull/14017#discussion_r1196299602