8354282: C2: more crashes in compiled code because of dependency on removed range check CastIIs

Reviewed-by: chagedorn
Backport-of: 00068a8

Author: rwestrel

src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp

@@ -1394,7 +1394,7 @@ void ShenandoahBarrierC2Support::pin_and_expand(PhaseIdealLoop* phase) {
     }
     if (addr->Opcode() == Op_AddP) {
       Node* orig_base = addr->in(AddPNode::Base);
-      Node* base = new CheckCastPPNode(ctrl, orig_base, orig_base->bottom_type(), ConstraintCastNode::StrongDependency);
+      Node* base = new CheckCastPPNode(ctrl, orig_base, orig_base->bottom_type(), ConstraintCastNode::DependencyType::NonFloatingNarrowing);
       phase->register_new_node(base, ctrl);
       if (addr->in(AddPNode::Base) == addr->in((AddPNode::Address))) {
         // Field access

src/hotspot/share/opto/castnode.cpp

@@ -22,7 +22,6 @@
  *
  */
 
-#include "castnode.hpp"
 #include "opto/addnode.hpp"
 #include "opto/callnode.hpp"
 #include "opto/castnode.hpp"
@@ -35,12 +34,22 @@
 #include "opto/type.hpp"
 #include "utilities/checkedCast.hpp"
 
+const ConstraintCastNode::DependencyType ConstraintCastNode::DependencyType::FloatingNarrowing(true, true, "floating narrowing dependency"); // not pinned, narrows type
+const ConstraintCastNode::DependencyType ConstraintCastNode::DependencyType::FloatingNonNarrowing(true, false, "floating non-narrowing dependency"); // not pinned, doesn't narrow type
+const ConstraintCastNode::DependencyType ConstraintCastNode::DependencyType::NonFloatingNarrowing(false, true, "non-floating narrowing dependency"); // pinned, narrows type
+const ConstraintCastNode::DependencyType ConstraintCastNode::DependencyType::NonFloatingNonNarrowing(false, false, "non-floating non-narrowing dependency"); // pinned, doesn't narrow type
+
 //=============================================================================
 // If input is already higher or equal to cast type, then this is an identity.
 Node* ConstraintCastNode::Identity(PhaseGVN* phase) {
-  if (_dependency == UnconditionalDependency) {
+  if (!_dependency.narrows_type()) {
+    // If this cast doesn't carry a type dependency (i.e. not used for type narrowing), we cannot optimize it.
     return this;
   }
+
+  // This cast node carries a type dependency. We can remove it if:
+  // - Its input has a narrower type
+  // - There's a dominating cast with same input but narrower type
   Node* dom = dominating_cast(phase, phase);
   if (dom != nullptr) {
     return dom;
@@ -109,15 +118,15 @@ Node* ConstraintCastNode::Ideal(PhaseGVN* phase, bool can_reshape) {
 }
 
 uint ConstraintCastNode::hash() const {
-  return TypeNode::hash() + (int)_dependency + (_extra_types != nullptr ? _extra_types->hash() : 0);
+  return TypeNode::hash() + _dependency.hash() + (_extra_types != nullptr ? _extra_types->hash() : 0);
 }
 
 bool ConstraintCastNode::cmp(const Node &n) const {
   if (!TypeNode::cmp(n)) {
     return false;
   }
   ConstraintCastNode& cast = (ConstraintCastNode&) n;
-  if (cast._dependency != _dependency) {
+  if (!cast._dependency.cmp(_dependency)) {
     return false;
   }
   if (_extra_types == nullptr || cast._extra_types == nullptr) {
@@ -130,7 +139,7 @@ uint ConstraintCastNode::size_of() const {
   return sizeof(*this);
 }
 
-Node* ConstraintCastNode::make_cast_for_basic_type(Node* c, Node* n, const Type* t, DependencyType dependency, BasicType bt) {
+Node* ConstraintCastNode::make_cast_for_basic_type(Node* c, Node* n, const Type* t, const DependencyType& dependency, BasicType bt) {
   switch(bt) {
   case T_INT:
     return new CastIINode(c, n, t, dependency);
@@ -143,9 +152,9 @@ Node* ConstraintCastNode::make_cast_for_basic_type(Node* c, Node* n, const Type*
 }
 
 TypeNode* ConstraintCastNode::dominating_cast(PhaseGVN* gvn, PhaseTransform* pt) const {
-  if (_dependency == UnconditionalDependency) {
-    return nullptr;
-  }
+  // See discussion at definition of ConstraintCastNode::DependencyType: replacing this cast with a dominating one is
+  // not safe if _dependency.narrows_type() is not true.
+  assert(_dependency.narrows_type(), "cast can't be replaced by dominating one");
   Node* val = in(1);
   Node* ctl = in(0);
   int opc = Opcode();
@@ -205,30 +214,21 @@ void ConstraintCastNode::dump_spec(outputStream *st) const {
     st->print(" extra types: ");
     _extra_types->dump_on(st);
   }
-  if (_dependency != RegularDependency) {
-    st->print(" %s dependency", _dependency == StrongDependency ? "strong" : "unconditional");
-  }
+  st->print(" ");
+  _dependency.dump_on(st);
 }
 #endif
 
-const Type* CastIINode::Value(PhaseGVN* phase) const {
-  const Type *res = ConstraintCastNode::Value(phase);
-  if (res == Type::TOP) {
-    return Type::TOP;
-  }
-  assert(res->isa_int(), "res must be int");
-
-  // Similar to ConvI2LNode::Value() for the same reasons
-  // see if we can remove type assertion after loop opts
-  res = widen_type(phase, res, T_INT);
+CastIINode* CastIINode::make_with(Node* parent, const TypeInteger* type, const DependencyType& dependency) const {
+  return new CastIINode(in(0), parent, type, dependency, _range_check_dependency, _extra_types);
+}
 
-  return res;
+CastLLNode* CastLLNode::make_with(Node* parent, const TypeInteger* type, const DependencyType& dependency) const {
+  return new CastLLNode(in(0), parent, type, dependency, _extra_types);
 }
 
-Node* ConstraintCastNode::find_or_make_integer_cast(PhaseIterGVN* igvn, Node* parent, const TypeInteger* type) const {
-  Node* n = clone();
-  n->set_req(1, parent);
-  n->as_ConstraintCast()->set_type(type);
+Node* ConstraintCastNode::find_or_make_integer_cast(PhaseIterGVN* igvn, Node* parent, const TypeInteger* type, const DependencyType& dependency) const {
+  Node* n = make_with(parent, type, dependency);
   Node* existing = igvn->hash_find_insert(n);
   if (existing != nullptr) {
     n->destruct(igvn);
@@ -242,14 +242,13 @@ Node *CastIINode::Ideal(PhaseGVN *phase, bool can_reshape) {
   if (progress != nullptr) {
     return progress;
   }
-  if (can_reshape && !phase->C->post_loop_opts_phase()) {
-    // makes sure we run ::Value to potentially remove type assertion after loop opts
+  if (!phase->C->post_loop_opts_phase()) {
+    // makes sure we run widen_type() to potentially common type assertions after loop opts
     phase->C->record_for_post_loop_opts_igvn(this);
   }
   if (!_range_check_dependency || phase->C->post_loop_opts_phase()) {
     return optimize_integer_cast(phase, T_INT);
   }
-  phase->C->record_for_post_loop_opts_igvn(this);
   return nullptr;
 }
 
@@ -279,9 +278,9 @@ void CastIINode::dump_spec(outputStream* st) const {
 #endif
 
 CastIINode* CastIINode::pin_array_access_node() const {
-  assert(_dependency == RegularDependency, "already pinned");
+  assert(_dependency.is_floating(), "already pinned");
   if (has_range_check()) {
-    return new CastIINode(in(0), in(1), bottom_type(), StrongDependency, has_range_check());
+    return new CastIINode(in(0), in(1), bottom_type(), _dependency.with_pinned_dependency(), has_range_check());
   }
   return nullptr;
 }
@@ -315,16 +314,6 @@ void CastIINode::remove_range_check_cast(Compile* C) {
 }
 
 
-const Type* CastLLNode::Value(PhaseGVN* phase) const {
-  const Type* res = ConstraintCastNode::Value(phase);
-  if (res == Type::TOP) {
-    return Type::TOP;
-  }
-  assert(res->isa_long(), "res must be long");
-
-  return widen_type(phase, res, T_LONG);
-}
-
 bool CastLLNode::is_inner_loop_backedge(ProjNode* proj) {
   if (proj != nullptr) {
     Node* ctrl_use = proj->unique_ctrl_out_or_null();
@@ -392,7 +381,7 @@ Node* CastLLNode::Ideal(PhaseGVN* phase, bool can_reshape) {
     return progress;
   }
   if (!phase->C->post_loop_opts_phase()) {
-    // makes sure we run ::Value to potentially remove type assertion after loop opts
+    // makes sure we run widen_type() to potentially common type assertions after loop opts
     phase->C->record_for_post_loop_opts_igvn(this);
   }
   // transform (CastLL (ConvI2L ..)) into (ConvI2L (CastII ..)) if the type of the CastLL is narrower than the type of
@@ -543,7 +532,7 @@ Node* CastP2XNode::Identity(PhaseGVN* phase) {
   return this;
 }
 
-Node* ConstraintCastNode::make_cast_for_type(Node* c, Node* in, const Type* type, DependencyType dependency,
+Node* ConstraintCastNode::make_cast_for_type(Node* c, Node* in, const Type* type, const DependencyType& dependency,
                                              const TypeTuple* types) {
   if (type->isa_int()) {
     return new CastIINode(c, in, type, dependency, false, types);
@@ -564,7 +553,7 @@ Node* ConstraintCastNode::make_cast_for_type(Node* c, Node* in, const Type* type
   return nullptr;
 }
 
-Node* ConstraintCastNode::optimize_integer_cast(PhaseGVN* phase, BasicType bt) {
+Node* ConstraintCastNode::optimize_integer_cast_of_add(PhaseGVN* phase, BasicType bt) {
   PhaseIterGVN *igvn = phase->is_IterGVN();
   const TypeInteger* this_type = this->type()->isa_integer(bt);
   if (this_type == nullptr) {
@@ -586,8 +575,42 @@ Node* ConstraintCastNode::optimize_integer_cast(PhaseGVN* phase, BasicType bt) {
     Node* x = z->in(1);
     Node* y = z->in(2);
 
-    Node* cx = find_or_make_integer_cast(igvn, x, rx);
-    Node* cy = find_or_make_integer_cast(igvn, y, ry);
+    const TypeInteger* tx = phase->type(x)->is_integer(bt);
+    const TypeInteger* ty = phase->type(y)->is_integer(bt);
+
+    // (Cast (Add x y) tz) is transformed into (Add (Cast x rx) (Cast y ry))
+    //
+    // tz = [tzlo, tzhi]
+    // rx = [rxlo, rxhi]
+    // ry = [rylo, ryhi]
+    // with type of x, tx = [txlo, txhi]
+    // with type of y, ty = [tylo, tyhi]
+    //
+    // From Compile::push_thru_add():
+    // rxlo = max(tzlo - tyhi, txlo)
+    // rxhi = min(tzhi - tylo, txhi)
+    // rylo = max(tzlo - txhi, tylo)
+    // ryhi = min(tzhi - txlo, tyhi)
+    //
+    // If x is a constant, then txlo = txhi
+    // rxlo = txlo, rxhi = txhi
+    // The bounds of the type of the Add after transformation then is:
+    // rxlo + rylo >= txlo + tzlo - txhi >= tzlo
+    // rxhi + ryhi <= txhi + tzhi - txlo <= tzhi
+    // The resulting type is not wider than the type of the Cast
+    // before transformation
+    //
+    // If neither x nor y are constant then the type of the resulting
+    // Add can be wider than the type of the type of the Cast before
+    // transformation.
+    // For instance, tx = [0, 10], ty = [0, 10], tz = [0, 10]
+    // then rx = [0, 10], ry = [0, 10]
+    // and rx + ry = [0, 20] which is wider than tz
+    //
+    // Same reasoning applies to (Cast (Sub x y) tz)
+    const DependencyType& dependency = (!tx->is_con() && !ty->is_con()) ? _dependency.with_non_narrowing() : _dependency;
+    Node* cx = find_or_make_integer_cast(igvn, x, rx, dependency);
+    Node* cy = find_or_make_integer_cast(igvn, y, ry, dependency);
     if (op == Op_Add(bt)) {
       return AddNode::make(cx, cy, bt);
     } else {
@@ -599,11 +622,26 @@ Node* ConstraintCastNode::optimize_integer_cast(PhaseGVN* phase, BasicType bt) {
   return nullptr;
 }
 
-const Type* ConstraintCastNode::widen_type(const PhaseGVN* phase, const Type* res, BasicType bt) const {
-  if (!phase->C->post_loop_opts_phase()) {
+Node* ConstraintCastNode::optimize_integer_cast(PhaseGVN* phase, BasicType bt) {
+  Node* res = optimize_integer_cast_of_add(phase, bt);
+  if (res != nullptr) {
     return res;
   }
+  const Type* t = Value(phase);
+  if (t != Type::TOP && phase->C->post_loop_opts_phase()) {
+    const Type* bottom_t = bottom_type();
+    const TypeInteger* wide_t = widen_type(phase, bottom_t, bt);
+    if (wide_t != bottom_t) {
+      // Widening the type of the Cast (to allow some commoning) causes the Cast to change how it can be optimized (if
+      // type of its input is narrower than the Cast's type, we can't remove it to not loose the control dependency).
+      return make_with(in(1), wide_t, _dependency.with_non_narrowing());
+    }
+  }
+  return nullptr;
+}
 
+const TypeInteger* ConstraintCastNode::widen_type(const PhaseGVN* phase, const Type* res, BasicType bt) const {
+  const TypeInteger* this_type = res->is_integer(bt);
   // At VerifyConstraintCasts == 1, we verify the ConstraintCastNodes that are present during code
   // emission. This allows us detecting possible mis-scheduling due to these nodes being pinned at
   // the wrong control nodes.
@@ -612,10 +650,9 @@ const Type* ConstraintCastNode::widen_type(const PhaseGVN* phase, const Type* re
   // mis-transformations that may happen due to these nodes being pinned at the wrong control
   // nodes.
   if (VerifyConstraintCasts > 1) {
-    return res;
+    return this_type;
   }
 
-  const TypeInteger* this_type = res->is_integer(bt);
   const TypeInteger* in_type = phase->type(in(1))->isa_integer(bt);
   if (in_type != nullptr &&
       (in_type->lo_as_long() != this_type->lo_as_long() ||
@@ -636,5 +673,5 @@ const Type* ConstraintCastNode::widen_type(const PhaseGVN* phase, const Type* re
                              MIN2(in_type->hi_as_long(), hi1),
                              MAX2((int)in_type->_widen, w1), bt);
   }
-  return res;
+  return this_type;
 }