RFR: 8330386: Replace Opaque4Node of Initialized Assertion Predicate with new OpaqueInitializedAssertionPredicateNode [v2]

Christian Hagedorn chagedorn at openjdk.org
Tue May 7 14:44:54 UTC 2024


On Mon, 6 May 2024 14:24:27 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> This patch replaces the `Opaque4Node` of the `If` for Initialized Assertion Predicates with a new `OpaqueInitializedAsseritonPredicateNode`. This helps to simplify pattern matching for predicate code and to distinguish from the two other uses of `Opaque4` nodes:
>> 1. Template Assertion Predicate: The goal is to get rid of its `Opaque4Node` as well by using a dedicated `TemplateAssertionPredicateNode` for the `IfNode`.
>> 2. Non-null-checks with instrinsics and unsafe accesses: This will eventually be the only use left. Once we get there, we should rename the node accordingly to `OpaqueNonNullCheck` or something like that.
>> 
>> I went through all the uses of `Opaque4` nodes and did the following:
>> - Could the `Opaque4` node be part of an Initialized Assertion Predicate?
>>   - No: Added an assert that we are not dealing with an Initialized Assertion Predicate.
>>   - Yes:
>>     - Yes **and only** for Initialized Assertion Predicates? Added an assert that we are only expecting an `OpaqueInitializedAsseritonPredicateNode` if appropriate.
>>     - Yes but could also be something else: Added case for `OpaqueInitializedAsseritonPredicateNode` next to the `Opaque4` case.
>> - Is this `Opaque4` node only used for Template Assertion Predicates?
>>   - Yes: Added assert with call to `assertion_predicate_has_loop_opaque_node()` to check that we find its `OpaqueLoop*Nodes`.
>>  - I've added test cases where I was not sure about whether an `Opaque4` node could be part of a Template, an Initialized Assertion Predicate or a non-null-check. This was a little tricky but I think it was still worth to prevent future bugs (even though most of these special cases are quite rare).
>> 
>> This is another patch split off from the full fix for Assertion Predicates.
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8330386
>  - Add more comments and asserts
>  - Add more tests
>  - 8330386: Replace Opaque4Node of Initialized Assertion Predicate with new OpaqueInitializedAssertionPredicateNode

Thanks Vladimir for your review!

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

PR Comment: https://git.openjdk.org/jdk/pull/18951#issuecomment-2098573489


More information about the hotspot-compiler-dev mailing list