RFR: 8328480: C2: SubTypeCheckNode in checkcast should use the klass constant of a unique concrete sub class [v3]

Christian Hagedorn chagedorn at openjdk.org
Fri Apr 12 10:05:15 UTC 2024


> While working on a [Valhalla bug](https://bugs.openjdk.org/browse/JDK-8321734), I've noticed that a `SubTypeCheckNode` for a `checkcast` does not take a unique concrete sub class `X` of an abstract class `A` as klass constant in the sub type check. Instead, it uses the abstract klass constant:
> 
> 
> abstract class A {}
> class X extends A {}
> 
> A x = (A)object; // Emits SubTypeCheckNode(object, A), but could have used X instead of A.
> 
> However, the `CheckCastPP` result already uses the improved instance type ptr `X` (i.e. `toop` which was improved from `A` by calling `try_improve()` to get the unique concrete sub class):
> https://github.com/openjdk/jdk/blob/614db2ea9e10346475eef34629eab54878aa482d/src/hotspot/share/opto/graphKit.cpp#L3257-L3261
> https://github.com/openjdk/jdk/blob/614db2ea9e10346475eef34629eab54878aa482d/src/hotspot/share/opto/graphKit.cpp#L3363
> 
> We should also plug in a unique concrete sub class constant in the `SubTypeCheckNode` which could be beneficial to fold away redundant sub type checks (see test cases).
> 
> This fix is required to completely fix the bug in Valhalla (this is only one of the broken cases). In Valhalla, the graph ends up being broken because a `CheckCastPP` node is folded because of an impossible type but the `SubTypeCheckNode` is not due to not using the improved unique concrete sub class constant for the `checkcast`. I don't think that there is currently a bug in mainline because of this limitation - it just blocks some optimizations. I'm therefore upstreaming this fix to mainline since it can be beneficial to have this fix here as well (see test cases).
> 
> Thanks,
> Christian

Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:

  Revert "using improved type for non-constants" + add comment

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/18515/files
  - new: https://git.openjdk.org/jdk/pull/18515/files/660c0dec..d7719279

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

  Stats: 6 lines in 1 file changed: 0 ins; 4 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/18515.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18515/head:pull/18515

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


More information about the hotspot-compiler-dev mailing list