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

[Vladimir Kozlov]

On Mon, 13 Nov 2023 07:32:39 GMT, Tobias Hartmann <thartmann 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.
>
> test/hotspot/jtreg/compiler/inlining/TestNullAtCallAfterLateInline.java line 29:
> 
>> 27:  * @summary C2: "Bad graph detected in build_loop_late" with incremental inlining
>> 28:  * @requires vm.compiler2.enabled
>> 29:  * @run main/othervm -XX:+AlwaysIncrementalInline -XX:-BackgroundCompilation TestNullAtCallAfterLateInline
> 
> `AlwaysIncrementalInline` is a debug option.

May be we should make it diagnostic and add it to stress testing flags.
Or, as Tobias pointed, `@requires` for debug VM is needed here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16581#discussion_r1391594320