RFR: 8318826: C2: "Bad graph detected in build_loop_late" with incremental inlining [v4]

[Christian Hagedorn]

On Wed, 15 Nov 2023 16:41:50 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> In test case, `test1()`, `inlined1()` returns null which is used as
>> receiver for the `notInlined()` virtual call. On the fallthrough
>> projection of the call, the receiver is casted to not null. In the
>> case of `test1()` the fallthrough should be unreachable:
>> `CatchNode::Value()` makes that path dead if it is a projection of a
>> virtual call with a null receiver. When `test1()` is compiled with
>> incremental inlining, before `inlined1()` is inlined, the receiver of
>> the virtual call is not known to be null so the fallthrough path is
>> not killed. When it is inlined, the first input argument to the
>> virtual call node is updated to be null. So that node is pushed on the
>> igvn work list. But the `CatchNode` that's not change in the the
>> process is not. As a result, the fallthrough path of the virtual call
>> is not killed. The input to the cast node in the fallthrough path is
>> changed to null which causes that node to become top. That causes the
>> crash.
>> 
>> IGVN has logic to enqueue nodes that are related to an edge change but
>> are not directly modified. We would need similar logic to be called
>> from late inlining code. This change refactors the igvn logic so it
>> can be called from elsewhere.
>> 
>> `test2()` is similar to `test1()` except the return of the inlined
>> method `inlined2()` is not set to null. Instead, a replaced node is
>> recorded from `a` to null (because class `D` is unrelated to `A` so
>> casting a `A` to `D` successfully only happens for null) and applied
>> when `inlined2()` is late inlined.
>
> Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
> 
>  - merge/review fix
>  - Merge branch 'master' into JDK-8318826
>  - Update src/hotspot/share/opto/replacednodes.cpp
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>  - Update src/hotspot/share/opto/phaseX.hpp
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>  - -XX:+IgnoreUnrecognizedVMOptions
>  - whitespaces
>  - fix & test

Still good, thanks for the update.

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16581#pullrequestreview-1733641177