RFR: 8342945: Replace predicate walking code in get_assertion_predicates() used for Loop Unswitching and cleaning useless Template Assertion Predicates with a predicate visitor
Christian Hagedorn
chagedorn at openjdk.org
Wed Nov 6 07:06:01 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)
- [JDK-8342945](https://bugs.openjdk.org/browse/JDK-8342945): `PhaseIdealLoop::get_assertion_predicates()` used for Loop Unswitching and removing useless Template Assertion Predicate (this PR)
- [JDK-8342946](https://bugs.openjdk.org/browse/JDK-8342946): Loop Unrolling (upcoming)
---
(Sections taken over from https://github.com/openjdk/jdk/pull/21679 / https://github.com/openjdk/jdk/pull/21790))
#### 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::get_assertion_predicates()` which is used for Loop Unswitching and removing useless Template Assertion Predicates (called from `PhaseIdealLoop::collect_useful_template_assertion_predicates_for_loop()`).
- Note that the cloning code in Loop Unswitching is not replaced, yet, because we clone the Template Assertion Predicates in the original order as currently found in the graph which also allowed us to use `PhaseIdealLoop::create_new_if_for_predicate()`.
This means that we first walk from the loop entry to the last Template Assertion Predicate and then start cloning them in the reverse order (which ensures that we keep the original order of the Template Assertion Predicates). I don't think that keeping the original order is a strong requirement.
Once we replace the UCTs with halt nodes, we do not require to call `create_new_if_for_predicate()` anymore and could theoretically just clone and initialize the Template Assertion Predicates in the opposite order as originally found in the graph which is easier to implement. This is currently also done for the other loop opts that require Assertion Predicates cloning/initialization. I think it's probably safe to do this for Loop Unswitching as well once we replace UCTs with halt nodes (@rwestrel what do you think?).
If at some point, we need to keep the Assertion Predicate order, we can just add this functionality to the `PredicateIterator` classes. Anyhow, I'm leaving this code in`clone_assertion_predicates_to_unswitched_loop()` as it is for now and revisit it later again.
Thanks,
Christian
-------------
Commit messages:
- 8342945: Replace predicate walking code in get_assertion_predicates() used for Loop Unswitching and cleaning useless Template Assertion Predicates with a predicate visitor
Changes: https://git.openjdk.org/jdk/pull/21918/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21918&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8342945
Stats: 59 lines in 4 files changed: 22 ins; 22 del; 15 mod
Patch: https://git.openjdk.org/jdk/pull/21918.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/21918/head:pull/21918
PR: https://git.openjdk.org/jdk/pull/21918
More information about the hotspot-compiler-dev
mailing list