[jdk20] RFR: 8298520: C2: assert(found_opaque == res) failed: wrong pattern

[Roland Westrelin]

On Wed, 14 Dec 2022 09:11:06 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> The assert fires because CountedLoopNode::is_canonical_loop_entry()
>> finds an Opaque1 where it expects an OpaqueZeroTripGuard. That Opaque1
>> guards the CountedLoopEnd of the pre loop: is_canonical_loop_entry()
>> should have stopped at the zero trip guard but walked past it. The
>> reason for that is the call for skip_predicates(): when
>> CountedLoopNode::skip_predicates_from_entry() encounters an If node
>> that has lost a projection, it assumes it's a predicate and moves to
>> the next one. In this case, the zero trip guard only has one
>> projection (the one that connects it through predicates to the loop
>> head), because igvn is in the process of updating a dead part of the
>> graph.
>> 
>> To fix this, I propose that:
>> 
>> 1- CountedLoopNode::is_canonical_loop_entry() fails when after
>> predicates it's at a CountedLoopEnd.
>> 
>> 2- CountedLoopNode::skip_predicates_from_entry() stops at a zero trip
>> guard (identified by a condition that uses a OpaqueZeroTripGuard)
>> 
>> Either 1- or 2- are good enough to fix this particular graph but I
>> propose doing both as making this logic more robust feels safer.
>> 
>> I don't provide a test case as this seems to only happen when igvn
>> processes a dying subgraph in a very specific order.
>
> All tests passed.

@TobiHartmann @chhagedorn thanks for the reviews and testing.

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

PR: https://git.openjdk.org/jdk20/pull/24