RFR: 8297933: [REDO] Compiler should only use verified interface types for optimization

Roland Westrelin roland at openjdk.org
Wed Dec 14 10:03:06 UTC 2022


This PR re-does 6312651 (Compiler should only use verified interface
types for optimization) with a couple fixes I had pushed afterward
(8297556 and 8297343) and fixes for some other issues.

The trickiest one is a fix for 8297345 (C2: SIGSEGV in
PhaseIdealLoop::push_pinned_nodes_thru_region) for which I added a
test case. 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 CheckCastPPs 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.

The other fixes are:

- arraycopynode.cpp: a crash happens because dest_offset and
  src_offset are the same. The call to transform that results in
  src_scale, causes src_offset (and thus dest_offset) to become
  dead. The fix is to add a hook node to preserve dest_offset. This is
  unrelated to 6312651 but it triggers with that change for some
  reason.
  
- castnode.cpp: I removed CheckCastPPNode::Identity(), a piece of code
  that the change in the handling of interfaces make obsolete and that
  I missed in the PR for 6312651.
  
- castnode.cpp: the change in CheckCastPPNode::Value() fixes a rare
  assert when during CCP, Value() is called with an input raw constant
  ptr.
  
- type.cpp: a _klass = NULL field in arrays used to indicate only top
  or bottom but I changed that so _klass is only guaranteed non null
  for basic type arrays. The fix in type.cpp updates a piece of code
  that I didn't adapt to the new meaning of _klass = NULL.
  
- the other changes are due to StressReflectiveCode. With 6312651, a
  CheckCastPP can fold to top if it sees a type for its input that
  conflicts with its own type. That wasn't the case before. So if a
  type check fails, a CheckCastPP will fold to top and the control
  flow branch it's in must die. That doesn't always happen with
  StressReflectiveCode: the CheckCastPP folds but not the control flow
  path. With ExpandSubTypeCheckAtParseTime on, that's because of a
  code path in LoadNode::Value() that's disabled with
  StressReflectiveCode. With ExpandSubTypeCheckAtParseTime off, it's
  because Compile::static_subtype_check() is always pessimistic with
  StressReflectiveCode but it's used by SubTypeCheckNode::sub() to
  find when a node can constant fold.

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

Commit messages:
 - more fixes
 - Revert "8297934: [BACKOUT] Compiler should only use verified interface types for optimization"

Changes: https://git.openjdk.org/jdk/pull/11666/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11666&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8297933
  Stats: 2245 lines in 29 files changed: 1252 ins; 623 del; 370 mod
  Patch: https://git.openjdk.org/jdk/pull/11666.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/11666/head:pull/11666

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


More information about the hotspot-compiler-dev mailing list