Integrated: 8342043: Split Opaque4Node into OpaqueTemplateAssertionPredicateNode and OpaqueNotNullNode

[Christian Hagedorn]

On Mon, 21 Oct 2024 12:46:16 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> ### 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 Pre...

This pull request has now been integrated.

Changeset: 7131f053
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/7131f053b0d26b62cbf0d8376ec117d6e8d79f9e
Stats:     237 lines in 18 files changed: 66 ins; 19 del; 152 mod

8342043: Split Opaque4Node into OpaqueTemplateAssertionPredicateNode and OpaqueNotNullNode

Reviewed-by: kvn, roland

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

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