Reviewers: William Hesse,

Description:
Move swapping of operands for commutative binary operations to the lithium IR.

There is no reason to have this helper functions already at the hydrogen level.

At the hydrogen level commutativity depends on the representation type and the
information is only used to swap the operands when we can emit an immediate
operand in the code generator.



Please review this at http://codereview.chromium.org/6328009/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
  M     src/arm/lithium-arm.cc
  M     src/hydrogen-instructions.h
  M     src/ia32/lithium-ia32.cc
  M     src/x64/lithium-x64.cc


Index: src/arm/lithium-arm.cc
===================================================================
--- src/arm/lithium-arm.cc      (revision 6458)
+++ src/arm/lithium-arm.cc      (working copy)
@@ -730,8 +730,12 @@
   ASSERT(instr->left()->representation().IsInteger32());
   ASSERT(instr->right()->representation().IsInteger32());

-  LOperand* left = UseRegisterAtStart(instr->LeastConstantOperand());
-  LOperand* right = UseOrConstantAtStart(instr->MostConstantOperand());
+  LOperand* left = UseRegisterAtStart(instr->left()->IsConstant()
+                                      ? instr->right()
+                                      : instr->left());
+  LOperand* right = UseOrConstantAtStart(instr->left()->IsConstant()
+                                         ? instr->left()
+                                         : instr->right());
   return DefineSameAsFirst(new LBitI(op, left, right));
 }

@@ -1274,8 +1278,12 @@
   if (instr->representation().IsInteger32()) {
     ASSERT(instr->left()->representation().IsInteger32());
     ASSERT(instr->right()->representation().IsInteger32());
-    LOperand* left = UseRegisterAtStart(instr->LeastConstantOperand());
-    LOperand* right = UseOrConstant(instr->MostConstantOperand());
+    LOperand* left = UseRegisterAtStart(instr->left()->IsConstant()
+                                        ? instr->right()
+                                        : instr->left());
+    LOperand* right = UseOrConstant(instr->left()->IsConstant()
+                                    ? instr->left()
+                                    : instr->right());
     LOperand* temp = NULL;
     if (instr->CheckFlag(HValue::kBailoutOnMinusZero)) {
       temp = TempRegister();
@@ -1314,8 +1322,12 @@
   if (instr->representation().IsInteger32()) {
     ASSERT(instr->left()->representation().IsInteger32());
     ASSERT(instr->right()->representation().IsInteger32());
-    LOperand* left = UseRegisterAtStart(instr->LeastConstantOperand());
-    LOperand* right = UseOrConstantAtStart(instr->MostConstantOperand());
+    LOperand* left = UseRegisterAtStart(instr->left()->IsConstant()
+                                        ? instr->right()
+                                        : instr->left());
+    LOperand* right = UseOrConstantAtStart(instr->left()->IsConstant()
+                                           ? instr->left()
+                                           : instr->right());
     LAddI* add = new LAddI(left, right);
     LInstruction* result = DefineSameAsFirst(add);
     if (instr->CheckFlag(HValue::kCanOverflow)) {
Index: src/hydrogen-instructions.h
===================================================================
--- src/hydrogen-instructions.h (revision 6458)
+++ src/hydrogen-instructions.h (working copy)
@@ -1871,19 +1871,6 @@
   HValue* left() const { return OperandAt(0); }
   HValue* right() const { return OperandAt(1); }

-  // TODO(kasperl): Move these helpers to the IA-32 Lithium
-  // instruction sequence builder.
-  HValue* LeastConstantOperand() const {
-    if (IsCommutative() && left()->IsConstant()) return right();
-    return left();
-  }
-  HValue* MostConstantOperand() const {
-    if (IsCommutative() && left()->IsConstant()) return left();
-    return right();
-  }
-
-  virtual bool IsCommutative() const { return false; }
-
   virtual void PrintDataTo(StringStream* stream) const;
   virtual int OperandCount() const { return operands_.length(); }
   virtual HValue* OperandAt(int index) const { return operands_[index]; }
@@ -2335,12 +2322,6 @@
     SetFlag(kCanOverflow);
   }

-  // Add is only commutative if two integer values are added and not if two
-  // tagged values are added (because it might be a String concatenation).
-  virtual bool IsCommutative() const {
-    return !representation().IsTagged();
-  }
-
   virtual HValue* EnsureAndPropagateNotMinusZero(BitVector* visited);

   virtual HType CalculateInferredType() const;
@@ -2375,11 +2356,6 @@

   virtual HValue* EnsureAndPropagateNotMinusZero(BitVector* visited);

- // Only commutative if it is certain that not two objects are multiplicated.
-  virtual bool IsCommutative() const {
-    return !representation().IsTagged();
-  }
-
   DECLARE_CONCRETE_INSTRUCTION(Mul, "mul")

  protected:
@@ -2423,7 +2399,6 @@
   HBitAnd(HValue* left, HValue* right)
       : HBitwiseBinaryOperation(left, right) { }

-  virtual bool IsCommutative() const { return true; }
   virtual HType CalculateInferredType() const;

   DECLARE_CONCRETE_INSTRUCTION(BitAnd, "bit_and")
@@ -2438,7 +2413,6 @@
   HBitXor(HValue* left, HValue* right)
       : HBitwiseBinaryOperation(left, right) { }

-  virtual bool IsCommutative() const { return true; }
   virtual HType CalculateInferredType() const;

   DECLARE_CONCRETE_INSTRUCTION(BitXor, "bit_xor")
@@ -2450,7 +2424,6 @@
   HBitOr(HValue* left, HValue* right)
       : HBitwiseBinaryOperation(left, right) { }

-  virtual bool IsCommutative() const { return true; }
   virtual HType CalculateInferredType() const;

   DECLARE_CONCRETE_INSTRUCTION(BitOr, "bit_or")
Index: src/ia32/lithium-ia32.cc
===================================================================
--- src/ia32/lithium-ia32.cc    (revision 6458)
+++ src/ia32/lithium-ia32.cc    (working copy)
@@ -733,8 +733,12 @@
     ASSERT(instr->left()->representation().IsInteger32());
     ASSERT(instr->right()->representation().IsInteger32());

-    LOperand* left = UseRegisterAtStart(instr->LeastConstantOperand());
-    LOperand* right = UseOrConstantAtStart(instr->MostConstantOperand());
+    LOperand* left = UseRegisterAtStart(instr->left()->IsConstant()
+                                        ? instr->right()
+                                        : instr->left());
+    LOperand* right = UseOrConstantAtStart(instr->left()->IsConstant()
+                                           ? instr->left()
+                                           : instr->right());
     return DefineSameAsFirst(new LBitI(op, left, right));
   } else {
     ASSERT(instr->representation().IsTagged());
@@ -1306,8 +1310,12 @@
   if (instr->representation().IsInteger32()) {
     ASSERT(instr->left()->representation().IsInteger32());
     ASSERT(instr->right()->representation().IsInteger32());
-    LOperand* left = UseRegisterAtStart(instr->LeastConstantOperand());
-    LOperand* right = UseOrConstant(instr->MostConstantOperand());
+    LOperand* left = UseRegisterAtStart(instr->left()->IsConstant()
+                                        ? instr->right()
+                                        : instr->left());
+    LOperand* right = UseOrConstant(instr->left()->IsConstant()
+                                    ? instr->left()
+                                    : instr->right());
     LOperand* temp = NULL;
     if (instr->CheckFlag(HValue::kBailoutOnMinusZero)) {
       temp = TempRegister();
@@ -1348,8 +1356,12 @@
   if (instr->representation().IsInteger32()) {
     ASSERT(instr->left()->representation().IsInteger32());
     ASSERT(instr->right()->representation().IsInteger32());
-    LOperand* left = UseRegisterAtStart(instr->LeastConstantOperand());
-    LOperand* right = UseOrConstantAtStart(instr->MostConstantOperand());
+    LOperand* left = UseRegisterAtStart(instr->left()->IsConstant()
+                                        ? instr->right()
+                                        : instr->left());
+    LOperand* right = UseOrConstantAtStart(instr->left()->IsConstant()
+                                           ? instr->left()
+                                           : instr->right());
     LAddI* add = new LAddI(left, right);
     LInstruction* result = DefineSameAsFirst(add);
     if (instr->CheckFlag(HValue::kCanOverflow)) {
Index: src/x64/lithium-x64.cc
===================================================================
--- src/x64/lithium-x64.cc      (revision 6458)
+++ src/x64/lithium-x64.cc      (working copy)
@@ -1183,8 +1183,12 @@
   if (instr->representation().IsInteger32()) {
     ASSERT(instr->left()->representation().IsInteger32());
     ASSERT(instr->right()->representation().IsInteger32());
-    LOperand* left = UseRegisterAtStart(instr->LeastConstantOperand());
-    LOperand* right = UseOrConstantAtStart(instr->MostConstantOperand());
+    LOperand* left = UseRegisterAtStart(instr->left()->IsConstant()
+                                        ? instr->right()
+                                        : instr->left());
+    LOperand* right = UseOrConstantAtStart(instr->left()->IsConstant()
+                                           ? instr->left()
+                                           : instr->right());
     LAddI* add = new LAddI(left, right);
     LInstruction* result = DefineSameAsFirst(add);
     if (instr->CheckFlag(HValue::kCanOverflow)) {


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to