RFR: 8355674: C2: Partial Peeling should not introduce Phi nodes above OpaqueInitializedAssertionPredicate nodes

[Emanuel Peter]

On Fri, 2 May 2025 13:44:10 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> In the test case, we have two Initialized Assertion Predicate that share the same `Bool` which is perfectly fine:
> 
> ![image](https://github.com/user-attachments/assets/a7a1673f-b5df-49e8-8a22-c35aa0ee1693)
> 
> These Initialized Assertion Predicates were created for loops that have been folded away. They then end up in a new inner most loop which is partial peeled. Partial Peeling finds that we need to do the cut between `580 IfTrue` and `581 If`. This means, that the Initialized Assertion Predicate `569 RangeCheck` with its `535 OpaqueInitializedAssertionPredicate` is in the peel set and the second Initialized Assertion Predicate `504 RangeCheck` with its `503 OpaqueInitializedAssertionPredicate` is in the not peel set. As a result of that, we are introducing a `Phi` node between an `OpaqueInitializedAssertionPredicate` and a `Bool` node:
> 
> ![image](https://github.com/user-attachments/assets/3bbc2b88-300a-4c40-99ac-056cbeab822a)
> 
> We eventually remove the `OpaqueInitializedAssertionPredicate` and are left with the following graph shape
> 
> ![image](https://github.com/user-attachments/assets/e839366f-119b-4411-b422-84639c68aa80)
> 
> which cannot be handled by the backend.
> 
> The fix I propose is to prohibit Partial Peeling from inserting such a `Phi` node by updating `clone_for_special_use_inside_loop()` which takes care of not inserting phis for an `If/Bool`. We need to also special case `OpaqueInitializedAssertionPredicate`.
> 
> Thanks,
> Christian

Thanks for fixing this, looks reasonable :)

test/hotspot/jtreg/compiler/predicates/assertion/TestPhiAboveOpaqueInitializedAssertionPredicate.java line 29:

> 27:  * @summary Check that we do not introduce a Phi above a OpaqueInitializedAssertionPredicateNode during Partial Peeling.
> 28:  * @run main/othervm -Xcomp -XX:CompileOnly=compiler.predicates.assertion.TestPhiAboveOpaqueInitializedAssertionPredicate::test
> 29:  *                   compiler.predicates.assertion.TestPhiAboveOpaqueInitializedAssertionPredicate

What about a run without `Xcomp`?

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

Marked as reviewed by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25006#pullrequestreview-2819013567
PR Review Comment: https://git.openjdk.org/jdk/pull/25006#discussion_r2075900903