[turbofan] Properly cover memory operands in comparisons.

The InstructionSelector::CanCover() heuristic was not correctly set to
match loads that are wired into the effect chain (i.e. when the input
comes from the JavaScript pipeline instead of the RawMachineAssembler).
Also the InstructionSelector on x64 was confused by the
CanBeBetterLeftOperand heuristic, which prevented proper covering for
map checks generated by the JavaScript pipeline.

R=epertoso@chromium.org

Review URL: https://codereview.chromium.org/1734503002

Cr-Commit-Position: refs/heads/master@{#34242}

src/compiler/instruction-selector.cc

@@ -218,10 +218,25 @@ Instruction* InstructionSelector::Emit(Instruction* instr) {
 
 
 bool InstructionSelector::CanCover(Node* user, Node* node) const {
-  return node->OwnedBy(user) &&
-         schedule()->block(node) == schedule()->block(user) &&
-         (node->op()->HasProperty(Operator::kPure) ||
-          GetEffectLevel(node) == GetEffectLevel(user));
+  // 1. Both {user} and {node} must be in the same basic block.
+  if (schedule()->block(node) != schedule()->block(user)) {
+    return false;
+  }
+  // 2. Pure {node}s must be owned by the {user}.
+  if (node->op()->HasProperty(Operator::kPure)) {
+    return node->OwnedBy(user);
+  }
+  // 3. Impure {node}s must match the effect level of {user}.
+  if (GetEffectLevel(node) != GetEffectLevel(user)) {
+    return false;
+  }
+  // 4. Only {node} must have value edges pointing to {user}.
+  for (Edge const edge : node->use_edges()) {
+    if (edge.from() != user && NodeProperties::IsValueEdge(edge)) {
+      return false;
+    }
+  }
+  return true;
 }
 
 int InstructionSelector::GetVirtualRegister(const Node* node) {

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

@@ -36,6 +36,22 @@ class X64OperandGenerator final : public OperandGenerator {
     }
   }
 
+  bool CanBeMemoryOperand(InstructionCode opcode, Node* node, Node* input) {
+    if (input->opcode() != IrOpcode::kLoad ||
+        !selector()->CanCover(node, input)) {
+      return false;
+    }
+    MachineRepresentation rep =
+        LoadRepresentationOf(input->op()).representation();
+    if (rep == MachineRepresentation::kWord64 ||
+        rep == MachineRepresentation::kTagged) {
+      return opcode == kX64Cmp || opcode == kX64Test;
+    } else if (rep == MachineRepresentation::kWord32) {
+      return opcode == kX64Cmp32 || opcode == kX64Test32;
+    }
+    return false;
+  }
+
   AddressingMode GenerateMemoryOperandInputs(Node* index, int scale_exponent,
                                              Node* base, Node* displacement,
                                              InstructionOperand inputs[],
@@ -1371,23 +1387,6 @@ void VisitCompareWithMemoryOperand(InstructionSelector* selector,
   }
 }
 
-// Determines if {input} of {node} can be replaced by a memory operand.
-bool CanUseMemoryOperand(InstructionSelector* selector, InstructionCode opcode,
-                         Node* node, Node* input) {
-  if (input->opcode() != IrOpcode::kLoad || !selector->CanCover(node, input)) {
-    return false;
-  }
-  MachineRepresentation rep =
-      LoadRepresentationOf(input->op()).representation();
-  if (rep == MachineRepresentation::kWord64 ||
-      rep == MachineRepresentation::kTagged) {
-    return opcode == kX64Cmp || opcode == kX64Test;
-  } else if (rep == MachineRepresentation::kWord32) {
-    return opcode == kX64Cmp32 || opcode == kX64Test32;
-  }
-  return false;
-}
-
 // Shared routine for multiple compare operations.
 void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
                   InstructionOperand left, InstructionOperand right,
@@ -1425,15 +1424,18 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
   Node* left = node->InputAt(0);
   Node* right = node->InputAt(1);
 
-  // If one of the two inputs is an immediate, make sure it's on the right.
-  if (!g.CanBeImmediate(right) && g.CanBeImmediate(left)) {
+  // 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))) {
     if (!node->op()->HasProperty(Operator::kCommutative)) cont->Commute();
     std::swap(left, right);
   }
 
   // Match immediates on right side of comparison.
   if (g.CanBeImmediate(right)) {
-    if (CanUseMemoryOperand(selector, opcode, node, left)) {
+    if (g.CanBeMemoryOperand(opcode, node, left)) {
       return VisitCompareWithMemoryOperand(selector, opcode, left,
                                            g.UseImmediate(right), cont);
     }
@@ -1441,15 +1443,17 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
                         cont);
   }
 
+  // Match memory operands on left side of comparison.
+  if (g.CanBeMemoryOperand(opcode, node, left)) {
+    return VisitCompareWithMemoryOperand(selector, opcode, left,
+                                         g.UseRegister(right), cont);
+  }
+
   if (g.CanBeBetterLeftOperand(right)) {
     if (!node->op()->HasProperty(Operator::kCommutative)) cont->Commute();
     std::swap(left, right);
   }
 
-  if (CanUseMemoryOperand(selector, opcode, node, left)) {
-    return VisitCompareWithMemoryOperand(selector, opcode, left,
-                                         g.UseRegister(right), cont);
-  }
   return VisitCompare(selector, opcode, left, right, cont,
                       node->op()->HasProperty(Operator::kCommutative));
 }