Fix problem with GenericBinaryOperationStub::GenerateCall for a Smi

left operand.  For non-commutative operations the right operand could
be overwritten with the Smi left operand.

We need better testing of all of these cases.  We will add more test
cases as a separate commit.
Review URL: http://codereview.chromium.org/598059

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3834 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index a6652618501a7b94312c576f5459a363935f4eca..5caac2e041db84a67150f614d40154540f0f7c4b 100644 (file)
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -7036,6 +7036,8 @@ void GenericBinaryOpStub::GenerateCall(
         }
       } else if (left.is(left_arg)) {
         __ mov(right_arg, right);
+      } else if (right.is(right_arg)) {
+        __ mov(left_arg, left);
       } else if (left.is(right_arg)) {
         if (IsOperationCommutative()) {
           __ mov(left_arg, right);
@@ -7054,8 +7056,6 @@ void GenericBinaryOpStub::GenerateCall(
           __ mov(right_arg, right);
           __ mov(left_arg, left);
         }
-      } else if (right.is(right_arg)) {
-        __ mov(left_arg, left);
       } else {
         // Order of moves is not important.
         __ mov(left_arg, left);
@@ -7091,6 +7091,10 @@ void GenericBinaryOpStub::GenerateCall(
       __ mov(left_arg, Immediate(right));
       SetArgsReversed();
     } else {
+      // For non-commutative operations, left and right_arg might be
+      // the same register.  Therefore, the order of the moves is
+      // important here in order to not overwrite left before moving
+      // it to left_arg.
       __ mov(left_arg, left);
       __ mov(right_arg, Immediate(right));
     }
@@ -7123,8 +7127,12 @@ void GenericBinaryOpStub::GenerateCall(
       __ mov(right_arg, Immediate(left));
       SetArgsReversed();
     } else {
-      __ mov(left_arg, Immediate(left));
+      // For non-commutative operations, right and left_arg might be
+      // the same register.  Therefore, the order of the moves is
+      // important here in order to not overwrite right before moving
+      // it to right_arg.
       __ mov(right_arg, right);
+      __ mov(left_arg, Immediate(left));
     }
     // Update flags to indicate that arguments are in registers.
     SetArgsInRegisters();

diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc
index 90a92cc8e48afa5e8a509bd9fa32ca5eca5d6970..cb0495b031e571fed390616f74582a023e4412fb 100644 (file)
--- a/src/x64/codegen-x64.cc
+++ b/src/x64/codegen-x64.cc
@@ -8051,6 +8051,8 @@ void GenericBinaryOpStub::GenerateCall(
         }
       } else if (left.is(left_arg)) {
         __ movq(right_arg, right);
+      } else if (right.is(right_arg)) {
+        __ movq(left_arg, left);
       } else if (left.is(right_arg)) {
         if (IsOperationCommutative()) {
           __ movq(left_arg, right);
@@ -8069,8 +8071,6 @@ void GenericBinaryOpStub::GenerateCall(
           __ movq(right_arg, right);
           __ movq(left_arg, left);
         }
-      } else if (right.is(right_arg)) {
-        __ movq(left_arg, left);
       } else {
         // Order of moves is not important.
         __ movq(left_arg, left);
@@ -8106,6 +8106,10 @@ void GenericBinaryOpStub::GenerateCall(
       __ Move(left_arg, right);
       SetArgsReversed();
     } else {
+      // For non-commutative operations, left and right_arg might be
+      // the same register.  Therefore, the order of the moves is
+      // important here in order to not overwrite left before moving
+      // it to left_arg.
       __ movq(left_arg, left);
       __ Move(right_arg, right);
     }
@@ -8138,8 +8142,12 @@ void GenericBinaryOpStub::GenerateCall(
       __ Move(right_arg, left);
       SetArgsReversed();
     } else {
-      __ Move(left_arg, left);
+      // For non-commutative operations, right and left_arg might be
+      // the same register.  Therefore, the order of the moves is
+      // important here in order to not overwrite right before moving
+      // it to right_arg.
       __ movq(right_arg, right);
+      __ Move(left_arg, left);
     }
     // Update flags to indicate that arguments are in registers.
     SetArgsInRegisters();