Commit b412b789 authored by zhengxing.li's avatar zhengxing.li Committed by Commit bot

X87: [turbofan] Fix invalid comparison operator narrowing.

  port a7581443 (r38231)

  original commit message:
  When we narrow a signed32 comparison to uint8 or uint16 representation,
  we also need to change the condition to unsigned comparisons otherwise
  the comparison will be done on int16/int8 which interprets the narrowed
  bits wrong.

BUG=

Review-Url: https://codereview.chromium.org/2206913002
Cr-Commit-Position: refs/heads/master@{#38274}
parent c7e9a8dc
...@@ -1211,10 +1211,7 @@ void VisitCompare(InstructionSelector* selector, InstructionCode opcode, ...@@ -1211,10 +1211,7 @@ void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
// Tries to match the size of the given opcode to that of the operands, if // Tries to match the size of the given opcode to that of the operands, if
// possible. // possible.
InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left, InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
Node* right) { Node* right, FlagsContinuation* cont) {
if (opcode != kX87Cmp && opcode != kX87Test) {
return opcode;
}
// Currently, if one of the two operands is not a Load, we don't know what its // Currently, if one of the two operands is not a Load, we don't know what its
// machine representation is, so we bail out. // machine representation is, so we bail out.
// TODO(epertoso): we can probably get some size information out of immediates // TODO(epertoso): we can probably get some size information out of immediates
...@@ -1224,19 +1221,39 @@ InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left, ...@@ -1224,19 +1221,39 @@ InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
} }
// If the load representations don't match, both operands will be // If the load representations don't match, both operands will be
// zero/sign-extended to 32bit. // zero/sign-extended to 32bit.
LoadRepresentation left_representation = LoadRepresentationOf(left->op()); MachineType left_type = LoadRepresentationOf(left->op());
if (left_representation != LoadRepresentationOf(right->op())) { MachineType right_type = LoadRepresentationOf(right->op());
return opcode; if (left_type == right_type) {
} switch (left_type.representation()) {
switch (left_representation.representation()) { case MachineRepresentation::kBit:
case MachineRepresentation::kBit: case MachineRepresentation::kWord8: {
case MachineRepresentation::kWord8: if (opcode == kX87Test) return kX87Test8;
return opcode == kX87Cmp ? kX87Cmp8 : kX87Test8; if (opcode == kX87Cmp) {
case MachineRepresentation::kWord16: if (left_type.semantic() == MachineSemantic::kUint32) {
return opcode == kX87Cmp ? kX87Cmp16 : kX87Test16; cont->OverwriteUnsignedIfSigned();
default: } else {
return opcode; CHECK_EQ(MachineSemantic::kInt32, left_type.semantic());
}
return kX87Cmp8;
}
break;
}
case MachineRepresentation::kWord16:
if (opcode == kX87Test) return kX87Test16;
if (opcode == kX87Cmp) {
if (left_type.semantic() == MachineSemantic::kUint32) {
cont->OverwriteUnsignedIfSigned();
} else {
CHECK_EQ(MachineSemantic::kInt32, left_type.semantic());
}
return kX87Cmp16;
}
break;
default:
break;
}
} }
return opcode;
} }
// Shared routine for multiple float32 compare operations (inputs commuted). // Shared routine for multiple float32 compare operations (inputs commuted).
...@@ -1287,7 +1304,8 @@ void VisitWordCompare(InstructionSelector* selector, Node* node, ...@@ -1287,7 +1304,8 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
Node* left = node->InputAt(0); Node* left = node->InputAt(0);
Node* right = node->InputAt(1); Node* right = node->InputAt(1);
InstructionCode narrowed_opcode = TryNarrowOpcodeSize(opcode, left, right); InstructionCode narrowed_opcode =
TryNarrowOpcodeSize(opcode, left, right, cont);
int effect_level = selector->GetEffectLevel(node); int effect_level = selector->GetEffectLevel(node);
if (cont->IsBranch()) { if (cont->IsBranch()) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment