8058744: Crash in C1 OSRed method w/ Unsafe usage

Fix UnsafeRawOp optimizations

Reviewed-by: kvn, drchase, vlivanov

hotspot/src/share/vm/c1/c1_Canonicalizer.cpp

@@ -327,7 +327,7 @@ void Canonicalizer::do_ShiftOp        (ShiftOp*         x) {
   if (t2->is_constant()) {
     switch (t2->tag()) {
       case intTag   : if (t2->as_IntConstant()->value() == 0)  set_canonical(x->x()); return;
-      case longTag  : if (t2->as_IntConstant()->value() == 0)  set_canonical(x->x()); return;
+      case longTag  : if (t2->as_LongConstant()->value() == (jlong)0)  set_canonical(x->x()); return;
       default       : ShouldNotReachHere();
     }
   }
@@ -808,28 +808,41 @@ void Canonicalizer::do_ExceptionObject(ExceptionObject* x) {}
 
 static bool match_index_and_scale(Instruction*  instr,
                                   Instruction** index,
-                                  int*          log2_scale,
+                                  int*          log2_scale) {
-                                  Instruction** instr_to_unpin) {
+  // Skip conversion ops. This works only on 32bit because of the implicit l2i that the
-  *instr_to_unpin = NULL;
+  // unsafe performs.
-
+#ifndef _LP64
-  // Skip conversion ops
   Convert* convert = instr->as_Convert();
-  if (convert != NULL) {
+  if (convert != NULL && convert->op() == Bytecodes::_i2l) {
+    assert(convert->value()->type() == intType, "invalid input type");
     instr = convert->value();
   }
+#endif
 
   ShiftOp* shift = instr->as_ShiftOp();
   if (shift != NULL) {
-    if (shift->is_pinned()) {
+    if (shift->op() == Bytecodes::_lshl) {
-      *instr_to_unpin = shift;
+      assert(shift->x()->type() == longType, "invalid input type");
+    } else {
+#ifndef _LP64
+      if (shift->op() == Bytecodes::_ishl) {
+        assert(shift->x()->type() == intType, "invalid input type");
+      } else {
+        return false;
       }
+#else
+      return false;
+#endif
+    }
+
+
     // Constant shift value?
     Constant* con = shift->y()->as_Constant();
     if (con == NULL) return false;
     // Well-known type and value?
     IntConstant* val = con->type()->as_IntConstant();
-    if (val == NULL) return false;
+    assert(val != NULL, "Should be an int constant");
-    if (shift->x()->type() != intType) return false;
+
     *index = shift->x();
     int tmp_scale = val->value();
     if (tmp_scale >= 0 && tmp_scale < 4) {
@@ -842,11 +855,6 @@ static bool match_index_and_scale(Instruction*  instr,
 
   ArithmeticOp* arith = instr->as_ArithmeticOp();
   if (arith != NULL) {
-    if (arith->is_pinned()) {
-      *instr_to_unpin = arith;
-    }
-    // Check for integer multiply
-    if (arith->op() == Bytecodes::_imul) {
     // See if either arg is a known constant
     Constant* con = arith->x()->as_Constant();
     if (con != NULL) {
@@ -856,11 +864,28 @@ static bool match_index_and_scale(Instruction*  instr,
       if (con == NULL) return false;
       *index = arith->x();
     }
-      if ((*index)->type() != intType) return false;
+    long const_value;
-      // Well-known type and value?
+    // Check for integer multiply
+    if (arith->op() == Bytecodes::_lmul) {
+      assert((*index)->type() == longType, "invalid input type");
+      LongConstant* val = con->type()->as_LongConstant();
+      assert(val != NULL, "expecting a long constant");
+      const_value = val->value();
+    } else {
+#ifndef _LP64
+      if (arith->op() == Bytecodes::_imul) {
+        assert((*index)->type() == intType, "invalid input type");
         IntConstant* val = con->type()->as_IntConstant();
-      if (val == NULL) return false;
+        assert(val != NULL, "expecting an int constant");
-      switch (val->value()) {
+        const_value = val->value();
+      } else {
+        return false;
+      }
+#else
+      return false;
+#endif
+    }
+    switch (const_value) {
     case 1: *log2_scale = 0; return true;
     case 2: *log2_scale = 1; return true;
     case 4: *log2_scale = 2; return true;
@@ -868,7 +893,6 @@ static bool match_index_and_scale(Instruction*  instr,
     default:            return false;
     }
   }
-  }
 
   // Unknown instruction sequence; don't touch it
   return false;
@@ -879,29 +903,37 @@ static bool match(UnsafeRawOp* x,
                   Instruction** base,
                   Instruction** index,
                   int*          log2_scale) {
-  Instruction* instr_to_unpin = NULL;
   ArithmeticOp* root = x->base()->as_ArithmeticOp();
   if (root == NULL) return false;
   // Limit ourselves to addition for now
   if (root->op() != Bytecodes::_ladd) return false;
+
+  bool match_found = false;
   // Try to find shift or scale op
-  if (match_index_and_scale(root->y(), index, log2_scale, &instr_to_unpin)) {
+  if (match_index_and_scale(root->y(), index, log2_scale)) {
     *base = root->x();
-  } else if (match_index_and_scale(root->x(), index, log2_scale, &instr_to_unpin)) {
+    match_found = true;
+  } else if (match_index_and_scale(root->x(), index, log2_scale)) {
     *base = root->y();
-  } else if (root->y()->as_Convert() != NULL) {
+    match_found = true;
+  } else if (NOT_LP64(root->y()->as_Convert() != NULL) LP64_ONLY(false)) {
+    // Skipping i2l works only on 32bit because of the implicit l2i that the unsafe performs.
+    // 64bit needs a real sign-extending conversion.
     Convert* convert = root->y()->as_Convert();
-    if (convert->op() == Bytecodes::_i2l && convert->value()->type() == intType) {
+    if (convert->op() == Bytecodes::_i2l) {
+      assert(convert->value()->type() == intType, "should be an int");
       // pick base and index, setting scale at 1
       *base  = root->x();
       *index = convert->value();
       *log2_scale = 0;
-    } else {
+      match_found = true;
-      return false;
     }
-  } else {
+  }
-    // doesn't match any expected sequences
+  // The default solution
-    return false;
+  if (!match_found) {
+    *base = root->x();
+    *index = root->y();
+    *log2_scale = 0;
   }
 
   // If the value is pinned then it will be always be computed so

hotspot/src/share/vm/c1/c1_LIRGenerator.cpp

@@ -2045,6 +2045,8 @@ void LIRGenerator::do_RoundFP(RoundFP* x) {
   }
 }
 
+// Here UnsafeGetRaw may have x->base() and x->index() be int or long
+// on both 64 and 32 bits. Expecting x->base() to be always long on 64bit.
 void LIRGenerator::do_UnsafeGetRaw(UnsafeGetRaw* x) {
   LIRItem base(x->base(), this);
   LIRItem idx(this);
@@ -2059,50 +2061,73 @@ void LIRGenerator::do_UnsafeGetRaw(UnsafeGetRaw* x) {
 
   int   log2_scale = 0;
   if (x->has_index()) {
-    assert(x->index()->type()->tag() == intTag, "should not find non-int index");
     log2_scale = x->log2_scale();
   }
 
   assert(!x->has_index() || idx.value() == x->index(), "should match");
 
   LIR_Opr base_op = base.result();
+  LIR_Opr index_op = idx.result();
 #ifndef _LP64
   if (x->base()->type()->tag() == longTag) {
     base_op = new_register(T_INT);
     __ convert(Bytecodes::_l2i, base.result(), base_op);
-  } else {
-    assert(x->base()->type()->tag() == intTag, "must be");
   }
+  if (x->has_index()) {
+    if (x->index()->type()->tag() == longTag) {
+      LIR_Opr long_index_op = index_op;
+      if (x->index()->type()->is_constant()) {
+        long_index_op = new_register(T_LONG);
+        __ move(index_op, long_index_op);
+      }
+      index_op = new_register(T_INT);
+      __ convert(Bytecodes::_l2i, long_index_op, index_op);
+    } else {
+      assert(x->index()->type()->tag() == intTag, "must be");
+    }
+  }
+  // At this point base and index should be all ints.
+  assert(base_op->type() == T_INT && !base_op->is_constant(), "base should be an non-constant int");
+  assert(!x->has_index() || index_op->type() == T_INT, "index should be an int");
+#else
+  if (x->has_index()) {
+    if (x->index()->type()->tag() == intTag) {
+      if (!x->index()->type()->is_constant()) {
+        index_op = new_register(T_LONG);
+        __ convert(Bytecodes::_i2l, idx.result(), index_op);
+      }
+    } else {
+      assert(x->index()->type()->tag() == longTag, "must be");
+      if (x->index()->type()->is_constant()) {
+        index_op = new_register(T_LONG);
+        __ move(idx.result(), index_op);
+      }
+    }
+  }
+  // At this point base is a long non-constant
+  // Index is a long register or a int constant.
+  // We allow the constant to stay an int because that would allow us a more compact encoding by
+  // embedding an immediate offset in the address expression. If we have a long constant, we have to
+  // move it into a register first.
+  assert(base_op->type() == T_LONG && !base_op->is_constant(), "base must be a long non-constant");
+  assert(!x->has_index() || (index_op->type() == T_INT && index_op->is_constant()) ||
+                            (index_op->type() == T_LONG && !index_op->is_constant()), "unexpected index type");
 #endif
 
   BasicType dst_type = x->basic_type();
-  LIR_Opr index_op = idx.result();
 
   LIR_Address* addr;
   if (index_op->is_constant()) {
     assert(log2_scale == 0, "must not have a scale");
+    assert(index_op->type() == T_INT, "only int constants supported");
     addr = new LIR_Address(base_op, index_op->as_jint(), dst_type);
   } else {
 #ifdef X86
-#ifdef _LP64
-    if (!index_op->is_illegal() && index_op->type() == T_INT) {
-      LIR_Opr tmp = new_pointer_register();
-      __ convert(Bytecodes::_i2l, index_op, tmp);
-      index_op = tmp;
-    }
-#endif
     addr = new LIR_Address(base_op, index_op, LIR_Address::Scale(log2_scale), 0, dst_type);
 #elif defined(ARM)
     addr = generate_address(base_op, index_op, log2_scale, 0, dst_type);
 #else
     if (index_op->is_illegal() || log2_scale == 0) {
-#ifdef _LP64
-      if (!index_op->is_illegal() && index_op->type() == T_INT) {
-        LIR_Opr tmp = new_pointer_register();
-        __ convert(Bytecodes::_i2l, index_op, tmp);
-        index_op = tmp;
-      }
-#endif
       addr = new LIR_Address(base_op, index_op, dst_type);
     } else {
       LIR_Opr tmp = new_pointer_register();
@@ -2129,7 +2154,6 @@ void LIRGenerator::do_UnsafePutRaw(UnsafePutRaw* x) {
   BasicType type = x->basic_type();
 
   if (x->has_index()) {
-    assert(x->index()->type()->tag() == intTag, "should not find non-int index");
     log2_scale = x->log2_scale();
   }
 
@@ -2152,38 +2176,39 @@ void LIRGenerator::do_UnsafePutRaw(UnsafePutRaw* x) {
   set_no_result(x);
 
   LIR_Opr base_op = base.result();
+  LIR_Opr index_op = idx.result();
+
 #ifndef _LP64
   if (x->base()->type()->tag() == longTag) {
     base_op = new_register(T_INT);
     __ convert(Bytecodes::_l2i, base.result(), base_op);
-  } else {
-    assert(x->base()->type()->tag() == intTag, "must be");
   }
+  if (x->has_index()) {
+    if (x->index()->type()->tag() == longTag) {
+      index_op = new_register(T_INT);
+      __ convert(Bytecodes::_l2i, idx.result(), index_op);
+    }
+  }
+  // At this point base and index should be all ints and not constants
+  assert(base_op->type() == T_INT && !base_op->is_constant(), "base should be an non-constant int");
+  assert(!x->has_index() || (index_op->type() == T_INT && !index_op->is_constant()), "index should be an non-constant int");
+#else
+  if (x->has_index()) {
+    if (x->index()->type()->tag() == intTag) {
+      index_op = new_register(T_LONG);
+      __ convert(Bytecodes::_i2l, idx.result(), index_op);
+    }
+  }
+  // At this point base and index are long and non-constant
+  assert(base_op->type() == T_LONG && !base_op->is_constant(), "base must be a non-constant long");
+  assert(!x->has_index() || (index_op->type() == T_LONG && !index_op->is_constant()), "index must be a non-constant long");
 #endif
 
-  LIR_Opr index_op = idx.result();
   if (log2_scale != 0) {
     // temporary fix (platform dependent code without shift on Intel would be better)
-    index_op = new_pointer_register();
-#ifdef _LP64
-    if(idx.result()->type() == T_INT) {
-      __ convert(Bytecodes::_i2l, idx.result(), index_op);
-    } else {
-#endif
     // TODO: ARM also allows embedded shift in the address
-      __ move(idx.result(), index_op);
-#ifdef _LP64
-    }
-#endif
     __ shift_left(index_op, log2_scale, index_op);
   }
-#ifdef _LP64
-  else if(!index_op->is_illegal() && index_op->type() == T_INT) {
-    LIR_Opr tmp = new_pointer_register();
-    __ convert(Bytecodes::_i2l, index_op, tmp);
-    index_op = tmp;
-  }
-#endif
 
   LIR_Address* addr = new LIR_Address(base_op, index_op, x->basic_type());
   __ move(value.result(), addr);