RFR: 8307927: C2: "malformed control flow" with irreducible loop

[Emanuel Peter]

On Fri, 23 Jun 2023 08:38:09 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> The test contains a loop nest with 2 loops. The outer loop is an
>> irreducible loop. The safepoint for that loop is also in the inner
>> loop. Because `IdealLoopTree::check_safepts()` ignores irreducible
>> loops, that safepoint is not marked as being required and is
>> eliminated from the inner loop. The inner loop is then optimized out
>> and the outer loop becomes an infinite loop with no safepoint (a
>> single node loop). That, in turn, causes the loop to be eliminated
>> because it has not use and the assert fires.
>> 
>> The fix I propose is to make `IdealLoopTree::check_safepts()` work
>> with irreducible loops. I think
>> `IdealLoopTree::allpaths_check_safepts()` can be used for that. When
>> working on this I wondered if that method could be called with a loop
>> whose head has more than 3 inputs. I couldn't write a test case with
>> an irreducible loop whose head had more than 3 inputs but I added an
>> assert in the method and ran some testing. That assert fired so I also
>> propose to tweak the method so it's robust in that case.
>
> src/hotspot/share/opto/loopnode.cpp line 3515:
> 
>> 3513: // Allpaths backwards scan from loop tail, terminating each path at first safepoint
>> 3514: // encountered.  Helper for check_safepts.
>> 3515: void IdealLoopTree::allpaths_check_safepts(VectorSet &visited, Node_List &stack) {
> 
> @rwestrel you should update the description here. Suggestion:
> 
> Allpaths backwards scan. Starting at the head, traversing all backedges, and the body. Terminating each path at first safepoint encountered.  Helper for check_safepts.

Also the line above is not accurate enough anymore:
`_required_safept->push(n);  // save the one closest to the tail`

For one: could there not be multiple such SafePoints? If so: what does it mean to take "the closest"? And we may have multiple backedges / tails, now that we allow irreducible loops.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14522#discussion_r1239540658