RFR: 8271340: Crash PhaseIdealLoop::clone_outer_loop [v3]

[Roland Westrelin]

On Fri, 3 Sep 2021 08:16:04 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> The test has a counted loop and an infinite loop. The infinite loop is
>> reachable from multiple paths and its loop head is a Region with more
>> than 3 inputs. One of these paths is from the counted loop. When loop
>> opts run, a NeverBranch is added to the infinite loop that's removed
>> by NeverBranchNode::Ideal() next because in(0) of the NeverBranch is a
>> Region and not a Loop.
>> 
>> When CCP runs, it finds the counted loop exit is never reached because
>> a test in the loop body that depends on a loop phi is never taken. As
>> a consequence nodes along the path from the counted loop end to the
>> infinite loop Region have type top. One of these nodes is a Call
>> node. PhaseCCP::do_transform() should then cause the path between the
>> counted loop and the infinite loop to optimize out but that doesn't
>> happen because do_transform() starts from Root by following inputs.
>> The dead path is only reachable from the infinite loop but the there's
>> no edge between Root and the infinite loop.
>> 
>> IGVN next runs, processes the Call Node, finds it's dead, kills
>> everything around it which causes the OuterStripMinedLoopEnd to loose
>> a projection. That later triggers the crash.
>> 
>> The fix I propose is to be more conservative in
>> NeverBranchNode::Ideal() and to check for a in(0) that's a Region. As
>> a consequence, at CCP time, the infinite loop is reachable from Root.
>> 
>> This change also requires some adjustments to Shenandoah specific code
>> that makes assumptions about the shape of infinite loops.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   test fix

> Would the following code I added to Valhalla help?
> 
> [openjdk/valhalla at f43fafc#diff-f4a11a2c3f7d7342641c878277f778856ad329e4e09026e39d08afb03efd10a2R1958](https://github.com/openjdk/valhalla/commit/f43fafc7d81f3d34f6e971765dbadae41ae5c393#diff-f4a11a2c3f7d7342641c878277f778856ad329e4e09026e39d08afb03efd10a2R1958)

I'm not sure. How would it help in this particular case?

In any case, it seems to me NeverBranchNode::Ideal() is simply wrong. As I understand, when loop opts hit an infinite loop it creates a NeverBranchNode but because the infinite loop is not properly registered in the loop tree, the loop's Region is not transformed into a Loop node. On the next pass of loop opts, if the NeverBranch is still in the infinite loop, a Loop is created for the infinite loop. But if NeverBranchNode::Ideal() runs in between then a new NeverBranch is added and maybe and only in some later loop opts pass a Loop created. So AFAICT, the behavior is inconsistent depending on whether NeverBranchNode::Ideal() runs or not which doesn't seem right.

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

PR: https://git.openjdk.java.net/jdk/pull/5220