Use registers to pass arguments to GenericBinaryOpStub....

src/ia32/codegen-ia32.cc

 // Copyright 2006-2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 723 matching lines @@
   frame_->Push(&value);  // Undo the Pop() from above.
   ToBooleanStub stub;
   Result temp = frame_->CallStub(&stub, 1);
   // Convert the result to a condition code.
   __ test(temp.reg(), Operand(temp.reg()));
   temp.Unuse();
   dest->Split(not_equal);
 }
 
 
+enum ArgLocation {

[Kevin Millikin (Chromium), 2010/01/25 16:27:44]

It seems like this should be a member of FloatingPointHelper.

[Vladislav Kaznacheev, 2010/01/25 17:44:53]

Done.

+  ARGS_ON_STACK,
+  ARGS_IN_REGISTERS
+};
+
+
 class FloatingPointHelper : public AllStatic {
  public:
   // Code pattern for loading a floating point value. Input value must
   // be either a smi or a heap number object (fp value). Requirements:
   // operand in register number. Returns operand as floating point number
   // on FPU stack.
   static void LoadFloatOperand(MacroAssembler* masm, Register number);
   // Code pattern for loading floating point values. Input values must
   // be either smi or heap number objects (fp values). Requirements:
-  // operand_1 on TOS+1 , operand_2 on TOS+2; Returns operands as
-  // floating point numbers on FPU stack.
-  static void LoadFloatOperands(MacroAssembler* masm, Register scratch);
+  // operand_1 on TOS+1 or in edx, operand_2 on TOS+2 or in eax.
+  // Returns operands as floating point numbers on FPU stack.
+  static void LoadFloatOperands(MacroAssembler* masm,
+                                Register scratch,
+                                ArgLocation arg_location = ARGS_ON_STACK);
+
+  // Similar to LoadFloatOperand but assumes that both operands are smis.
+  // Accepts operands on stack or in eax, ebx.
+  static void LoadFloatSmis(MacroAssembler* masm,
+                            Register scratch,
+                            ArgLocation arg_location);
+
   // Test if operands are smi or number objects (fp). Requirements:
   // operand_1 in eax, operand_2 in edx; falls through on float
   // operands, jumps to the non_float label otherwise.
   static void CheckFloatOperands(MacroAssembler* masm,
                                  Label* non_float,
                                  Register scratch);
   // Takes the operands in edx and eax and loads them as integers in eax
   // and ecx.
   static void LoadAsIntegers(MacroAssembler* masm,
                              bool use_sse3,
                              Label* operand_conversion_failure);
   // Test if operands are numbers (smi or HeapNumber objects), and load
   // them into xmm0 and xmm1 if they are.  Jump to label not_numbers if
   // either operand is not a number.  Operands are in edx and eax.
   // Leaves operands unchanged.
   static void LoadSse2Operands(MacroAssembler* masm, Label* not_numbers);
+
+  // Similar to LoadSse2Operands but assumes that both operands are smis.
+  // Accepts operands on stack or in eax, ebx.
+  static void LoadSse2Smis(MacroAssembler* masm,
+                           Register scratch,
+                           ArgLocation arg_location);
 };
 
 
 const char* GenericBinaryOpStub::GetName() {
   if (name_ != NULL) return name_;
   const int kMaxNameLength = 100;
   name_ = Bootstrapper::AllocateAutoDeletedArray(kMaxNameLength);
   if (name_ == NULL) return "OOM";
   const char* op_name = Token::Name(op_);
   const char* overwrite_name;
@@ skipping 542 matching lines @@
   } else {
     __ mov(answer.reg(), left->reg());
     __ or_(answer.reg(), Operand(right->reg()));
     ASSERT(kSmiTag == 0);  // Adjust test if not the case.
     __ test(answer.reg(), Immediate(kSmiTagMask));
   }
   deferred->Branch(not_zero);
   __ mov(answer.reg(), left->reg());
   switch (op) {
     case Token::ADD:
-      __ add(answer.reg(), Operand(right->reg()));  // Add optimistically.
+      __ add(answer.reg(), Operand(right->reg()));
       deferred->Branch(overflow);
       break;
 
     case Token::SUB:
-      __ sub(answer.reg(), Operand(right->reg()));  // Subtract optimistically.
+      __ sub(answer.reg(), Operand(right->reg()));
       deferred->Branch(overflow);
       break;
 
     case Token::MUL: {
       // If the smi tag is 0 we can just leave the tag on one operand.
       ASSERT(kSmiTag == 0);  // Adjust code below if not the case.
       // Remove smi tag from the left operand (but keep sign).
       // Left-hand operand has been copied into answer.
       __ SmiUntag(answer.reg());
       // Do multiplication of smis, leaving result in answer.
@@ skipping 5699 matching lines @@
     SetArgsInRegisters();
     __ IncrementCounter(&Counters::generic_binary_stub_calls_regs, 1);
   }
 
   // Call the stub.
   __ CallStub(this);
 }
 
 
 void GenericBinaryOpStub::GenerateSmiCode(MacroAssembler* masm, Label* slow) {
+  if (HasArgumentsInRegisters()) {
+    __ mov(ebx, eax);
+    __ mov(eax, edx);
+  } else {
+    __ mov(ebx, Operand(esp, 1 * kPointerSize));
+    __ mov(eax, Operand(esp, 2 * kPointerSize));
+  }
+
+  Label not_smis, not_smis_or_overflow, not_smis_undo_optimistic;
+  Label use_fp_on_smis, done;
+
   // Perform fast-case smi code for the operation (eax <op> ebx) and
   // leave result in register eax.
 
   // Prepare the smi check of both operands by or'ing them together
   // before checking against the smi mask.
   __ mov(ecx, Operand(ebx));
   __ or_(ecx, Operand(eax));
 
   switch (op_) {
     case Token::ADD:
       __ add(eax, Operand(ebx));  // add optimistically
-      __ j(overflow, slow, not_taken);
+      __ j(overflow, &not_smis_or_overflow, not_taken);
       break;
 
     case Token::SUB:
       __ sub(eax, Operand(ebx));  // subtract optimistically
-      __ j(overflow, slow, not_taken);
+      __ j(overflow, &not_smis_or_overflow, not_taken);
       break;
 
     case Token::DIV:
     case Token::MOD:
       // Sign extend eax into edx:eax.
       __ cdq();
       // Check for 0 divisor.
       __ test(ebx, Operand(ebx));
-      __ j(zero, slow, not_taken);
+      __ j(zero, &not_smis_or_overflow, not_taken);
       break;
 
     default:
       // Fall-through to smi check.
       break;
   }
 
   // Perform the actual smi check.
   ASSERT(kSmiTag == 0);  // adjust zero check if not the case
   __ test(ecx, Immediate(kSmiTagMask));
-  __ j(not_zero, slow, not_taken);
+  __ j(not_zero, &not_smis_undo_optimistic, not_taken);
 
   switch (op_) {
     case Token::ADD:
     case Token::SUB:
       // Do nothing here.
       break;
 
     case Token::MUL:
       // If the smi tag is 0 we can just leave the tag on one operand.
       ASSERT(kSmiTag == 0);  // adjust code below if not the case
       // Remove tag from one of the operands (but keep sign).
       __ SmiUntag(eax);
       // Do multiplication.
       __ imul(eax, Operand(ebx));  // multiplication of smis; result in eax
       // Go slow on overflows.
-      __ j(overflow, slow, not_taken);
+      __ j(overflow, &use_fp_on_smis, not_taken);
       // Check for negative zero result.
-      __ NegativeZeroTest(eax, ecx, slow);  // use ecx = x | y
+      __ NegativeZeroTest(eax, ecx, &use_fp_on_smis);  // use ecx = x | y
       break;
 
     case Token::DIV:
       // Divide edx:eax by ebx.
       __ idiv(ebx);
       // Check for the corner case of dividing the most negative smi
       // by -1. We cannot use the overflow flag, since it is not set
       // by idiv instruction.
       ASSERT(kSmiTag == 0 && kSmiTagSize == 1);
       __ cmp(eax, 0x40000000);
-      __ j(equal, slow);
+      __ j(equal, &use_fp_on_smis);
       // Check for negative zero result.
-      __ NegativeZeroTest(eax, ecx, slow);  // use ecx = x | y
+      __ NegativeZeroTest(eax, ecx, &use_fp_on_smis);  // use ecx = x | y
       // Check that the remainder is zero.
       __ test(edx, Operand(edx));
-      __ j(not_zero, slow);
+      __ j(not_zero, &use_fp_on_smis);
       // Tag the result and store it in register eax.
       __ SmiTag(eax);
       break;
 
     case Token::MOD:
       // Divide edx:eax by ebx.
       __ idiv(ebx);
       // Check for negative zero result.
       __ NegativeZeroTest(edx, ecx, slow);  // use ecx = x | y
       // Move remainder to register eax.
@@ skipping 34 matching lines @@
           // - 0x40000000: this number would convert to negative when
           // Smi tagging these two cases can only happen with shifts
           // by 0 or 1 when handed a valid smi.
           __ test(eax, Immediate(0xc0000000));
           __ j(not_zero, slow, not_taken);
           break;
         case Token::SHL:
           __ shl_cl(eax);
           // Check that the *signed* result fits in a smi.
           __ cmp(eax, 0xc0000000);
-          __ j(sign, slow, not_taken);
+          __ j(sign, &use_fp_on_smis, not_taken);
           break;
         default:
           UNREACHABLE();
       }
       // Tag the result and store it in register eax.
       __ SmiTag(eax);
       break;
 
     default:
       UNREACHABLE();
       break;
   }
+  GenerateReturn(masm);
+
+  __ bind(&not_smis_or_overflow);
+  // Revert optimistic operation.
+  switch (op_) {
+    case Token::ADD: __ sub(eax, Operand(ebx)); break;
+    case Token::SUB: __ add(eax, Operand(ebx)); break;
+    default: break;
+  }
+  ASSERT(kSmiTag == 0);  // Adjust zero check if not the case.
+  __ test(ecx, Immediate(kSmiTagMask));
+  __ j(not_zero, &not_smis, not_taken);
+  //  Correct operand values are in eax, ebx at this point.
+
+  __ bind(&use_fp_on_smis);
+  // Both operands are known to be SMIs but the result does not fit into a SMI.
+  switch (op_) {
+    case Token::ADD:
+    case Token::SUB:
+    case Token::MUL:
+    case Token::DIV: {
+      Label after_alloc_failure;
+
+      ArgLocation arg_location =
+          (op_ == Token::ADD || op_ == Token::SUB) ?
+          ARGS_IN_REGISTERS :
+          ARGS_ON_STACK;
+
+      __ AllocateHeapNumber(
+          edx,
+          ecx,
+          no_reg,
+          arg_location == ARGS_IN_REGISTERS ? &after_alloc_failure : slow);
+
+      if (CpuFeatures::IsSupported(SSE2)) {
+        CpuFeatures::Scope use_sse2(SSE2);
+        FloatingPointHelper::LoadSse2Smis(masm, ecx, arg_location);
+        switch (op_) {
+          case Token::ADD: __ addsd(xmm0, xmm1); break;
+          case Token::SUB: __ subsd(xmm0, xmm1); break;
+          case Token::MUL: __ mulsd(xmm0, xmm1); break;
+          case Token::DIV: __ divsd(xmm0, xmm1); break;
+          default: UNREACHABLE();
+        }
+        __ movdbl(FieldOperand(edx, HeapNumber::kValueOffset), xmm0);
+      } else {  // SSE2 not available, use FPU.
+        FloatingPointHelper::LoadFloatSmis(masm, ecx, arg_location);
+        switch (op_) {
+          case Token::ADD: __ faddp(1); break;
+          case Token::SUB: __ fsubp(1); break;
+          case Token::MUL: __ fmulp(1); break;
+          case Token::DIV: __ fdivp(1); break;
+          default: UNREACHABLE();
+        }
+        __ fstp_d(FieldOperand(edx, HeapNumber::kValueOffset));
+      }
+      __ mov(eax, edx);
+      GenerateReturn(masm);
+
+      if (HasArgumentsInRegisters()) {

[Kevin Millikin (Chromium), 2010/01/25 16:27:44]

Is this right?  What if op_ == Token::ADD or Token::SUB and
!HasArgumentsInRegisters().  Won't the heap allocation target
&after_alloc_failure on a failure, but the label will never be bound?

[Vladislav Kaznacheev, 2010/01/25 17:44:53]

This was a bug. Fixed.
On 2010/01/25 16:27:44, Kevin Millikin wrote:

+        __ bind(&after_alloc_failure);
+        __ mov(edx, eax);
+        __ mov(eax, ebx);
+        __ jmp(slow);
+      }
+    }

[Kevin Millikin (Chromium), 2010/01/25 16:27:44]

Missing break would be a problem (at least dead code generation) if code was
added to the fall through or clauses were reordered.

Add break or add a "Fall through" comment.  (Explicit break is better).

[Vladislav Kaznacheev, 2010/01/25 17:44:53]

Done.

+    case Token::BIT_OR:
+    case Token::BIT_AND:
+    case Token::BIT_XOR:
+    case Token::SAR:
+      // Do nothing here as these operations always succeed on a pair of smis.
+      break;
+
+    case Token::MOD:
+    case Token::SHR:
+      // Do nothing here as these go directly to runtime.
+      break;
+
+    case Token::SHL: {
+      __ AllocateHeapNumber(ebx, ecx, edx, slow);
+      // Store the result in the HeapNumber and return.
+      if (CpuFeatures::IsSupported(SSE2)) {
+        CpuFeatures::Scope use_sse2(SSE2);
+        __ cvtsi2sd(xmm0, Operand(eax));
+        __ movdbl(FieldOperand(ebx, HeapNumber::kValueOffset), xmm0);
+      } else {
+        __ mov(Operand(esp, 1 * kPointerSize), eax);
+        __ fild_s(Operand(esp, 1 * kPointerSize));
+        __ fstp_d(FieldOperand(ebx, HeapNumber::kValueOffset));
+      }
+      __ mov(eax, ebx);
+      GenerateReturn(masm);
+      break;
+    }
+
+    default: UNREACHABLE(); break;
+  }
+
+  __ bind(&not_smis_undo_optimistic);
+  switch (op_) {
+    case Token::ADD: __ sub(eax, Operand(ebx)); break;
+    case Token::SUB: __ add(eax, Operand(ebx)); break;
+    default: break;
+  }
+
+  __ bind(&not_smis);
+  __ mov(edx, eax);
+  __ mov(eax, ebx);
+
+  __ bind(&done);
 }
 
 
 void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
   Label call_runtime;
 
   __ IncrementCounter(&Counters::generic_binary_stub_calls, 1);
 
   // Generate fast case smi code if requested. This flag is set when the fast
   // case smi code is not generated by the caller. Generating it here will speed
   // up common operations.
   if (HasSmiCodeInStub()) {
-    Label slow;
-    __ mov(ebx, Operand(esp, 1 * kPointerSize));
-    __ mov(eax, Operand(esp, 2 * kPointerSize));
-    GenerateSmiCode(masm, &slow);
-    GenerateReturn(masm);
-    // Too bad. The fast case smi code didn't succeed.
-    __ bind(&slow);
+    GenerateSmiCode(masm, &call_runtime);
+  } else if (op_ != Token::MOD) {  // MOD goes straight to runtime.
+    GenerateLoadArguments(masm);
   }
 
-  // Make sure the arguments are in edx and eax.
-  GenerateLoadArguments(masm);
-
   // Floating point case.
   switch (op_) {
     case Token::ADD:
     case Token::SUB:
     case Token::MUL:
     case Token::DIV: {
-      // eax: y
-      // edx: x
-
       if (CpuFeatures::IsSupported(SSE2)) {
         CpuFeatures::Scope use_sse2(SSE2);
         FloatingPointHelper::LoadSse2Operands(masm, &call_runtime);
 
         switch (op_) {
           case Token::ADD: __ addsd(xmm0, xmm1); break;
           case Token::SUB: __ subsd(xmm0, xmm1); break;
           case Token::MUL: __ mulsd(xmm0, xmm1); break;
           case Token::DIV: __ divsd(xmm0, xmm1); break;
           default: UNREACHABLE();
         }
-        // Allocate a heap number, if needed.
-        Label skip_allocation;
-        switch (mode_) {
-          case OVERWRITE_LEFT:
-            __ mov(eax, Operand(edx));
-            // Fall through!
-          case OVERWRITE_RIGHT:
-            // If the argument in eax is already an object, we skip the
-            // allocation of a heap number.
-            __ test(eax, Immediate(kSmiTagMask));
-            __ j(not_zero, &skip_allocation, not_taken);
-            // Fall through!
-          case NO_OVERWRITE: {
-            // Allocate a heap number for the result. Keep eax and edx intact
-            // for the possible runtime call.
-            __ AllocateHeapNumber(ebx, ecx, no_reg, &call_runtime);
-            // Now eax can be overwritten losing one of the arguments as we are
-            // now done and will not need it any more.
-            __ mov(eax, ebx);
-            __ bind(&skip_allocation);
-            break;
-          }
-          default: UNREACHABLE();
-        }
+        GenerateHeapResultAllocation(masm, &call_runtime);
         __ movdbl(FieldOperand(eax, HeapNumber::kValueOffset), xmm0);
         GenerateReturn(masm);
       } else {  // SSE2 not available, use FPU.
         FloatingPointHelper::CheckFloatOperands(masm, &call_runtime, ebx);
-        // Allocate a heap number, if needed.
-        Label skip_allocation;
-        switch (mode_) {
-          case OVERWRITE_LEFT:
-            __ mov(eax, Operand(edx));
-            // Fall through!
-          case OVERWRITE_RIGHT:
-            // If the argument in eax is already an object, we skip the
-            // allocation of a heap number.
-            __ test(eax, Immediate(kSmiTagMask));
-            __ j(not_zero, &skip_allocation, not_taken);
-            // Fall through!
-          case NO_OVERWRITE:
-            // Allocate a heap number for the result. Keep eax and edx intact
-            // for the possible runtime call.
-            __ AllocateHeapNumber(ebx, ecx, no_reg, &call_runtime);
-            // Now eax can be overwritten losing one of the arguments as we are
-            // now done and will not need it any more.
-            __ mov(eax, ebx);
-            __ bind(&skip_allocation);
-            break;
-          default: UNREACHABLE();
-        }
-        FloatingPointHelper::LoadFloatOperands(masm, ecx);
-
+        FloatingPointHelper::LoadFloatOperands(masm, ecx, ARGS_IN_REGISTERS);
         switch (op_) {
           case Token::ADD: __ faddp(1); break;
           case Token::SUB: __ fsubp(1); break;
           case Token::MUL: __ fmulp(1); break;
           case Token::DIV: __ fdivp(1); break;
           default: UNREACHABLE();
         }
+        Label after_alloc_failure;
+        GenerateHeapResultAllocation(masm, &after_alloc_failure);
         __ fstp_d(FieldOperand(eax, HeapNumber::kValueOffset));
         GenerateReturn(masm);
+        __ bind(&after_alloc_failure);
+        __ ffree();
+        __ jmp(&call_runtime);
       }
     }
     case Token::MOD: {
       // For MOD we go directly to runtime in the non-smi case.
       break;
     }
     case Token::BIT_OR:
     case Token::BIT_AND:
     case Token::BIT_XOR:
     case Token::SAR:
     case Token::SHL:
     case Token::SHR: {
-      Label non_smi_result, skip_allocation;
-      Label operand_conversion_failure;
-      FloatingPointHelper::LoadAsIntegers(
-        masm,
-        use_sse3_,
-        &operand_conversion_failure);
+      Label non_smi_result;
+      FloatingPointHelper::LoadAsIntegers(masm, use_sse3_, &call_runtime);
       switch (op_) {
         case Token::BIT_OR:  __ or_(eax, Operand(ecx)); break;
         case Token::BIT_AND: __ and_(eax, Operand(ecx)); break;
         case Token::BIT_XOR: __ xor_(eax, Operand(ecx)); break;
         case Token::SAR: __ sar_cl(eax); break;
         case Token::SHL: __ shl_cl(eax); break;
         case Token::SHR: __ shr_cl(eax); break;
         default: UNREACHABLE();
       }
       if (op_ == Token::SHR) {
         // Check if result is non-negative and fits in a smi.
         __ test(eax, Immediate(0xc0000000));
-        __ j(not_zero, &non_smi_result);
+        __ j(not_zero, &call_runtime);
       } else {
         // Check if result fits in a smi.
         __ cmp(eax, 0xc0000000);
         __ j(negative, &non_smi_result);
       }
       // Tag smi result and return.
       __ SmiTag(eax);
       GenerateReturn(masm);
 
       // All ops except SHR return a signed int32 that we load in a HeapNumber.
       if (op_ != Token::SHR) {
         __ bind(&non_smi_result);
         // Allocate a heap number if needed.
         __ mov(ebx, Operand(eax));  // ebx: result
+       Label skip_allocation;
         switch (mode_) {
           case OVERWRITE_LEFT:
           case OVERWRITE_RIGHT:
             // If the operand was an object, we skip the
             // allocation of a heap number.
             __ mov(eax, Operand(esp, mode_ == OVERWRITE_RIGHT ?
                                 1 * kPointerSize : 2 * kPointerSize));
             __ test(eax, Immediate(kSmiTagMask));
             __ j(not_zero, &skip_allocation, not_taken);
             // Fall through!
           case NO_OVERWRITE:
             __ AllocateHeapNumber(eax, ecx, edx, &call_runtime);
             __ bind(&skip_allocation);
             break;
           default: UNREACHABLE();
         }
         // Store the result in the HeapNumber and return.
         if (CpuFeatures::IsSupported(SSE2)) {
           CpuFeatures::Scope use_sse2(SSE2);
           __ cvtsi2sd(xmm0, Operand(ebx));
           __ movdbl(FieldOperand(eax, HeapNumber::kValueOffset), xmm0);
         } else {
           __ mov(Operand(esp, 1 * kPointerSize), ebx);
           __ fild_s(Operand(esp, 1 * kPointerSize));
           __ fstp_d(FieldOperand(eax, HeapNumber::kValueOffset));
         }
         GenerateReturn(masm);
       }
-
-      // Go to runtime for non-number inputs.
-      __ bind(&operand_conversion_failure);
-      // SHR should return uint32 - go to runtime for non-smi/negative result.
-      if (op_ == Token::SHR) {
-        __ bind(&non_smi_result);
-      }
-      __ mov(eax, Operand(esp, 1 * kPointerSize));
-      __ mov(edx, Operand(esp, 2 * kPointerSize));
       break;
     }
     default: UNREACHABLE(); break;
   }
 
   // If all else fails, use the runtime system to get the correct
   // result. If arguments was passed in registers now place them on the
   // stack in the correct order below the return address.
   __ bind(&call_runtime);
   if (HasArgumentsInRegisters()) {
     __ pop(ecx);
     if (HasArgumentsReversed()) {
       __ push(eax);
       __ push(edx);
     } else {
       __ push(edx);
       __ push(eax);
     }
     __ push(ecx);
   }
   switch (op_) {
     case Token::ADD: {
       // Test for string arguments before calling runtime.
       Label not_strings, not_string1, string1;
       Result answer;
-      __ mov(eax, Operand(esp, 2 * kPointerSize));  // First argument.
-      __ mov(edx, Operand(esp, 1 * kPointerSize));  // Second argument.
-      __ test(eax, Immediate(kSmiTagMask));
+      __ test(edx, Immediate(kSmiTagMask));
       __ j(zero, &not_string1);
-      __ CmpObjectType(eax, FIRST_NONSTRING_TYPE, eax);
+      __ CmpObjectType(edx, FIRST_NONSTRING_TYPE, ecx);
       __ j(above_equal, &not_string1);
 
-      // First argument is a a string, test second.
-      __ test(edx, Immediate(kSmiTagMask));
+      // First argument is a string, test second.
+      __ test(eax, Immediate(kSmiTagMask));
       __ j(zero, &string1);
-      __ CmpObjectType(edx, FIRST_NONSTRING_TYPE, edx);
+      __ CmpObjectType(eax, FIRST_NONSTRING_TYPE, ecx);
       __ j(above_equal, &string1);
 
       // First and second argument are strings. Jump to the string add stub.
       StringAddStub stub(NO_STRING_CHECK_IN_STUB);
       __ TailCallStub(&stub);
 
       // Only first argument is a string.
       __ bind(&string1);
-      __ InvokeBuiltin(Builtins::STRING_ADD_LEFT, JUMP_FUNCTION);
+      __ InvokeBuiltin(
+         HasArgumentsReversed() ?

[Mads Ager (chromium), 2010/01/25 15:44:03]

Indent by one more space. Looks like 3-space indent instead of 4-space indent?

[Vladislav Kaznacheev, 2010/01/25 16:24:47]

Done.

+             Builtins::STRING_ADD_RIGHT :
+             Builtins::STRING_ADD_LEFT,
+         JUMP_FUNCTION);
 
       // First argument was not a string, test second.
       __ bind(&not_string1);
-      __ test(edx, Immediate(kSmiTagMask));
+      __ test(eax, Immediate(kSmiTagMask));
       __ j(zero, &not_strings);
-      __ CmpObjectType(edx, FIRST_NONSTRING_TYPE, edx);
+      __ CmpObjectType(eax, FIRST_NONSTRING_TYPE, ecx);
       __ j(above_equal, &not_strings);
 
       // Only second argument is a string.
-      __ InvokeBuiltin(Builtins::STRING_ADD_RIGHT, JUMP_FUNCTION);
+      __ InvokeBuiltin(
+         HasArgumentsReversed() ?

[Mads Ager (chromium), 2010/01/25 15:44:03]

Indent by one more space.

[Vladislav Kaznacheev, 2010/01/25 16:24:47]

Done.

+             Builtins::STRING_ADD_LEFT :
+             Builtins::STRING_ADD_RIGHT,
+         JUMP_FUNCTION);
 
       __ bind(&not_strings);
       // Neither argument is a string.
       __ InvokeBuiltin(Builtins::ADD, JUMP_FUNCTION);
       break;
     }
     case Token::SUB:
       __ InvokeBuiltin(Builtins::SUB, JUMP_FUNCTION);
       break;
     case Token::MUL:
       __ InvokeBuiltin(Builtins::MUL, JUMP_FUNCTION);
-        break;
+      break;
     case Token::DIV:
       __ InvokeBuiltin(Builtins::DIV, JUMP_FUNCTION);
       break;
     case Token::MOD:
       __ InvokeBuiltin(Builtins::MOD, JUMP_FUNCTION);
       break;
     case Token::BIT_OR:
       __ InvokeBuiltin(Builtins::BIT_OR, JUMP_FUNCTION);
       break;
     case Token::BIT_AND:
@@ skipping 10 matching lines @@
       break;
     case Token::SHR:
       __ InvokeBuiltin(Builtins::SHR, JUMP_FUNCTION);
       break;
     default:
       UNREACHABLE();
   }
 }
 
 
+void GenericBinaryOpStub::GenerateHeapResultAllocation(MacroAssembler* masm,
+                                                       Label* alloc_failure) {
+  Label skip_allocation;
+  OverwriteMode mode = mode_;
+  if (HasArgumentsReversed()) {
+    if (mode == OVERWRITE_RIGHT)

[Mads Ager (chromium), 2010/01/25 15:44:03]

Please uses '{' and '}' when having multi-line if statements.

[Vladislav Kaznacheev, 2010/01/25 16:24:47]

Done.

+      mode = OVERWRITE_LEFT;
+    else if (mode == OVERWRITE_LEFT)
+      mode = OVERWRITE_RIGHT;
+  }
+  switch (mode) {
+    case OVERWRITE_LEFT: {
+      // If the argument in edx is already an object, we skip the
+      // allocation of a heap number.
+      __ test(edx, Immediate(kSmiTagMask));
+      __ j(not_zero, &skip_allocation, not_taken);
+      // Allocate a heap number for the result. Keep eax and edx intact
+      // for the possible runtime call.
+      __ AllocateHeapNumber(ebx, ecx, no_reg, alloc_failure);
+      // Now edx can be overwritten losing one of the arguments as we are
+      // now done and will not need it any more.
+      __ mov(edx, Operand(ebx));
+      __ bind(&skip_allocation);
+      // Use object in edx as a result holder
+      __ mov(eax, Operand(edx));
+      break;
+    }
+    case OVERWRITE_RIGHT:
+      // If the argument in eax is already an object, we skip the
+      // allocation of a heap number.
+      __ test(eax, Immediate(kSmiTagMask));
+      __ j(not_zero, &skip_allocation, not_taken);
+      // Fall through!
+    case NO_OVERWRITE:
+      // Allocate a heap number for the result. Keep eax and edx intact
+      // for the possible runtime call.
+      __ AllocateHeapNumber(ebx, ecx, no_reg, alloc_failure);
+      // Now eax can be overwritten losing one of the arguments as we are
+      // now done and will not need it any more.
+      __ mov(eax, ebx);
+      __ bind(&skip_allocation);
+      break;
+    default: UNREACHABLE();
+  }
+}
+
+
 void GenericBinaryOpStub::GenerateLoadArguments(MacroAssembler* masm) {
   // If arguments are not passed in registers read them from the stack.
   if (!HasArgumentsInRegisters()) {
     __ mov(eax, Operand(esp, 1 * kPointerSize));
     __ mov(edx, Operand(esp, 2 * kPointerSize));
   }
 }
 
 
 void GenericBinaryOpStub::GenerateReturn(MacroAssembler* masm) {
@@ skipping 242 matching lines @@
   __ SmiUntag(eax);  // Untag smi before converting to float.
   __ cvtsi2sd(xmm1, Operand(eax));
   __ SmiTag(eax);  // Retag smi for heap number overwriting test.
   __ jmp(&done);
   __ bind(&load_float_eax);
   __ movdbl(xmm1, FieldOperand(eax, HeapNumber::kValueOffset));
   __ bind(&done);
 }
 
 
+void FloatingPointHelper::LoadSse2Smis(MacroAssembler* masm,
+                                       Register scratch,
+                                       ArgLocation arg_location) {
+  if (arg_location == ARGS_IN_REGISTERS) {
+    __ mov(scratch, eax);
+  } else {
+    __ mov(scratch, Operand(esp, 2 * kPointerSize));
+  }
+  __ SmiUntag(scratch);  // Untag smi before converting to float.
+  __ cvtsi2sd(xmm0, Operand(scratch));
+
+
+  if (arg_location == ARGS_IN_REGISTERS) {
+    __ mov(scratch, ebx);
+  } else {
+    __ mov(scratch, Operand(esp, 1 * kPointerSize));
+  }
+  __ SmiUntag(scratch);  // Untag smi before converting to float.
+  __ cvtsi2sd(xmm1, Operand(scratch));
+}
+
+
 void FloatingPointHelper::LoadFloatOperands(MacroAssembler* masm,
-                                            Register scratch) {
+                                            Register scratch,
+                                            ArgLocation arg_location) {
   Label load_smi_1, load_smi_2, done_load_1, done;
-  __ mov(scratch, Operand(esp, 2 * kPointerSize));
+  if (arg_location == ARGS_IN_REGISTERS) {
+    __ mov(scratch, edx);
+  } else {
+    __ mov(scratch, Operand(esp, 2 * kPointerSize));
+  }
   __ test(scratch, Immediate(kSmiTagMask));
   __ j(zero, &load_smi_1, not_taken);
   __ fld_d(FieldOperand(scratch, HeapNumber::kValueOffset));
   __ bind(&done_load_1);
 
-  __ mov(scratch, Operand(esp, 1 * kPointerSize));
+  if (arg_location == ARGS_IN_REGISTERS) {
+    __ mov(scratch, eax);
+  } else {
+    __ mov(scratch, Operand(esp, 1 * kPointerSize));
+  }
   __ test(scratch, Immediate(kSmiTagMask));
   __ j(zero, &load_smi_2, not_taken);
   __ fld_d(FieldOperand(scratch, HeapNumber::kValueOffset));
   __ jmp(&done);
 
   __ bind(&load_smi_1);
   __ SmiUntag(scratch);
   __ push(scratch);
   __ fild_s(Operand(esp, 0));
   __ pop(scratch);
   __ jmp(&done_load_1);
 
   __ bind(&load_smi_2);
   __ SmiUntag(scratch);
   __ push(scratch);
   __ fild_s(Operand(esp, 0));
   __ pop(scratch);
 
   __ bind(&done);
 }
 
 
+void FloatingPointHelper::LoadFloatSmis(MacroAssembler* masm,
+                                        Register scratch,
+                                        ArgLocation arg_location) {
+  if (arg_location == ARGS_IN_REGISTERS) {
+    __ mov(scratch, eax);
+  } else {
+    __ mov(scratch, Operand(esp, 2 * kPointerSize));
+  }
+  __ SmiUntag(scratch);
+  __ push(scratch);
+  __ fild_s(Operand(esp, 0));
+  __ pop(scratch);
+
+  if (arg_location == ARGS_IN_REGISTERS) {
+    __ mov(scratch, ebx);
+  } else {
+    __ mov(scratch, Operand(esp, 1 * kPointerSize));
+  }
+  __ SmiUntag(scratch);
+  __ push(scratch);
+  __ fild_s(Operand(esp, 0));
+  __ pop(scratch);
+}
+
+
 void FloatingPointHelper::CheckFloatOperands(MacroAssembler* masm,
                                              Label* non_float,
                                              Register scratch) {
   Label test_other, done;
   // Test if both operands are floats or smi -> scratch=k_is_float;
   // Otherwise scratch = k_not_float.
   __ test(edx, Immediate(kSmiTagMask));
   __ j(zero, &test_other, not_taken);  // argument in edx is OK
   __ mov(scratch, FieldOperand(edx, HeapObject::kMapOffset));
   __ cmp(scratch, Factory::heap_number_map());
@@ skipping 2007 matching lines @@
 
   // Call the runtime; it returns -1 (less), 0 (equal), or 1 (greater)
   // tagged as a small integer.
   __ bind(&runtime);
   __ TailCallRuntime(ExternalReference(Runtime::kStringCompare), 2, 1);
 }
 
 #undef __
 
 } }  // namespace v8::internal