RFR: 8350578: Refactor useless Parse and Template Assertion Predicate elimination code by using a PredicateVisitor

Christian Hagedorn chagedorn at openjdk.org
Thu Mar 13 06:55:01 UTC 2025


This patch cleans the Parse and Template Assertion Predicate elimination code up. We now use a single `PredicateVisitor` and share code in a new `EliminateUselessPredicates` class which contains the code previously found in `PhaseIdealLoop::eliminate_useless_predicates()`.

### Unified Logic to Clean Up Parse and Template Assertion Predicates
We now use the following algorithm:
https://github.com/openjdk/jdk/blob/5e4b6ca0ddafa80eee60690caacd257b74305d4e/src/hotspot/share/opto/predicates.cpp#L1174-L1179

This is different from the old algorithm where we used a single boolean state `_useless`. But that does no longer work because when we first mark Template Assertion Predicates useless, we are no longer visiting them when iterating through predicates:

https://github.com/openjdk/jdk/blob/a21fa463c4f8d067c18c09a072f3cdfa772aea5e/src/hotspot/share/opto/predicates.hpp#L704-L708

We therefore require a third state. Thus, I introduced a new tri-state `PredicateState` that provides a special `MaybeUseful` value which we can set each Predicate to.

#### Ignoring Useless Parse Predicates
While working on this patch, I've noticed that we are always visiting Parse Predicates - even when they useless. We should change that to align it with what we have for the other Predicates (changed in JDK-8351280). To make this work, we also replace the `_useless` state in `ParsePredicateNode` with a new `PredicateState`.

#### Sharing Code for Parse and Template Assertion Predicates
With all the mentioned changes in place, I could nicely share code for the elimination of Parse and Template Assertion Predicates in `EliminateUselessPredicates` by using templates. The following additional changes were required:

- Changing the template parameter of `_template_assertion_predicate_opaques` to the more specific `OpaqueTemplateAssertionPredicateNode` type.
- Adding accessor methods to get the Predicate lists from `Compile`.
- Updating `ParsePredicate::mark_useless()` to pass in `PhaseIterGVN`, as done for Assertion Predicates

Note that we still do not directly replace the useless Predicates but rather mark them useless as initiated by JDK-8351280.

### Other Included Changes
- During the various refactoring steps, I somehow dropped the code to add newly cloned Template Assertion Predicate to the `_template_assertion_predicate_opaques` list. It was done directly in the old cloning methods. This is not relevant for correctness but could hinder some optimizations. I've added the code now in `Node::clone()` to make sure we do not miss any Template Assertion Predicates (similar to what we do for Parse Predicates already):
https://github.com/openjdk/jdk/blob/5e4b6ca0ddafa80eee60690caacd257b74305d4e/src/hotspot/share/opto/node.cpp#L514-L516
- Adding verification code to `TemplateAssertionPredicate::is_predicate()` and `InitializedAssertionPredicate::is_predicate()` to verify that when we find an `Opaque*AssertionPredicate` node that we also find the associated `Halt` node.
- Some small refactorings here and there like renamings.

Thanks,
Christian

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

Commit messages:
 - Typos etc.
 - Revert has_halt() verification
 - cleanup
 - 8350578: Refactor useless Template Assertion Predicate elimination code

Changes: https://git.openjdk.org/jdk/pull/24013/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24013&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8350578
  Stats: 426 lines in 11 files changed: 234 ins; 139 del; 53 mod
  Patch: https://git.openjdk.org/jdk/pull/24013.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24013/head:pull/24013

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


More information about the hotspot-compiler-dev mailing list