RFR: 8342043: Split Opaque4Node into OpaqueTemplateAssertionPredicateNode and OpaqueNotNullNode
Christian Hagedorn
chagedorn at openjdk.org
Mon Oct 21 13:45:11 UTC 2024
### Two Uses of `Opaque4`
The `Opaque4` node is currently used for two things:
1. Non-null checks of intrinsics to accompany a cast to non-null where we implicitly know that an object is non-null but C2 does not. This is required to fold control away if the cast becomes top at some point.
2. Template Assertion Predicates
### How to Differentiate between Uses
The only reliable way to differentiate between the two uses is to check if there are `OpaqueLoop*Nodes` above the `Opaque4` node which should only be found for a Template Assertion Predicate. This can be done with the method `assertion_predicate_has_loop_opaque_node()` which follows the inputs of the `Opaque4` recursively.
### Problems by Sharing `Opaque4` Nodes for Two Concepts
This sharing of the `Opaque4` comes with some problems:
- One need to be careful when checking for `Opaque4` nodes if the code can be applied for non-null checks and/or Template Assertion Predicates. Ideally, we should add assertions to exclude the unexpected case (mostly done).
- Walking the graph with `assertion_predicate_has_loop_opaque_node()` has some overhead, especially when checking the negation, we could walk quite some nodes in the worst case.
- There is no real benefit of (re)using `Opaque4` nodes for two concepts and could easily be separated.
### Split `Opaque4` into Two Classes to Separate Uses
Therefore, this patch proposes to split `Opaque4` into a new `OpaqueNonNull` and a `OpaqueTemplateAssertionPredicate` class (the latter accompanies the already split off `OpaqueInitializedAssertionPredicate` class). I went through all the uses and updated them accordingly.
As a consequence, I could turn `assertion_predicate_has_loop_opaque_node()` into a debug only method since it's now only used in assertion code. I additionally removed the second input of the `Opaque4` nodes since these nodes have always eventually been replaced by true.
### Turning `OpaqueTemplateAssertionPredicate` into a Non-Macro Node
The `Opaque4` was a macro node. The `OpaqueNotNull` still is but the `OpaqueTemplateAssertionPredicate` is turned into a non-macro node and is removed after loop opts. At that point, loops can no longer be split and we do no need to create Initialized Assertion Predicates from the templates anymore. Thus, they can be cleaned up in the post loop opts IGVN.
### Pre-Requisite for Replacing Uncommon Traps with Halt Nodes for Template Assertion Predicates
Eventually, I want to get rid of UCTs for Template Assertion Predicates completely. They have been used out of convenience to reuse code in Loop Predication. But there is a problem when copying them to loops where we do not have traps (for example, for the remaining loop when peeling one iteration off or when copying them to the main loop). We cannot use UCTs anymore and need to fall back to `Halt` nodes.
Supporting the UCT and `Halt` node format for Template Assertion Predicates is difficult and does not really give us a benefit. Therefore, I want to get rid of UCTs and only use `Halt` nodes with [JDK-8342047](https://bugs.openjdk.org/browse/JDK-8342047). This patch lays the foundation to enable this change to still easily detect a Template Assertion Predicate.
Thanks,
Christian
-------------
Commit messages:
- Fix whitespaces
- clean-up
- 8342043: Split Opaque4Node into OpaqueTemplateAssertionPredicateNode and OpaqueNotNullNode
Changes: https://git.openjdk.org/jdk/pull/21608/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21608&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8342043
Stats: 237 lines in 18 files changed: 66 ins; 19 del; 152 mod
Patch: https://git.openjdk.org/jdk/pull/21608.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/21608/head:pull/21608
PR: https://git.openjdk.org/jdk/pull/21608
More information about the hotspot-compiler-dev
mailing list