Integrated: 8268019: C2: assert(no_dead_loop) failed: dead loop detected

[Christian Hagedorn]

On Wed, 21 Jul 2021 14:47:08 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> In the testcase, a path containing a loop is found to be dead. These nodes are then removed during IGVN after parsing (before loop opts). One of these nodes is a loop header region through which an `If` is split. Splitting an `If` through a loop header region is not intended and was disabled by [JDK-8232539](https://bugs.openjdk.java.net/browse/JDK-8232539). However, the bailouts in the current code are not enough to prevent the split if optimization for this dying loop header node due to the following reasons:
> - The region is not a `LoopNode`, yet, and we do not bail out at:
> https://github.com/openjdk/jdk/blob/cd8783c08ee18167f15df621e997015b971bfb01/src/hotspot/share/opto/ifnode.cpp#L119
> - The loop header region has only one phi left (the other phis were already killed) and we do not bail out at:
> https://github.com/openjdk/jdk/blob/cd8783c08ee18167f15df621e997015b971bfb01/src/hotspot/share/opto/ifnode.cpp#L143-L148
> - The phi merges a `ConP` constant (dead path) and a `CastPP` node which is also allowed as constant:
> https://github.com/openjdk/jdk/blob/cd8783c08ee18167f15df621e997015b971bfb01/src/hotspot/share/opto/ifnode.cpp#L98-L101
> - The loop entry was already replaced by top and thus no predicates can be found to bail out:
> https://github.com/openjdk/jdk/blob/cd8783c08ee18167f15df621e997015b971bfb01/src/hotspot/share/opto/ifnode.cpp#L244-L247
> 
> These conditions let the split if optimization to be applied without a bailout. The `CastPP` node is a use of the phi and is thus rewired in the process. The problem now is that the `CastPP` node is an input *and* output of the loop phi while being a constant to be merged:
> ![before_split_if](https://user-images.githubusercontent.com/17833009/126507548-62583105-b746-4551-8037-aba1b15b1d5a.png)
> 
> This lets the split if optimization to create a data loop by setting the input of the `CastPP` to itself.
> 
> This scenario is rare and highly depends on the order in which IGVN processes the nodes. As a fix, I propose to extend the bailout checks for loop header regions to catch this edge case. The new check looks at all inputs of the region and bail out if only one path is non-top. This also avoids unnecessary splits through regions which are dying anyways.
> 
> Thanks,
> Christian

This pull request has now been integrated.

Changeset: 489e5fd1
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/489e5fd12a37a45f4f5ea64b05f85c6f99f70811
Stats:     83 lines in 2 files changed: 78 ins; 0 del; 5 mod

8268019: C2: assert(no_dead_loop) failed: dead loop detected

Reviewed-by: kvn, thartmann

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

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