Integrated: 8286800: Assert in PhaseIdealLoop::dump_real_LCA is too strong

[Christian Hagedorn]

On Mon, 7 Nov 2022 11:21:36 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> We sometimes hit the following assert when dumping a bad graph (before crashing with the bad graph assertion):
> 
> assert(real_LCA != NULL, "must always find an LCA"
> ``` 
> The algorithm is not correct as we should always find an LCA of two nodes. To fix this, I've re-implemented the algorithm and improved the dumped idom chains:
> - I limited the node dump to idx + node name to reduce the noise which made it hard to read.
> - Reversed the idom chain dumps to reflect the graph structure.
> 
> Example output:
> 
> Bad graph detected in build_loop_late
> n:  138  CastPP  === 205 38  [[ 263 140 140 168 ]]  #Test:NotNull *  Oop:Test:NotNull * !jvms: Test::mainTest @ bci:40 (line 154)
> 
> [... same output as before ...]
> 
> idoms of early "197 IfFalse":
> idom[2]:    42  If
> idom[1]:    44  IfTrue
> idom[0]:   196  If
> n:         197  IfFalse
> 
> idoms of (wrong) LCA "205 IfTrue":
> idom[4]:    42  If
> idom[3]:    37  Region
> idom[2]:    73  If
> idom[1]:    83  IfTrue
> idom[0]:   204  If
> n:         205  IfTrue
> 
> Real LCA of early "197 IfFalse" (idom[2]) and wrong LCA "205 IfTrue" (idom[4]):
>   42  If  === 30 41  [[ 43 44 ]] P=0.999000, C=-1.000000 !jvms: Test::mainTest @ bci:32 (line 153)
> 
> Tested by manually calling `dump_idoms` during a compilation and by running reproducers of different bad graph assertion bugs.
> 
> Thanks,
> Christian

This pull request has now been integrated.

Changeset: decb1b79
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/decb1b79bc475f024a02135fa3394ff97098e758
Stats:     145 lines in 2 files changed: 70 ins; 40 del; 35 mod

8286800: Assert in PhaseIdealLoop::dump_real_LCA is too strong

Reviewed-by: kvn, rcastanedalo

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

PR: https://git.openjdk.org/jdk/pull/11015