[turbofan] Take the immediate size in account when narrowing ia32/x64 word comparison operators.

Trying to re-land http://crrev.com/1948453002 after fixing assembler-x64.cc in http://crrev.com/1962563003.

Before this patch, we would emit a cmp or test with a memory operand only if both of the operands in the IR were loads. Now if either of them is a load and the other one is an immediate, we can use a memory operand if the load representation machine size is wide enough to represent the latter.

Review-Url: https://codereview.chromium.org/1968453002
Cr-Commit-Position: refs/heads/master@{#36136}

src/compiler/ia32/instruction-selector-ia32.cc

@@ -1172,6 +1172,26 @@ void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
   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;
+  }
+  if (node->opcode() != IrOpcode::kInt32Constant) {
+    return false;
+  }
+  int32_t value = OpParameter<int32_t>(node->op());
+  if (is_int8(value)) {
+    *representation = MachineRepresentation::kWord8;
+  } else if (is_int16(value)) {
+    *representation = MachineRepresentation::kWord16;
+  } else {
+    *representation = MachineRepresentation::kWord32;
+  }
+  return true;
+}
+
 // Tries to match the size of the given opcode to that of the operands, if
 // possible.
 InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
@@ -1179,20 +1199,22 @@ InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
   if (opcode != kIA32Cmp && opcode != kIA32Test) {
     return opcode;
   }
-  // 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) {
+  // 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) {
+    return opcode;
+  }
+  MachineRepresentation left_representation, right_representation;
+  if (!InferMachineRepresentation(left, &left_representation) ||
+      !InferMachineRepresentation(right, &right_representation)) {
     return opcode;
   }
-  // If the load representations don't match, both operands will be
+  // If the 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())) {
+  if (left_representation != right_representation) {
     return opcode;
   }
-  switch (left_representation.representation()) {
+  switch (left_representation) {
     case MachineRepresentation::kBit:
     case MachineRepresentation::kWord8:
       return opcode == kIA32Cmp ? kIA32Cmp8 : kIA32Test8;
@@ -1240,10 +1262,33 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
 
   // Match immediates on right side of comparison.
   if (g.CanBeImmediate(right)) {
-    if (g.CanBeMemoryOperand(opcode, node, left)) {
-      // TODO(epertoso): we should use `narrowed_opcode' here once we match
-      // immediates too.
-      return VisitCompareWithMemoryOperand(selector, opcode, left,
+    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,
                                            g.UseImmediate(right), cont);
     }
     return VisitCompare(selector, opcode, g.Use(left), g.UseImmediate(right),

src/compiler/x64/instruction-selector-x64.cc

@@ -1500,6 +1500,35 @@ void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
   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,
@@ -1507,20 +1536,22 @@ InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
   if (opcode != kX64Cmp32 && opcode != kX64Test32) {
     return opcode;
   }
-  // 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) {
+  // 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) {
     return opcode;
   }
-  // If the load representations don't match, both operands will be
+  MachineRepresentation left_representation, right_representation;
+  if (!InferMachineRepresentation(left, &left_representation) ||
+      !InferMachineRepresentation(right, &right_representation)) {
+    return opcode;
+  }
+  // If the 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())) {
+  if (left_representation != right_representation) {
     return opcode;
   }
-  switch (left_representation.representation()) {
+  switch (left_representation) {
     case MachineRepresentation::kBit:
     case MachineRepresentation::kWord8:
       return opcode == kX64Cmp32 ? kX64Cmp8 : kX64Test8;
@@ -1538,21 +1569,46 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
   Node* left = node->InputAt(0);
   Node* right = node->InputAt(1);
 
-  opcode = TryNarrowOpcodeSize(opcode, left, right);
+  InstructionCode narrowed_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(opcode, node, right) &&
-       !g.CanBeMemoryOperand(opcode, node, left))) {
+      (g.CanBeMemoryOperand(narrowed_opcode, node, right) &&
+       !g.CanBeMemoryOperand(narrowed_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(opcode, node, left)) {
-      return VisitCompareWithMemoryOperand(selector, opcode, left,
+    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,
                                            g.UseImmediate(right), cont);
     }
     return VisitCompare(selector, opcode, g.Use(left), g.UseImmediate(right),
@@ -1560,8 +1616,8 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
   }
 
   // Match memory operands on left side of comparison.
-  if (g.CanBeMemoryOperand(opcode, node, left)) {
-    return VisitCompareWithMemoryOperand(selector, opcode, left,
+  if (g.CanBeMemoryOperand(narrowed_opcode, node, left)) {
+    return VisitCompareWithMemoryOperand(selector, narrowed_opcode, left,
                                          g.UseRegister(right), cont);
   }
 

test/cctest/compiler/test-run-load-store.cc

@@ -251,6 +251,26 @@ TEST(RunLoadStoreSignExtend32) {
   }
 }
 
+TEST(RunLoadEliminationWithCompareAndTest) {
+  int8_t byte = 0x81;
+  int16_t word = 0xf00f;
+  RawMachineAssemblerTester<int32_t> m;
+  Node* load8 = m.LoadFromPointer(&byte, MachineType::Int8());
+  RawMachineLabel a, b, c, d;
+  m.Branch(m.Word32And(load8, m.Int32Constant(-0x80)), &b, &a);
+  m.Bind(&a);
+  m.Return(m.Int32Constant(0));
+  m.Bind(&b);
+  Node* load16 = m.LoadFromPointer(&word, MachineType::Int16());
+  m.Branch(m.Word32And(load16, m.Int32Constant(-0x7fff)), &d, &c);
+  m.Bind(&c);
+  m.Return(m.Int32Constant(0));
+  m.Bind(&d);
+  m.Return(m.Int32Constant(1));
+
+  CHECK_EQ(1, m.Call());
+}
+
 TEST(RunLoadStoreZeroExtend32) {
   uint32_t buffer[4];
   RawMachineAssemblerTester<uint32_t> m;