Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(322)

Unified Diff: src/compiler/x64/instruction-selector-x64.cc

Issue 1995303003: Revert of [turbofan] Take the immediate size in account when narrowing ia32/x64 word comparison operators. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/compiler/ia32/instruction-selector-ia32.cc ('k') | test/cctest/compiler/test-run-load-store.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/compiler/x64/instruction-selector-x64.cc
diff --git a/src/compiler/x64/instruction-selector-x64.cc b/src/compiler/x64/instruction-selector-x64.cc
index a4ade55198ae508b1b6a96b73a2727446a2aa6a2..e494dd1d1f83db33c6f5a391f1353cd6d757b605 100644
--- a/src/compiler/x64/instruction-selector-x64.cc
+++ b/src/compiler/x64/instruction-selector-x64.cc
@@ -1500,35 +1500,6 @@
VisitCompare(selector, opcode, g.UseRegister(left), g.Use(right), cont);
}
-bool InferMachineRepresentation(Node* node,
- MachineRepresentation* representation) {
- if (node->opcode() == IrOpcode::kLoad) {
- *representation = LoadRepresentationOf(node->op()).representation();
- return true;
- }
- int64_t value = 0;
- switch (node->opcode()) {
- case IrOpcode::kInt32Constant:
- value = OpParameter<int32_t>(node->op());
- break;
- case IrOpcode::kInt64Constant:
- value = OpParameter<int64_t>(node->op());
- break;
- default:
- return false;
- }
- if (is_int8(value)) {
- *representation = MachineRepresentation::kWord8;
- } else if (is_int16(value)) {
- *representation = MachineRepresentation::kWord16;
- } else if (is_int32(value)) {
- *representation = MachineRepresentation::kWord32;
- } else {
- return false;
- }
- return true;
-}
-
// Tries to match the size of the given opcode to that of the operands, if
// possible.
InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
@@ -1536,22 +1507,20 @@
if (opcode != kX64Cmp32 && opcode != kX64Test32) {
return opcode;
}
- // We only do this if at least one of the two operands is a load.
- // TODO(epertoso): we can probably get some size information out of phi nodes.
- if (left->opcode() != IrOpcode::kLoad && right->opcode() != IrOpcode::kLoad) {
+ // Currently, if one of the two operands is not a Load, we don't know what its
+ // machine representation is, so we bail out.
+ // TODO(epertoso): we can probably get some size information out of immediates
+ // and phi nodes.
+ if (left->opcode() != IrOpcode::kLoad || right->opcode() != IrOpcode::kLoad) {
return opcode;
}
- MachineRepresentation left_representation, right_representation;
- if (!InferMachineRepresentation(left, &left_representation) ||
- !InferMachineRepresentation(right, &right_representation)) {
+ // If the load representations don't match, both operands will be
+ // zero/sign-extended to 32bit.
+ LoadRepresentation left_representation = LoadRepresentationOf(left->op());
+ if (left_representation != LoadRepresentationOf(right->op())) {
return opcode;
}
- // If the representations don't match, both operands will be
- // zero/sign-extended to 32bit.
- if (left_representation != right_representation) {
- return opcode;
- }
- switch (left_representation) {
+ switch (left_representation.representation()) {
case MachineRepresentation::kBit:
case MachineRepresentation::kWord8:
return opcode == kX64Cmp32 ? kX64Cmp8 : kX64Test8;
@@ -1569,46 +1538,21 @@
Node* left = node->InputAt(0);
Node* right = node->InputAt(1);
- InstructionCode narrowed_opcode = TryNarrowOpcodeSize(opcode, left, right);
+ opcode = TryNarrowOpcodeSize(opcode, left, right);
// If one of the two inputs is an immediate, make sure it's on the right, or
// if one of the two inputs is a memory operand, make sure it's on the left.
if ((!g.CanBeImmediate(right) && g.CanBeImmediate(left)) ||
- (g.CanBeMemoryOperand(narrowed_opcode, node, right) &&
- !g.CanBeMemoryOperand(narrowed_opcode, node, left))) {
+ (g.CanBeMemoryOperand(opcode, node, right) &&
+ !g.CanBeMemoryOperand(opcode, node, left))) {
if (!node->op()->HasProperty(Operator::kCommutative)) cont->Commute();
std::swap(left, right);
}
// Match immediates on right side of comparison.
if (g.CanBeImmediate(right)) {
- if (g.CanBeMemoryOperand(narrowed_opcode, node, left)) {
- // If we're truncating the immediate (32 bits to 16 or 8), comparison
- // semantics should take the signedness/unsignedness of the op into
- // account.
- if (narrowed_opcode != opcode &&
- LoadRepresentationOf(left->op()).IsUnsigned()) {
- switch (cont->condition()) {
- case FlagsCondition::kSignedLessThan:
- cont->OverwriteAndNegateIfEqual(FlagsCondition::kUnsignedLessThan);
- break;
- case FlagsCondition::kSignedGreaterThan:
- cont->OverwriteAndNegateIfEqual(
- FlagsCondition::kUnsignedGreaterThan);
- break;
- case FlagsCondition::kSignedLessThanOrEqual:
- cont->OverwriteAndNegateIfEqual(
- FlagsCondition::kUnsignedLessThanOrEqual);
- break;
- case FlagsCondition::kSignedGreaterThanOrEqual:
- cont->OverwriteAndNegateIfEqual(
- FlagsCondition::kUnsignedGreaterThanOrEqual);
- break;
- default:
- break;
- }
- }
- return VisitCompareWithMemoryOperand(selector, narrowed_opcode, left,
+ if (g.CanBeMemoryOperand(opcode, node, left)) {
+ return VisitCompareWithMemoryOperand(selector, opcode, left,
g.UseImmediate(right), cont);
}
return VisitCompare(selector, opcode, g.Use(left), g.UseImmediate(right),
@@ -1616,8 +1560,8 @@
}
// Match memory operands on left side of comparison.
- if (g.CanBeMemoryOperand(narrowed_opcode, node, left)) {
- return VisitCompareWithMemoryOperand(selector, narrowed_opcode, left,
+ if (g.CanBeMemoryOperand(opcode, node, left)) {
+ return VisitCompareWithMemoryOperand(selector, opcode, left,
g.UseRegister(right), cont);
}
« no previous file with comments | « src/compiler/ia32/instruction-selector-ia32.cc ('k') | test/cctest/compiler/test-run-load-store.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698