Skip to content
This repository has been archived by the owner on Aug 27, 2022. It is now read-only.

Commit

Permalink
8256823: C2 compilation fails with "assert(isShiftCount(imm8 >> 1)) f…
Browse files Browse the repository at this point in the history 
…ailed: illegal shift count"

Reviewed-by: vlivanov, kvn, chagedorn
  • Loading branch information
@TobiHartmann
TobiHartmann committed Nov 24, 2020
1 parent 3b3e90e commit 1c4c99e
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 65 deletions.
4 changes: 2 additions & 2 deletions src/hotspot/cpu/x86/assembler_x86.cpp
View file
Expand Up @@ -4905,7 +4905,7 @@ void Assembler::ret(int imm16) {
}

void Assembler::roll(Register dst, int imm8) {
assert(isShiftCount(imm8 >> 1), "illegal shift count");
assert(isShiftCount(imm8), "illegal shift count");
int encode = prefix_and_encode(dst->encoding());
if (imm8 == 1) {
emit_int16((unsigned char)0xD1, (0xC0 | encode));
Expand All @@ -4920,7 +4920,7 @@ void Assembler::roll(Register dst) {
}

void Assembler::rorl(Register dst, int imm8) {
assert(isShiftCount(imm8 >> 1), "illegal shift count");
assert(isShiftCount(imm8), "illegal shift count");
int encode = prefix_and_encode(dst->encoding());
if (imm8 == 1) {
emit_int16((unsigned char)0xD1, (0xC8 | encode));
Expand Down
26 changes: 13 additions & 13 deletions src/hotspot/share/opto/addnode.cpp
View file
Expand Up @@ -767,22 +767,22 @@ Node* OrINode::Ideal(PhaseGVN* phase, bool can_reshape) {
int ropcode = in(2)->Opcode();
if (Matcher::match_rule_supported(Op_RotateLeft) &&
lopcode == Op_LShiftI && ropcode == Op_URShiftI && in(1)->in(1) == in(2)->in(1)) {
Node* lshift = in(1)->in(2);
Node* rshift = in(2)->in(2);
Node* shift = rotate_shift(phase, lshift, rshift, 0x1F);
if (shift != NULL) {
return new RotateLeftNode(in(1)->in(1), shift, TypeInt::INT);
}
return NULL;
Node* lshift = in(1)->in(2);
Node* rshift = in(2)->in(2);
Node* shift = rotate_shift(phase, lshift, rshift, 0x1F);
if (shift != NULL) {
return new RotateLeftNode(in(1)->in(1), shift, TypeInt::INT);
}
return NULL;
}
if (Matcher::match_rule_supported(Op_RotateRight) &&
lopcode == Op_URShiftI && ropcode == Op_LShiftI && in(1)->in(1) == in(2)->in(1)) {
Node *rshift = in(1)->in(2);
Node *lshift = in(2)->in(2);
Node* shift = rotate_shift(phase, rshift, lshift, 0x1F);
if (shift != NULL) {
return new RotateRightNode(in(1)->in(1), shift, TypeInt::INT);
}
Node* rshift = in(1)->in(2);
Node* lshift = in(2)->in(2);
Node* shift = rotate_shift(phase, rshift, lshift, 0x1F);
if (shift != NULL) {
return new RotateRightNode(in(1)->in(1), shift, TypeInt::INT);
}
}
return NULL;
}
Expand Down
151 changes: 102 additions & 49 deletions src/hotspot/share/opto/mulnode.cpp
View file
Expand Up @@ -637,35 +637,44 @@ Node *AndLNode::Ideal(PhaseGVN *phase, bool can_reshape) {

//=============================================================================

static int getShiftCon(PhaseGVN *phase, Node *shiftNode, int retVal) {
const Type *t = phase->type(shiftNode->in(2));
if (t == Type::TOP) return retVal; // Right input is dead.
const TypeInt *t2 = t->isa_int();
if (!t2 || !t2->is_con()) return retVal; // Right input is a constant.

return t2->get_con();
static bool const_shift_count(PhaseGVN* phase, Node* shiftNode, int* count) {
const TypeInt* tcount = phase->type(shiftNode->in(2))->isa_int();
if (tcount != NULL && tcount->is_con()) {
*count = tcount->get_con();
return true;
}
return false;
}

static int maskShiftAmount(PhaseGVN *phase, Node *shiftNode, int nBits) {
int shift = getShiftCon(phase, shiftNode, 0);
int maskedShift = shift & (nBits - 1);

if (maskedShift == 0) return 0; // Let Identity() handle 0 shift count.
static int maskShiftAmount(PhaseGVN* phase, Node* shiftNode, int nBits) {
int count = 0;
if (const_shift_count(phase, shiftNode, &count)) {
int maskedShift = count & (nBits - 1);
if (maskedShift == 0) {
// Let Identity() handle 0 shift count.
return 0;
}

if (shift != maskedShift) {
shiftNode->set_req(2, phase->intcon(maskedShift)); // Replace shift count with masked value.
PhaseIterGVN* igvn = phase->is_IterGVN();
if (igvn) {
igvn->rehash_node_delayed(shiftNode);
if (count != maskedShift) {
shiftNode->set_req(2, phase->intcon(maskedShift)); // Replace shift count with masked value.
PhaseIterGVN* igvn = phase->is_IterGVN();
if (igvn) {
igvn->rehash_node_delayed(shiftNode);
}
}
return maskedShift;
}

return maskedShift;
return 0;
}

//------------------------------Identity---------------------------------------
Node* LShiftINode::Identity(PhaseGVN* phase) {
return ((getShiftCon(phase, this, -1) & (BitsPerJavaInteger - 1)) == 0) ? in(1) : this;
int count = 0;
if (const_shift_count(phase, this, &count) && (count & (BitsPerJavaInteger - 1)) == 0) {
// Shift by a multiple of 32 does nothing
return in(1);
}
return this;
}

//------------------------------Ideal------------------------------------------
Expand Down Expand Up @@ -773,7 +782,12 @@ const Type* LShiftINode::Value(PhaseGVN* phase) const {
//=============================================================================
//------------------------------Identity---------------------------------------
Node* LShiftLNode::Identity(PhaseGVN* phase) {
return ((getShiftCon(phase, this, -1) & (BitsPerJavaLong - 1)) == 0) ? in(1) : this;
int count = 0;
if (const_shift_count(phase, this, &count) && (count & (BitsPerJavaLong - 1)) == 0) {
// Shift by a multiple of 64 does nothing
return in(1);
}
return this;
}

//------------------------------Ideal------------------------------------------
Expand Down Expand Up @@ -878,26 +892,30 @@ const Type* LShiftLNode::Value(PhaseGVN* phase) const {
//=============================================================================
//------------------------------Identity---------------------------------------
Node* RShiftINode::Identity(PhaseGVN* phase) {
int shift = getShiftCon(phase, this, -1);
if (shift == -1) return this;
if ((shift & (BitsPerJavaInteger - 1)) == 0) return in(1);

// Check for useless sign-masking
if (in(1)->Opcode() == Op_LShiftI &&
in(1)->req() == 3 &&
in(1)->in(2) == in(2)) {
shift &= BitsPerJavaInteger-1; // semantics of Java shifts
// Compute masks for which this shifting doesn't change
int lo = (-1 << (BitsPerJavaInteger - ((uint)shift)-1)); // FFFF8000
int hi = ~lo; // 00007FFF
const TypeInt *t11 = phase->type(in(1)->in(1))->isa_int();
if (!t11) return this;
// Does actual value fit inside of mask?
if (lo <= t11->_lo && t11->_hi <= hi) {
return in(1)->in(1); // Then shifting is a nop
int count = 0;
if (const_shift_count(phase, this, &count)) {
if ((count & (BitsPerJavaInteger - 1)) == 0) {
// Shift by a multiple of 32 does nothing
return in(1);
}
// Check for useless sign-masking
if (in(1)->Opcode() == Op_LShiftI &&
in(1)->req() == 3 &&
in(1)->in(2) == in(2)) {
count &= BitsPerJavaInteger-1; // semantics of Java shifts
// Compute masks for which this shifting doesn't change
int lo = (-1 << (BitsPerJavaInteger - ((uint)count)-1)); // FFFF8000
int hi = ~lo; // 00007FFF
const TypeInt* t11 = phase->type(in(1)->in(1))->isa_int();
if (t11 == NULL) {
return this;
}
// Does actual value fit inside of mask?
if (lo <= t11->_lo && t11->_hi <= hi) {
return in(1)->in(1); // Then shifting is a nop
}
}
}

return this;
}

Expand Down Expand Up @@ -1082,8 +1100,11 @@ const Type* RShiftLNode::Value(PhaseGVN* phase) const {
//=============================================================================
//------------------------------Identity---------------------------------------
Node* URShiftINode::Identity(PhaseGVN* phase) {
int shift = getShiftCon(phase, this, -1);
if ((shift & (BitsPerJavaInteger - 1)) == 0) return in(1);
int count = 0;
if (const_shift_count(phase, this, &count) && (count & (BitsPerJavaInteger - 1)) == 0) {
// Shift by a multiple of 32 does nothing
return in(1);
}

// Check for "((x << LogBytesPerWord) + (wordSize-1)) >> LogBytesPerWord" which is just "x".
// Happens during new-array length computation.
Expand Down Expand Up @@ -1266,7 +1287,12 @@ const Type* URShiftINode::Value(PhaseGVN* phase) const {
//=============================================================================
//------------------------------Identity---------------------------------------
Node* URShiftLNode::Identity(PhaseGVN* phase) {
return ((getShiftCon(phase, this, -1) & (BitsPerJavaLong - 1)) == 0) ? in(1) : this;
int count = 0;
if (const_shift_count(phase, this, &count) && (count & (BitsPerJavaLong - 1)) == 0) {
// Shift by a multiple of 64 does nothing
return in(1);
}
return this;
}

//------------------------------Ideal------------------------------------------
Expand Down Expand Up @@ -1444,6 +1470,21 @@ uint MulAddS2INode::hash() const {

//------------------------------Rotate Operations ------------------------------

Node* RotateLeftNode::Identity(PhaseGVN* phase) {
const Type* t1 = phase->type(in(1));
if (t1 == Type::TOP) {
return this;
}
int count = 0;
assert(t1->isa_int() || t1->isa_long(), "Unexpected type");
int mask = (t1->isa_int() ? BitsPerJavaInteger : BitsPerJavaLong) - 1;
if (const_shift_count(phase, this, &count) && (count & mask) == 0) {
// Rotate by a multiple of 32/64 does nothing
return in(1);
}
return this;
}

const Type* RotateLeftNode::Value(PhaseGVN* phase) const {
const Type* t1 = phase->type(in(1));
const Type* t2 = phase->type(in(2));
Expand All @@ -1460,11 +1501,10 @@ const Type* RotateLeftNode::Value(PhaseGVN* phase) const {
if (r1 == TypeInt::ZERO) {
return TypeInt::ZERO;
}
// Shift by zero does nothing
// Rotate by zero does nothing
if (r2 == TypeInt::ZERO) {
return r1;
}

if (r1->is_con() && r2->is_con()) {
int shift = r2->get_con() & (BitsPerJavaInteger - 1); // semantics of Java shifts
return TypeInt::make((r1->get_con() << shift) | (r1->get_con() >> (32 - shift)));
Expand All @@ -1479,11 +1519,10 @@ const Type* RotateLeftNode::Value(PhaseGVN* phase) const {
if (r1 == TypeLong::ZERO) {
return TypeLong::ZERO;
}
// Shift by zero does nothing
// Rotate by zero does nothing
if (r2 == TypeInt::ZERO) {
return r1;
}

if (r1->is_con() && r2->is_con()) {
int shift = r2->get_con() & (BitsPerJavaLong - 1); // semantics of Java shifts
return TypeLong::make((r1->get_con() << shift) | (r1->get_con() >> (64 - shift)));
Expand All @@ -1508,6 +1547,21 @@ Node* RotateLeftNode::Ideal(PhaseGVN *phase, bool can_reshape) {
return NULL;
}

Node* RotateRightNode::Identity(PhaseGVN* phase) {
const Type* t1 = phase->type(in(1));
if (t1 == Type::TOP) {
return this;
}
int count = 0;
assert(t1->isa_int() || t1->isa_long(), "Unexpected type");
int mask = (t1->isa_int() ? BitsPerJavaInteger : BitsPerJavaLong) - 1;
if (const_shift_count(phase, this, &count) && (count & mask) == 0) {
// Rotate by a multiple of 32/64 does nothing
return in(1);
}
return this;
}

const Type* RotateRightNode::Value(PhaseGVN* phase) const {
const Type* t1 = phase->type(in(1));
const Type* t2 = phase->type(in(2));
Expand All @@ -1524,7 +1578,7 @@ const Type* RotateRightNode::Value(PhaseGVN* phase) const {
if (r1 == TypeInt::ZERO) {
return TypeInt::ZERO;
}
// Shift by zero does nothing
// Rotate by zero does nothing
if (r2 == TypeInt::ZERO) {
return r1;
}
Expand All @@ -1533,7 +1587,6 @@ const Type* RotateRightNode::Value(PhaseGVN* phase) const {
return TypeInt::make((r1->get_con() >> shift) | (r1->get_con() << (32 - shift)));
}
return TypeInt::INT;

} else {
assert(t1->isa_long(), "Type must be a long");
const TypeLong* r1 = t1->is_long();
Expand All @@ -1542,7 +1595,7 @@ const Type* RotateRightNode::Value(PhaseGVN* phase) const {
if (r1 == TypeLong::ZERO) {
return TypeLong::ZERO;
}
// Shift by zero does nothing
// Rotate by zero does nothing
if (r2 == TypeInt::ZERO) {
return r1;
}
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/opto/mulnode.hpp
View file
Expand Up @@ -220,6 +220,7 @@ class RotateLeftNode : public TypeNode {
init_req(2, in2);
}
virtual int Opcode() const;
virtual Node* Identity(PhaseGVN* phase);
virtual const Type* Value(PhaseGVN* phase) const;
virtual Node* Ideal(PhaseGVN* phase, bool can_reshape);
};
Expand All @@ -232,6 +233,7 @@ class RotateRightNode : public TypeNode {
init_req(2, in2);
}
virtual int Opcode() const;
virtual Node* Identity(PhaseGVN* phase);
virtual const Type* Value(PhaseGVN* phase) const;
};

Expand Down

0 comments on commit 1c4c99e

Please sign in to comment.