RFR: 8341977: Replace predicate walking and cloning code for Loop Peeling with a predicate visitor

Christian Hagedorn chagedorn at openjdk.org
Thu Oct 24 11:00:37 UTC 2024


On Thu, 24 Oct 2024 10:45:12 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> #### Replacing the Remaining Predicate Walking and Cloning Code 
> In the next series of patches (this, [JDK-8342943](https://bugs.openjdk.org/browse/JDK-8342943), [JDK-8342945](https://bugs.openjdk.org/browse/JDK-8342945), and [JDK-8342946](https://bugs.openjdk.org/browse/JDK-8342946)) I want to replace the predicate walking and cloning code used for Loop Peeling, Pre/Main/Post Loops, Loop Unswitching, removing useless Assertion Predicates and Loop Unrolling with new `PredicateVisitors` which can be used in combination with the new `PredicateIterator`.
> 
> #### Single Template Assertion Predicate Check
> This replacement allows us to have a single `TemplateAssertionPredicate::is_predicate()` check that is called for all predicate matching code. This enables the removal of uncommon traps for Template Assertion Predicates with [JDK-8342047](https://bugs.openjdk.org/browse/JDK-8342047) which is a missing piece in order to fix the remaining problems with Assertion Predicates ([JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981)).
> 
> #### Common Refactorings for all the Patches in this Series
> In each of the patch, I will do similar refactoring ideas:
> - Replace the existing code in the corresponding `PhaseIdealLoop` method with call to a new (or existing) predicate visitor which extends the `PredicateVisitor` interface. 
> - The visitor implements the Assertion Predicate `visit()` methods to implement the cloning and initialization of the Template Assertion Predicates.
> - The predicate visitor is then passed to the `PredicateIterator` which walks through all predicates found at a loop and applies the visitor for each predicate. 
> - The visitor creates new nodes (if there are Template Assertion Predicates) either in place or at the loop entry of a target loop. In the latter case, the calling code of the `PredicateIterator` must make sure to connect the tail of the newly created predicate chain after the old loop entry to the target loop head (see **P1** PR comment).
> - Keep the semantics which includes to only apply the Template Assertion Predicate processing if there are Parse Predicates (see **P2** PR comment). This limitation should eventually be removed. But I want to do that separately at a later point.
> 
> #### Refactorings of this Patch
> This first patch replaces the predicate walking and cloning code for Loop Peeling and lays the foundation for the replacement for main/post loops ([JDK-8342943](https://bugs.openjdk.org/browse/JDK-8342943)) which is quite similar. Th...

src/hotspot/share/opto/loopTransform.cpp line 819:

> 817:   if (counted_loop && UseLoopPredicate) {
> 818:       initialize_assertion_predicates_for_peeled_loop(new_head->as_CountedLoop(), head->as_CountedLoop(),
> 819:                                                       first_node_index_in_cloned_loop_body, old_new);

We can infer many values from other variables.

src/hotspot/share/opto/loopTransform.cpp line 1451:

> 1449:   }
> 1450: }
> 1451: #endif // ASSERT

Noticed that after JDK-8342043, `count_opaque_loop_nodes()` can also be made debug-build only.

src/hotspot/share/opto/loopTransform.cpp line 1975:

> 1973:                                                                      const Node_List& old_new) {
> 1974:   const NodeInOriginalLoopBody node_in_original_loop_body(first_node_index_in_cloned_loop_body, old_new);
> 1975:   create_assertion_predicates_at_loop(peeled_loop_head, remaining_loop_head, node_in_original_loop_body);

We can do something similar for the main and post loop which will be proposed in the next PR.

src/hotspot/share/opto/loopTransform.cpp line 1994:

> 1992:     _igvn.replace_input_of(target_outer_loop_head, LoopNode::EntryControl, last_created_node);
> 1993:     set_idom(target_outer_loop_head, last_created_node, dom_depth(target_outer_loop_head));
> 1994:   }

**P1** (see PR description)

src/hotspot/share/opto/loopnode.hpp line 955:

> 953:   Node* clone_template_assertion_predicate(IfNode* iff, Node* new_init, Node* predicate, Node* uncommon_proj, Node* control,
> 954:                                            IdealLoopTree* outer_loop, Node* new_control);
> 955:  public:

Now needs to be called from `AssertionPredicatesForLoop`.

src/hotspot/share/opto/predicates.cpp line 744:

> 742:     // Only process if we are in the correct Predicate Block.
> 743:     return;
> 744:   }

**P2** (see PR description)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21679#discussion_r1814740963
PR Review Comment: https://git.openjdk.org/jdk/pull/21679#discussion_r1814742470
PR Review Comment: https://git.openjdk.org/jdk/pull/21679#discussion_r1814743208
PR Review Comment: https://git.openjdk.org/jdk/pull/21679#discussion_r1814743609
PR Review Comment: https://git.openjdk.org/jdk/pull/21679#discussion_r1814744478
PR Review Comment: https://git.openjdk.org/jdk/pull/21679#discussion_r1814745959


More information about the hotspot-compiler-dev mailing list