RFR: 8298848: C2: clone all of (CmpP (LoadKlass (AddP down at split if

Roland Westrelin roland at openjdk.org
Thu Dec 15 11:04:03 UTC 2022


As suggested by Vladimir in:
https://github.com/openjdk/jdk/pull/11666

Thus extract one for the fixes as a separate PR. The bug as described
in the above PR is:

The crash occurs because a` (If (Bool (CmpP (LoadKlass ..))))`
only has a single projection. It lost the other projection because of
a `CheckCastPP` that becomes `top`. Initially the pattern is, in pseudo
code:


if (obj.klass == some_class) {
  obj = CheckCastPP#1(obj);
}


`obj` itself is a `CheckCastPP` that's pinned at a dominating if. That
dominating if goes through split through phi. The `LoadKlass` for the
pseudo code above also has control set to the dominating if being
transformed. This result in:


if (phi1 == some_class) {
  obj = CheckCastPP#1(phi2);
}


with` phi1 = (Phi (LoadKlass obj) (LoadKlass obj))` and phi2 = (Phi obj obj)
with `obj = (CheckCastPP#2 obj')`

`PhiNode::Ideal()` transforms `phi2` into a new `CheckCastPP`:
`(CheckCastPP#3 obj' obj') `with control set to the region right above
the if in the pseudo code above. There happens to be another
`CheckCastPP` at the same control which casts obj' to a narrower
type. So the new `CheckCastPP#3` is replaced by that one (because of
`ConstraintCastNode::dominating_cast()`) and pseudo code becomes:


if (phi1 == some_class) {
  obj = CheckCastPP#1(CheckCastPP#4(obj'));
}


and then:


if (phi1 == some_class) {
  obj = top;
}


because the types of the 2 `CheckCastPP`s conflict. That would be ok if:

`phi1 == some_class`

would constant fold. It would if the test was:

`if (CheckCastPP#4(obj').klass == some_klass) {
`
but because of split if, the `(CmpP (LoadKlass ..))` and the
`CheckCastPP#1` ended up with 2 different object inputs that then were
transformed differently. The fix I propose is to have split if clone the entire:

`(Bool (CmpP (LoadKlass (AddP ..))))`

down the same way `(Bool (CmpP ..))` is cloned down. After split if, the
pseudo code becomes:


if (phi.klass == some_class) {
  obj = CheckCastPP#1(phi);
}


The bug can't occur because the `CheckCastPP` and` (CmpP (LoadKlass ..))`
operate on the same phi input. The change in split_if.cpp implements
that.

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

Commit messages:
 - test & fix

Changes: https://git.openjdk.org/jdk/pull/11689/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11689&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8298848
  Stats: 556 lines in 3 files changed: 442 ins; 104 del; 10 mod
  Patch: https://git.openjdk.org/jdk/pull/11689.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/11689/head:pull/11689

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


More information about the hotspot-compiler-dev mailing list