RFR: 8293798: Fix test bugs due to incompatibility with -XX:+AlwaysIncrementalInline

Emanuel Peter epeter at openjdk.org
Mon Sep 19 06:42:20 UTC 2022


I investigated and fixed the following three files, which failed (test asserts) if they were run with `-XX:+AlwaysIncrementalInline`.

Manually tested the 3 test-files.
I also ran some other sanity tests.

Looking forward to your feedback,
Emanuel

**compiler/uncommontrap/Decompile.java**
Assert triggered:
`java.lang.RuntimeException: Wrong compilation status.: expected false to equal true`
We expected the test to deoptimize after introducing the third class Y to a method that is expected to be call with bimorphic inlining.

Analysis:
It seems that we don't to bimorphic inlining of function calls (2 static inlinings, uncommon_trap for others).
https://github.com/openjdk/jdk/blob/aa7ccdf44549a52cce9e99f6569097d3343d9ee4/src/hotspot/share/opto/doCall.cpp#L269-L275
Instead, we create two `LateInlineCallGenerator`, one for a class (hit), and one for the other cases (miss).
https://github.com/openjdk/jdk/blob/aa7ccdf44549a52cce9e99f6569097d3343d9ee4/src/hotspot/share/opto/doCall.cpp#L200-L201
`LateInlineCallGenerator` has `is_inline` false, so we do not go into the bimorphic inlining case (would require both receivers to have is_inline true).
https://github.com/openjdk/jdk/blob/aa7ccdf44549a52cce9e99f6569097d3343d9ee4/src/hotspot/share/opto/doCall.cpp#L255-L259
Instead, we generate a static call for one class (hit), and a virtual call for the rest (miss).
https://github.com/openjdk/jdk/blob/aa7ccdf44549a52cce9e99f6569097d3343d9ee4/src/hotspot/share/opto/doCall.cpp#L279-L280
Therefore, when the third class comes along the function does not trap, and not deopt either.

Solution:
For now, we will just deactivate the flag in the test. But more discussion is required if we want this flag to change the inline behavior in this way.

**compiler/intrinsics/klass/CastNullCheckDroppingsTest.java**
Assert triggered:
`java.lang.AssertionError: compilation must not get deoptimized`
We expect the function `testMHCast(String s)` to be compiled without a `null_check` for input `s`. We test that by calling it with `Null`. We trigger the assert because of a `null_check`, leading to deoptimization.

Analysis:
We are in `GraphKit::gen_checkcast`. We have a `cast` that is wrapped in a `MethodHandle`, which we want to call via `invokeExact`. Normally, this would get directly inlined, and the output is deduced to be `String*`. With the flag, however, we do not inline `invokeExact`, but create a `JavaStaticCall`, which represents `invokeExact`. This has 4 `Object*` inputs and 1 `Object*` output. So now the result of the cast is a `Object*`.
We have a `String*` in the normal case, and `Object*` in the flag case. We now do a local cast to `String*`. For that we first check subtyping.

https://github.com/openjdk/jdk/blob/aa7ccdf44549a52cce9e99f6569097d3343d9ee4/src/hotspot/share/opto/graphKit.cpp#L3311
    In the normal case, we have a perfect match - so we rely on profiling and do speculative `String::NonNull*`. But in the flag case, we see `Object*` is not subtype of `String*`. So we continue on, and find that profiling has determined `never_see_null = true`, so we set an `uncommon_trap` for the `null_check`.

Somehow, the speculative profiling case does not lead to a trap - not sure why yet.
Of course in the test we eventually do feed in `Null` - in the regular case there is no trap, in the flag case we trap and deopt, breaking the test assumption.

Solution:
At any rate, this really looks like a test bug - the flag can change the decisions that have an impact on the asserts of the test. I will disable the flag.

**compiler/ciReplay/TestInliningProtectionDomain.java**
Assert triggered:
`java.lang.RuntimeException: assertTrue: expected true, was false`
https://github.com/openjdk/jdk/blob/aa7ccdf44549a52cce9e99f6569097d3343d9ee4/test/hotspot/jtreg/compiler/ciReplay/TestInliningProtectionDomain.java#L73

Analysis:
Turns out `isForcedByReplay()` returns false because the "reason" is expected to be `force inline by ciReplay`, but we get a reason `force (incremental) inline by ciReplay`. This seems expected. We should use `isForcedIncrementalInlineByReplay()` if we expect incremental inline to happen.

Solution:
`isForcedByReplay()` or `isForcedIncrementalInlineByReplay()`

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

Commit messages:
 - 8293798: Fix test bugs due to incompatibility with -XX:+AlwaysIncrementalInline

Changes: https://git.openjdk.org/jdk/pull/10310/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=10310&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8293798
  Stats: 3 lines in 3 files changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/10310.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10310/head:pull/10310

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


More information about the hotspot-compiler-dev mailing list