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

[Christian Hagedorn]

On Fri, 5 Nov 2021 13:00:00 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> In the test case, we apply the following optimization in `PhiNode::Ideal()` for the memory phi 989 that is on a dead path but still has both its inputs set to non-top nodes:
> https://github.com/openjdk/jdk/blob/3c0faa73522bd004b66cb9e477f43e15a29842e6/src/hotspot/share/opto/cfgnode.cpp#L2269-L2270
> ![Screenshot from 2021-11-05 11-57-49](https://user-images.githubusercontent.com/17833009/140502849-9f00fd62-9714-4f54-8f98-f22f74d11430.png)
> 
> In this process, we create `11853 Phi` for the new `11850 MergeMem` which is going to replace `989 Phi` (`this`). We then transform `11853 Phi` before returning:
> https://github.com/openjdk/jdk/blob/3c0faa73522bd004b66cb9e477f43e15a29842e6/src/hotspot/share/opto/cfgnode.cpp#L2314
> 
> During `Ideal()` for `11853 Phi`, we transform `11769 MergeMem` into top (because the base memory is top) and use this as new input instead:
> https://github.com/openjdk/jdk/blob/3c0faa73522bd004b66cb9e477f43e15a29842e6/src/hotspot/share/opto/cfgnode.cpp#L2230-L2240
> 
> But even if the `MergeMem` node would not be transformed into top, the slice itself could be top (L2237) and we would still replace the phi input with top. This replacement by top will fold the `11853 Phi` and we will build a cycle `11850 MergeMem` <-> `1064 StoreB` because `989 Phi` will be replaced by `11850 MergeMem`. This results in the assertion failure.
> 
> I tried some approaches by marking `11853 Phi` and/or `989 Phi` to specially treat them during the optimizations in `Ideal()` (e.g. skipping `989 Phi` during the dead loop detection etc.) or to improve the dead loop detection before applying the `MergeMem` optimization in `Ideal()`. But that seemed rather complicated/fragile.
> 
> I therefore propose to simply not transform the newly created phi nodes directly but wait instead for IGVN to revisit them again. This allows the `this` phi to be replaced with the new `MergeMem` node and the dead loop detection will work correctly when processing the new phis again later in IGVN.
> 
> I could only reproduce this bug with the replay file for the attached test case in the JBS issue. The test case itself did not trigger with repeated runs with `StressIGVN` + `RepeatCompilation`.
> 
> Thanks,
> Christian

This pull request has now been integrated.

Changeset: 70d5dffb
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/70d5dffb4e7110902b59b56efaef31614916148c
Stats:     16 lines in 1 file changed: 8 ins; 3 del; 5 mod

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

Reviewed-by: kvn, thartmann

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

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