[turbofan] IA: support better left operand for commutative binops

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/compiler/instruction-selector-impl.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties-inl.h"
 
 namespace v8 {
 namespace internal {
@@ skipping 19 matching lines @@
       case IrOpcode::kHeapConstant: {
         // Constants in new space cannot be used as immediates in V8 because
         // the GC does not scan code objects when collecting the new generation.
         Unique<HeapObject> value = OpParameter<Unique<HeapObject> >(node);
         return !isolate()->heap()->InNewSpace(*value.handle());
       }
       default:
         return false;
     }
   }
+
+  bool CanBeBetterLeftOperand(Node* node) { return !selector()->IsUsed(node); }

[Benedikt Meurer, 2014/09/23 04:56:31]

Nit: method should be const.

[Weiliang, 2014/09/23 05:22:12]

Done.

 };
 
 
 void InstructionSelector::VisitLoad(Node* node) {
   MachineType rep = RepresentationOf(OpParameter<LoadRepresentation>(node));
   MachineType typ = TypeOf(OpParameter<LoadRepresentation>(node));
   IA32OperandGenerator g(this);
   Node* base = node->InputAt(0);
   Node* index = node->InputAt(1);
 
@@ skipping 109 matching lines @@
   }
   // TODO(turbofan): addressing modes [r+r*{2,4,8}+K]
 }
 
 
 // Shared routine for multiple binary operations.
 static void VisitBinop(InstructionSelector* selector, Node* node,
                        InstructionCode opcode, FlagsContinuation* cont) {
   IA32OperandGenerator g(selector);
   Int32BinopMatcher m(node);
+  Node* left = m.left().node();
+  Node* right = m.right().node();
   InstructionOperand* inputs[4];
   size_t input_count = 0;
   InstructionOperand* outputs[2];
   size_t output_count = 0;
 
   // TODO(turbofan): match complex addressing modes.
-  // TODO(turbofan): if commutative, pick the non-live-in operand as the left as
-  // this might be the last use and therefore its register can be reused.
-  if (g.CanBeImmediate(m.right().node())) {
-    inputs[input_count++] = g.Use(m.left().node());
-    inputs[input_count++] = g.UseImmediate(m.right().node());
+  if (g.CanBeImmediate(right)) {
+    inputs[input_count++] = g.Use(left);
+    inputs[input_count++] = g.UseImmediate(right);
   } else {
-    inputs[input_count++] = g.UseRegister(m.left().node());
-    inputs[input_count++] = g.Use(m.right().node());
+    if (node->op()->HasProperty(Operator::kCommutative) &&
+        g.CanBeBetterLeftOperand(right)) {
+      std::swap(left, right);
+    }
+    inputs[input_count++] = g.UseRegister(left);
+    inputs[input_count++] = g.Use(right);
   }
 
   if (cont->IsBranch()) {
     inputs[input_count++] = g.Label(cont->true_block());
     inputs[input_count++] = g.Label(cont->false_block());
   }
 
   outputs[output_count++] = g.DefineSameAsFirst(node);
   if (cont->IsSet()) {
     // TODO(turbofan): Use byte register here.
@@ skipping 96 matching lines @@
   if (m.left().Is(0)) {
     Emit(kIA32Neg, g.DefineSameAsFirst(node), g.Use(m.right().node()));
   } else {
     VisitBinop(this, node, kIA32Sub);
   }
 }
 
 
 void InstructionSelector::VisitInt32Mul(Node* node) {
   IA32OperandGenerator g(this);
-  Node* left = node->InputAt(0);
-  Node* right = node->InputAt(1);
+  Int32BinopMatcher m(node);
+  Node* left = m.left().node();
+  Node* right = m.right().node();
   if (g.CanBeImmediate(right)) {
     Emit(kIA32Imul, g.DefineAsRegister(node), g.Use(left),
          g.UseImmediate(right));
-  } else if (g.CanBeImmediate(left)) {
-    Emit(kIA32Imul, g.DefineAsRegister(node), g.Use(right),
-         g.UseImmediate(left));
   } else {
-    // TODO(turbofan): select better left operand.
+    if (g.CanBeBetterLeftOperand(right)) {
+      std::swap(left, right);
+    }
     Emit(kIA32Imul, g.DefineSameAsFirst(node), g.UseRegister(left),

[Benedikt Meurer, 2014/09/23 04:57:37]

There should be no need to use DefineSameAsFirst with imul.

[Weiliang, 2014/09/23 05:22:12]

My thought here is that three operand imul requires an imm input. If there is no
imm input, we can only use the two-operands/single-operand imul. That is why I
still use DefineSameAsFirst here.

          g.Use(right));
   }
 }
 
 
 static inline void VisitDiv(InstructionSelector* selector, Node* node,
                             ArchOpcode opcode) {
   IA32OperandGenerator g(selector);
   InstructionOperand* temps[] = {g.TempRegister(edx)};
   size_t temp_count = arraysize(temps);
@@ skipping 228 matching lines @@
   call_instr->MarkAsCall();
   if (deoptimization != NULL) {
     DCHECK(continuation != NULL);
     call_instr->MarkAsControl();
   }
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8