8058744: Crash in C1 OSRed method w/ Unsafe usage
Summary: Fix UnsafeRawOp optimizations
Reviewed-by: kvn, drchase, vlivanov
--- a/hotspot/src/share/vm/c1/c1_Canonicalizer.cpp Tue Sep 23 14:19:55 2014 +0200
+++ b/hotspot/src/share/vm/c1/c1_Canonicalizer.cpp Tue Sep 23 15:09:07 2014 -0700
@@ -327,7 +327,7 @@
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 @@
static bool match_index_and_scale(Instruction* instr,
Instruction** index,
- int* log2_scale,
- Instruction** instr_to_unpin) {
- *instr_to_unpin = NULL;
-
- // Skip conversion ops
+ int* log2_scale) {
+ // Skip conversion ops. This works only on 32bit because of the implicit l2i that the
+ // unsafe performs.
+#ifndef _LP64
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()) {
- *instr_to_unpin = shift;
+ if (shift->op() == Bytecodes::_lshl) {
+ 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;
- if (shift->x()->type() != intType) return false;
+ assert(val != NULL, "Should be an int constant");
+
*index = shift->x();
int tmp_scale = val->value();
if (tmp_scale >= 0 && tmp_scale < 4) {
@@ -842,31 +855,42 @@
ArithmeticOp* arith = instr->as_ArithmeticOp();
if (arith != NULL) {
- if (arith->is_pinned()) {
- *instr_to_unpin = arith;
+ // See if either arg is a known constant
+ Constant* con = arith->x()->as_Constant();
+ if (con != NULL) {
+ *index = arith->y();
+ } else {
+ con = arith->y()->as_Constant();
+ if (con == NULL) return false;
+ *index = arith->x();
}
+ long const_value;
// 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) {
- *index = arith->y();
+ 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();
+ assert(val != NULL, "expecting an int constant");
+ const_value = val->value();
} else {
- con = arith->y()->as_Constant();
- if (con == NULL) return false;
- *index = arith->x();
+ return false;
}
- if ((*index)->type() != intType) return false;
- // Well-known type and value?
- IntConstant* val = con->type()->as_IntConstant();
- if (val == NULL) return false;
- switch (val->value()) {
- case 1: *log2_scale = 0; return true;
- case 2: *log2_scale = 1; return true;
- case 4: *log2_scale = 2; return true;
- case 8: *log2_scale = 3; return true;
- default: 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;
+ case 8: *log2_scale = 3; return true;
+ default: return false;
}
}
@@ -879,29 +903,37 @@
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 {
- return false;
+ match_found = true;
}
- } else {
- // doesn't match any expected sequences
- return false;
+ }
+ // The default solution
+ 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
--- a/hotspot/src/share/vm/c1/c1_LIRGenerator.cpp Tue Sep 23 14:19:55 2014 +0200
+++ b/hotspot/src/share/vm/c1/c1_LIRGenerator.cpp Tue Sep 23 15:09:07 2014 -0700
@@ -2045,6 +2045,8 @@
}
}
+// 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 @@
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 @@
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 @@
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
+ // TODO: ARM also allows embedded shift in the address
__ 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);