RFR: 8316533: C2 compilation fails with assert(verify(phase)) failed: missing Value() optimization [v3]

Emanuel Peter epeter at openjdk.org
Thu Nov 2 16:14:45 UTC 2023


> **Problem**
> We have a `abstract` class `A` with no subtype. Hence, a reference of type `A` must always be `null` (unless a subclass were to be loaded, which we guard against with a compile dependency).
> 
> But there are at least these two ways a `A:NotNull` can be created:
> - Null-Check: CastPP after null-check improves type from `A` to `A:NotNull`.
> - Forced compilation (eg CTW) of a member method of `A`. Then `Parm0` has type `A`, which is improved to `A:NotNull` because the `this/self` pointer cannot be `null`.
> 
> This means we are now left with an impossible type `A:NotNull`, a path that uses this type will never be executed.
> 
> The question is now what should happen at a `SubTypeCheck` if we do:
> `SubTypeCheck(  oop #A:NotNull   ,   constant-classptr-A-exact )`
> 
> The verification happens because we do these two different things:
> - `SubTypeCheck`: we first detect that we have a constant classptr of a class `A`, which is abstract and has no subtype. Hence, we conclude that any oop compared to it cannot be a subtype (there are no subtypes), and it cannot be of the same type (class is abstract). Hence, any oop must be a supertype (TypeInt::CC_GT).
> - The verification code computes the subtype check by computing the klass of the oop via `LoadKlass` (this constant folds to `constant-classptr-A-exact`, because the type of the oop is `A:NotNull`). The `CmpP` node compares the two klasses, and sees that they are identical, and returns an `TypeInt::CC_EQ`.
> 
> **Alternatives**
> 
> Both results are reasonable, but they are in fact both supersets of the true result. We should take the intersection of the two and get `Type:TOP`, since the input type is already impossible. In fact, it would be best if the impossible type was never created. We could do that by improving `CmpP` to detect the impossible type and constant fold towards the `null` path, removing the `A:NotNull` path. It is harder to deal with the forced-compilation of non-static methods of an abstract class with no subclasses - here we would basically have to forbid compilation or replace the compilation with a `Halt`.
> 
> **Solution**
> 
> Instead, I have now decided to change the logic in `SubTypeCheckNode` to return `EQ` in case the oop has the same klass and is `NotNull`.
> 
> **Testing**
> 
> Tier1-6 and stress testing. Running...

Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:

 - Merge branch 'master' into JDK-8316533
 - Merge branch 'JDK-8316533' of https://github.com/eme64/jdk into JDK-8316533
 - Apply suggestions from code review
   
   From Roberto
   
   Co-authored-by: Roberto Castañeda Lozano <robcasloz at users.noreply.github.com>
 - fix type on class name, spotted by Roberto
 - Merge branch 'master' into JDK-8316533
 - remove CmpP and more comments
 - return EQ instead of TOP
 - CmpP null-check solution
 - add ignore flag to test
 - more comment
 - ... and 3 more: https://git.openjdk.org/jdk/compare/b065c5f4...0ac7d02f

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/16361/files
  - new: https://git.openjdk.org/jdk/pull/16361/files/53f5e742..0ac7d02f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16361&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16361&range=01-02

  Stats: 3420 lines in 111 files changed: 1595 ins; 1262 del; 563 mod
  Patch: https://git.openjdk.org/jdk/pull/16361.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16361/head:pull/16361

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


More information about the hotspot-compiler-dev mailing list