RFR: 8342946: Replace predicate walking code in Loop Unrolling with a predicate visitor

Christian Hagedorn chagedorn at openjdk.org
Thu Nov 7 09:37:03 UTC 2024


#### Replacing the Remaining Predicate Walking and Cloning Code
The goal is to replace and unify all the remaining custom predicate walking and cloning code currently used for:
-  <s>[JDK-8341977](https://bugs.openjdk.org/browse/JDK-8341977): Loop Peeling</s> (integrated with https://github.com/openjdk/jdk/pull/21679))
- <s>[JDK-8342943](https://bugs.openjdk.org/browse/JDK-8342943): Main and Post Loop</s> (integrated with https://github.com/openjdk/jdk/pull/21790)
- <s>[JDK-8342945](https://bugs.openjdk.org/browse/JDK-8342945): `PhaseIdealLoop::get_assertion_predicates()` used for Loop Unswitching and removing useless Template Assertion Predicate</s> (integrated with https://github.com/openjdk/jdk/pull/21918)
- [JDK-8342946](https://bugs.openjdk.org/browse/JDK-8342946): Loop Unrolling (this PR)

---
(Sections taken over from https://github.com/openjdk/jdk/pull/21679 / https://github.com/openjdk/jdk/pull/21790 / https://github.com/openjdk/jdk/pull/21918)

#### 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.
- Keep the semantics which includes to only apply the Template Assertion Predicate processing if there are Parse Predicates. This limitation should eventually be removed. But I want to do that separately at a later point.
---

#### Refactorings of this Patch
This patch replaces the predicate walking in `PhaseIdealLoop::update_main_loop_assertion_predicates()` which is used during Loop Unrolling to update the Template Assertion Predicates for the new unrolled stride and create new Initialized Assertion Predicates reflecting that change while the old Initialized Assertion Predicates with the pre-unrolled stride are killled.
- New visitor `UpdateStrideForAssertionPredicates` takes care of these tasks.
  - Update Template Assertion Predicates: `replace_opaque_stride_input()`
    - Uses new class `ReplaceOpaqueStrideInput` which does a simple BFS on a Template Assertion Expression to find the `OpaqueLoopStrideNode` to update it. Note that the existing class `DataNodesOnPathsToTargets` is not suitable since this class collects all nodes in between which is unnecessary for this task.
  - Create Initialized Assertion Predicate from template: `initialize_from_updated_template()`
    - Calls `clone_and_fold_opaque_loop_nodes()` that uses new strategy class `RemoveOpaqueLoopNodesStrategy` which is passed to the existing method `TemplateAssertionExpression::clone()` to do the Template Assertion Expression cloning. This strategy just folds the `OpaqueLoop*nodes` away for the cloned expression and only keeps their inputs.

#### Follow-up Work
In Loop Unrolling, we only update the stride and not the init value. Thus, we actually only require to update the Last Value Assertion Predicates because the Init Value Assertion Predicates do not use `OpaqueLoopStride`. So, we also would not be required to kill the old Init Value Initialized Assertion Predicates. This was already an inefficiency before but could now be tackled since we keep track of whether an Assertion Predicate is for the init or last value with `AssertionPredicateType`. I filed [JDK-8343745](https://bugs.openjdk.org/browse/JDK-8343745) for that.

Thanks,
Christian

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

Commit messages:
 - Add const
 - 8342946: Replace predicate walking code in Loop Unrolling with a predicate visitor

Changes: https://git.openjdk.org/jdk/pull/21944/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21944&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8342946
  Stats: 203 lines in 4 files changed: 161 ins; 29 del; 13 mod
  Patch: https://git.openjdk.org/jdk/pull/21944.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/21944/head:pull/21944

PR: https://git.openjdk.org/jdk/pull/21944


More information about the hotspot-compiler-dev mailing list