[turbofan] Emit memory operands for cmp and test on ia32 and x64 when it makes sense.

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

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/base/adapters.h"
 #include "src/compiler/instruction-selector-impl.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties.h"
 
 namespace v8 {
@@ skipping 966 matching lines @@
 }
 
 
 bool InstructionSelector::IsTailCallAddressImmediate() { return true; }
 
 
 namespace {
 
 // Shared routine for multiple compare operations.
 void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
+                  size_t left_operands_count, InstructionOperand* left_operands,
+                  InstructionOperand right, FlagsContinuation* cont) {
+  DCHECK(left_operands_count > 0 && left_operands_count <= 4);
+  IA32OperandGenerator g(selector);
+  opcode = cont->Encode(opcode);
+  int input_count = 0;
+  InstructionOperand inputs[6];
+  do {
+    inputs[input_count] = left_operands[input_count];
+  } while (++input_count < left_operands_count);
+  inputs[input_count++] = right;
+
+  if (cont->IsBranch()) {
+    inputs[input_count++] = g.Label(cont->true_block());
+    inputs[input_count++] = g.Label(cont->false_block());
+    selector->Emit(opcode, 0, nullptr, input_count, inputs);
+  } else {
+    DCHECK(cont->IsSet());
+    InstructionOperand output = g.DefineAsRegister(cont->result());
+    selector->Emit(opcode, 1, &output, input_count, inputs);
+  }
+}
+
+// Determines if `input' of `node' can be replaced by a memory operand.
+bool CanUseMemOp(InstructionCode opcode, Node* node, Node* input) {

[Benedikt Meurer, 2016/02/18 05:02:53]

Nit: Please use complete workds as in the rest of the file, i.e.
CanUseMemoryOperand

[epertoso, 2016/02/18 10:58:29]

Done.

+  if (input->opcode() == IrOpcode::kLoad && input->OwnedBy(node)) {

[Benedikt Meurer, 2016/02/18 05:02:53]

OwnedBy is not enough here. You need to use CanCover.

[epertoso, 2016/02/18 10:58:29]

Done. Also added the effect level check.

+    if (LoadRepresentationOf(input->op()).representation() ==
+        MachineRepresentation::kWord32) {
+      return (opcode == kIA32Cmp || opcode == kIA32Test);
+    }
+  }
+  return false;
+}
+
+// Shared routine for multiple compare operations.
+void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
                   InstructionOperand left, InstructionOperand right,
                   FlagsContinuation* cont) {
-  IA32OperandGenerator g(selector);
-  if (cont->IsBranch()) {
-    selector->Emit(cont->Encode(opcode), g.NoOutput(), left, right,
-                   g.Label(cont->true_block()), g.Label(cont->false_block()));
-  } else {
-    DCHECK(cont->IsSet());
-    selector->Emit(cont->Encode(opcode), g.DefineAsByteRegister(cont->result()),
-                   left, right);
-  }
+  VisitCompare(selector, opcode, 1, &left, right, cont);
 }
 
 
 // Shared routine for multiple compare operations.
 void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
                   Node* left, Node* right, FlagsContinuation* cont,
-                  bool commutative) {
+                  bool commutative, Node* node = nullptr) {
   IA32OperandGenerator g(selector);
   if (commutative && g.CanBeBetterLeftOperand(right)) {
     std::swap(left, right);
   }
-  VisitCompare(selector, opcode, g.UseRegister(left), g.Use(right), cont);
+  if (node && CanUseMemOp(opcode, node, left)) {

[Benedikt Meurer, 2016/02/18 05:02:53]

This is not safe unless you also know that there's is no other instruction in
between that invalidates the load, i.e.

 x = load o, i
 store o, i, v
 cmp x, 1

here you must not combine that load+cmp. As discussed earlier, you'll need to
compute an effect level for this to be safe.
 

[epertoso, 2016/02/18 10:58:29]

Done in CanUseMemoryOperand.

+    size_t input_count = 0;
+    InstructionOperand inputs[4];
+    AddressingMode addressing_mode =
+        g.GetEffectiveAddressMemoryOperand(left, inputs, &input_count);
+    opcode |= AddressingModeField::encode(addressing_mode);
+    VisitCompare(selector, opcode, input_count, inputs, g.Use(right), cont);
+  } else {
+    VisitCompare(selector, opcode, g.UseRegister(left), g.Use(right), cont);
+  }
 }
 
 
 // Shared routine for multiple float32 compare operations (inputs commuted).
 void VisitFloat32Compare(InstructionSelector* selector, Node* node,
                          FlagsContinuation* cont) {
   Node* const left = node->InputAt(0);
   Node* const right = node->InputAt(1);
   VisitCompare(selector, kSSEFloat32Cmp, right, left, cont, false);
 }
 
 
 // Shared routine for multiple float64 compare operations (inputs commuted).
 void VisitFloat64Compare(InstructionSelector* selector, Node* node,
                          FlagsContinuation* cont) {
   Node* const left = node->InputAt(0);
   Node* const right = node->InputAt(1);
   VisitCompare(selector, kSSEFloat64Cmp, right, left, cont, false);
 }
 
 
 // Shared routine for multiple word compare operations.
 void VisitWordCompare(InstructionSelector* selector, Node* node,
                       InstructionCode opcode, FlagsContinuation* cont) {
   IA32OperandGenerator g(selector);
-  Node* const left = node->InputAt(0);
-  Node* const right = node->InputAt(1);
+  Node* left = node->InputAt(0);
+  Node* right = node->InputAt(1);
 
-  // Match immediates on left or right side of comparison.
+  if (!g.CanBeImmediate(right) && g.CanBeImmediate(left)) {
+    if (!node->op()->HasProperty(Operator::kCommutative)) cont->Commute();
+    std::swap(left, right);
+  }
+
+  // Match immediates on right side of comparison.
   if (g.CanBeImmediate(right)) {
-    VisitCompare(selector, opcode, g.Use(left), g.UseImmediate(right), cont);
-  } else if (g.CanBeImmediate(left)) {
-    if (!node->op()->HasProperty(Operator::kCommutative)) cont->Commute();
-    VisitCompare(selector, opcode, g.Use(right), g.UseImmediate(left), cont);
+    if (CanUseMemOp(opcode, node, left)) {
+      size_t left_operands_count = 0;
+      InstructionOperand left_operands[4];
+      AddressingMode addressing_mode = g.GetEffectiveAddressMemoryOperand(
+          left, left_operands, &left_operands_count);
+      opcode |= AddressingModeField::encode(addressing_mode);
+      VisitCompare(selector, opcode, left_operands_count, left_operands,
+                   g.UseImmediate(right), cont);
+    } else {
+      VisitCompare(selector, opcode, g.Use(left), g.UseImmediate(right), cont);
+    }
   } else {
     VisitCompare(selector, opcode, left, right, cont,
-                 node->op()->HasProperty(Operator::kCommutative));
+                 node->op()->HasProperty(Operator::kCommutative), node);
   }
 }
 
 
 void VisitWordCompare(InstructionSelector* selector, Node* node,
                       FlagsContinuation* cont) {
   IA32OperandGenerator g(selector);
   Int32BinopMatcher m(node);
   if (m.left().IsLoad() && m.right().IsLoadStackPointer()) {
     LoadMatcher<ExternalReferenceMatcher> mleft(m.left().node());
@@ skipping 293 matching lines @@
              MachineOperatorBuilder::kFloat64RoundTruncate |
              MachineOperatorBuilder::kFloat32RoundTiesEven |
              MachineOperatorBuilder::kFloat64RoundTiesEven;
   }
   return flags;
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8