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

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

Issue 2202803003: [turbofan] Fix invalid comparison operator narrowing. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Created 4 years, 4 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/instruction-selector-impl.h ('k') | test/mjsunit/regress/regress-v8-5254-1.js » ('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 8e54897d814281686c98a1869306f4de6c6f6918..7315b22beab771571284b2acd99454ec56eb32ca 100644
--- a/src/compiler/x64/instruction-selector-x64.cc
+++ b/src/compiler/x64/instruction-selector-x64.cc
@@ -1581,10 +1581,7 @@ void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
// Tries to match the size of the given opcode to that of the operands, if
// possible.
InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
- Node* right) {
- if (opcode != kX64Cmp32 && opcode != kX64Test32) {
- return opcode;
- }
+ Node* right, FlagsContinuation* cont) {
// 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
@@ -1594,19 +1591,39 @@ InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
}
// 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;
- }
- switch (left_representation.representation()) {
- case MachineRepresentation::kBit:
- case MachineRepresentation::kWord8:
- return opcode == kX64Cmp32 ? kX64Cmp8 : kX64Test8;
- case MachineRepresentation::kWord16:
- return opcode == kX64Cmp32 ? kX64Cmp16 : kX64Test16;
- default:
- return opcode;
+ MachineType left_type = LoadRepresentationOf(left->op());
+ MachineType right_type = LoadRepresentationOf(right->op());
+ if (left_type == right_type) {
+ switch (left_type.representation()) {
+ case MachineRepresentation::kBit:
+ case MachineRepresentation::kWord8: {
+ if (opcode == kX64Test32) return kX64Test8;
+ if (opcode == kX64Cmp32) {
+ if (left_type.semantic() == MachineSemantic::kUint32) {
+ cont->OverwriteUnsignedIfSigned();
+ } else {
+ CHECK_EQ(MachineSemantic::kInt32, left_type.semantic());
+ }
+ return kX64Cmp8;
+ }
+ break;
+ }
+ case MachineRepresentation::kWord16:
+ if (opcode == kX64Test32) return kX64Test16;
+ if (opcode == kX64Cmp32) {
+ if (left_type.semantic() == MachineSemantic::kUint32) {
+ cont->OverwriteUnsignedIfSigned();
+ } else {
+ CHECK_EQ(MachineSemantic::kInt32, left_type.semantic());
+ }
+ return kX64Cmp16;
+ }
+ break;
+ default:
+ break;
+ }
}
+ return opcode;
}
// Shared routine for multiple word compare operations.
@@ -1616,7 +1633,7 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
Node* left = node->InputAt(0);
Node* right = node->InputAt(1);
- opcode = TryNarrowOpcodeSize(opcode, left, right);
+ opcode = TryNarrowOpcodeSize(opcode, left, right, cont);
// 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.
« no previous file with comments | « src/compiler/instruction-selector-impl.h ('k') | test/mjsunit/regress/regress-v8-5254-1.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698