Change inlined smi binary operations to use an unspilled frame.

src/codegen-ia32.cc

 // Copyright 2006-2008 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 163 matching lines @@
     // Allocate the arguments object and copy the parameters into it.
     if (scope_->arguments() != NULL) {
       ASSERT(scope_->arguments_shadow() != NULL);
       Comment cmnt(masm_, "[ Allocate arguments object");
       ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT);
       VirtualFrame::SpilledScope spilled_scope(this);
       __ lea(eax, frame_->Receiver());
       frame_->EmitPush(frame_->Function());
       frame_->EmitPush(eax);
       frame_->EmitPush(Immediate(Smi::FromInt(scope_->num_parameters())));
-      frame_->CallStub(&stub, 3);
-      frame_->Push(eax);
+      Result answer = frame_->CallStub(&stub, 3);
+      frame_->Push(&answer);
     }
 
     if (scope_->num_heap_slots() > 0) {
       Comment cmnt(masm_, "[ allocate local context");
       // Allocate local context.
       // Get outer context and create a new context based on it.
       VirtualFrame::SpilledScope spilled_scope(this);
       frame_->EmitPush(frame_->Function());
       frame_->CallRuntime(Runtime::kNewContext, 1);  // eax holds the result
 
@@ skipping 438 matching lines @@
   // Smi => false iff zero.
   ASSERT(kSmiTag == 0);
   __ test(value.reg(), Operand(value.reg()));
   false_target->Branch(zero);
   __ test(value.reg(), Immediate(kSmiTagMask));
   true_target->Branch(zero);
 
   // Call the stub for all other cases.
   frame_->Push(&value);  // Undo the Pop() from above.
   ToBooleanStub stub;
-  frame_->CallStub(&stub, 1);
-  // Convert the result (eax) to condition code.
-  Result temp = allocator_->Allocate(eax);
-  ASSERT(temp.is_valid());
-  __ test(eax, Operand(eax));
+  Result temp = frame_->CallStub(&stub, 1);
+  // Convert the result to a condition code.
+  __ test(temp.reg(), Operand(temp.reg()));
 
   ASSERT(not_equal == not_zero);
   cc_reg_ = not_equal;
 }
 
 
 class FloatingPointHelper : public AllStatic {
  public:
   // Code pattern for loading floating point values. Input values must
   // be either smi or heap number objects (fp values). Requirements:
@@ skipping 85 matching lines @@
  public:
   DeferredInlineBinaryOperation(CodeGenerator* generator,
                                 Token::Value op,
                                 OverwriteMode mode,
                                 GenericBinaryFlags flags)
       : DeferredCode(generator),
         stub_(op, mode, flags),
         op_(op) {
   }
 
-  void GenerateInlineCode();
+  // The binary operation's arguments are on top of the code generator's frame.
+  Result GenerateInlineCode();
 
   virtual void Generate() {
-    // The arguments are is actually passed in ebx and the top of the stack.
-    enter()->Bind();
-    VirtualFrame::SpilledScope spilled_scope(generator());
-    generator()->frame()->EmitPush(ebx);
-    generator()->frame()->CallStub(&stub_, 2);
-    // We must preserve the eax value here, because it will be written
-    // to the top-of-stack element when getting back to the fast case
-    // code. See comment in GenericBinaryOperation where
-    // deferred->exit() is bound.
-    generator()->frame()->EmitPush(eax);
-    // The result is actually returned on the top of the stack.
-    exit()->Jump();
+    Result left(generator());
+    Result right(generator());
+    enter()->Bind(&left, &right);
+    generator()->frame()->Push(&left);
+    generator()->frame()->Push(&right);
+    Result answer = generator()->frame()->CallStub(&stub_, 2);
+    exit()->Jump(&answer);
   }
 
  private:
   GenericBinaryOpStub stub_;
   Token::Value op_;
 };
 
 
 void CodeGenerator::GenericBinaryOperation(Token::Value op,
                                            StaticType* type,
                                            OverwriteMode overwrite_mode) {
   Comment cmnt(masm_, "[ BinaryOperation");
   Comment cmnt_token(masm_, Token::String(op));
 
   if (op == Token::COMMA) {
     // Simply discard left value.
     frame_->Nip(1);
     return;
   }
 
-  VirtualFrame::SpilledScope spilled_scope(this);
   // Set the flags based on the operation, type and loop nesting level.
   GenericBinaryFlags flags;
   switch (op) {
     case Token::BIT_OR:
     case Token::BIT_AND:
     case Token::BIT_XOR:
     case Token::SHL:
     case Token::SHR:
     case Token::SAR:
       // Bit operations always assume they likely operate on Smis. Still only
       // generate the inline Smi check code if this operation is part of a loop.
       flags = (loop_nesting() > 0)
               ? SMI_CODE_INLINED
               : SMI_CODE_IN_STUB;
       break;
 
     default:
       // By default only inline the Smi check code for likely smis if this
       // operation is part of a loop.
       flags = ((loop_nesting() > 0) && type->IsLikelySmi())
               ? SMI_CODE_INLINED
               : SMI_CODE_IN_STUB;
       break;
   }
 
   if (flags == SMI_CODE_INLINED) {
     // Create a new deferred code for the slow-case part.
-    //
-    // TODO(): When this code is updated to use the virtual frame, it
-    // has to properly flow to the inline code from this deferred code
-    // stub.
     DeferredInlineBinaryOperation* deferred =
         new DeferredInlineBinaryOperation(this, op, overwrite_mode, flags);
-    // Fetch the operands from the stack.
-    frame_->EmitPop(ebx);  // get y
-    __ mov(eax, frame_->Top());  // get x
     // Generate the inline part of the code.
-    deferred->GenerateInlineCode();
-    // Put result back on the stack. It seems somewhat weird to let
-    // the deferred code jump back before the assignment to the frame
-    // top, but this is just to let the peephole optimizer get rid of
-    // more code.
-    deferred->exit()->Bind();
-    __ mov(frame_->Top(), eax);
+    // The operands are on the frame.
+    Result answer = deferred->GenerateInlineCode();
+    deferred->exit()->Bind(&answer);
+    frame_->Push(&answer);
   } else {
     // Call the stub and push the result to the stack.
     GenericBinaryOpStub stub(op, overwrite_mode, flags);
-    frame_->CallStub(&stub, 2);
-    frame_->EmitPush(eax);
+    Result answer = frame_->CallStub(&stub, 2);
+    frame_->Push(&answer);
   }
 }
 
 
 class DeferredInlinedSmiOperation: public DeferredCode {
  public:
   DeferredInlinedSmiOperation(CodeGenerator* generator,
                               Token::Value op, int value,
                               OverwriteMode overwrite_mode) :
       DeferredCode(generator), op_(op), value_(value),
@@ skipping 572 matching lines @@
   int arg_count = args->length();
   for (int i = 0; i < arg_count; i++) {
     Load(args->at(i));
   }
 
   // Record the position for debugging purposes.
   CodeForSourcePosition(position);
 
   // Use the shared code stub to call the function.
   CallFunctionStub call_function(arg_count);
-  frame_->CallStub(&call_function, arg_count + 1);
-  Result result = allocator_->Allocate(eax);
-
+  Result answer = frame_->CallStub(&call_function, arg_count + 1);
   // Restore context and replace function on the stack with the
   // result of the stub invocation.
   frame_->RestoreContextRegister();
-  frame_->SetElementAt(0, &result);
+  frame_->SetElementAt(0, &answer);
 }
 
 
 void CodeGenerator::Branch(bool if_true, JumpTarget* target) {
   ASSERT(has_cc());
   Condition cc = if_true ? cc_reg_ : NegateCondition(cc_reg_);
   cc_reg_ = no_condition;
   target->Branch(cc);
 }
 
@@ skipping 3375 matching lines @@
   __ ret(1 * kPointerSize);
   __ bind(&false_result);
   __ mov(eax, 0);
   __ ret(1 * kPointerSize);
 }
 
 
 #undef __
 #define __ masm_->
 
-// This function's implementation is a copy of
-// GenericBinaryOpStub::GenerateSmiCode, with the slow-case label replaced
-// with the deferred code's entry target.  The duplicated code is a
-// temporary intermediate stage on the way to using the virtual frame in
-// more places.
-void DeferredInlineBinaryOperation::GenerateInlineCode() {
-  // 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));
-
+Result DeferredInlineBinaryOperation::GenerateInlineCode() {
+  // Perform fast-case smi code for the operation (left <op> right) and
+  // returns the result in a Result.
+  // If any fast-case tests fail, it jumps to the slow-case deferred code,
+  // which calls the binary operation stub, with the arguments (in registers)
+  // on top of the frame.
+
+  VirtualFrame* frame = generator()->frame();
+  // If operation is division or modulus, ensure
+  // that the special registers needed are free.
+  Result reg_eax(generator());  // Valid only if op is DIV or MOD.
+  Result reg_edx(generator());  // Valid only if op is DIV or MOD.
+  if (op_ == Token::DIV || op_ == Token::MOD) {
+    reg_eax = generator()->allocator()->Allocate(eax);
+    ASSERT(reg_eax.is_valid());
+    reg_edx = generator()->allocator()->Allocate(edx);
+    ASSERT(reg_edx.is_valid());
+  }
+
+  Result right = frame->Pop();
+  Result left = frame->Pop();
+  left.ToRegister();
+  right.ToRegister();
+  // Answer is used to compute the answer, leaving left and right unchanged.
+  // It is also returned from this function.
+  // It is used as a temporary register in a few places, as well.
+  Result answer(generator());
+  if (reg_eax.is_valid()) {
+    answer = reg_eax;
+  } else {
+    answer = generator()->allocator()->Allocate();
+  }
+  ASSERT(answer.is_valid());
+  // Perform the smi check.
+  __ mov(answer.reg(), Operand(left.reg()));
+  __ or_(answer.reg(), Operand(right.reg()));
+  ASSERT(kSmiTag == 0);  // adjust zero check if not the case
+  __ test(answer.reg(), Immediate(kSmiTagMask));
+  enter()->Branch(not_zero, &left, &right, not_taken);
+
+  // All operations start by copying the left argument into answer.
+  __ mov(answer.reg(), Operand(left.reg()));
   switch (op_) {
     case Token::ADD:
-      __ add(eax, Operand(ebx));  // add optimistically
-      enter()->Branch(overflow, not_taken);
+      __ add(answer.reg(), Operand(right.reg()));  // add optimistically
+      enter()->Branch(overflow, &left, &right, not_taken);
       break;
 
     case Token::SUB:
-      __ sub(eax, Operand(ebx));  // subtract optimistically
-      enter()->Branch(overflow, not_taken);
-      break;
-
-    case Token::DIV:
-    case Token::MOD:
+      __ sub(answer.reg(), Operand(right.reg()));  // subtract optimistically
+      enter()->Branch(overflow, &left, &right, not_taken);
+      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 the left operand (but keep sign).
+      // Left hand operand has been copied into answer.
+      __ sar(answer.reg(), kSmiTagSize);
+      // Do multiplication of smis, leaving result in answer.
+      __ imul(answer.reg(), Operand(right.reg()));
+      // Go slow on overflows.
+      enter()->Branch(overflow, &left, &right, not_taken);
+      // Check for negative zero result.  If product is zero,
+      // and one argument is negative, go to slow case.
+      // The frame is unchanged in this block, so local control flow can
+      // use a Label rather than a JumpTarget.
+      Label non_zero_result;
+      __ test(answer.reg(), Operand(answer.reg()));
+      __ j(not_zero, &non_zero_result, taken);
+      __ mov(answer.reg(), Operand(left.reg()));
+      __ or_(answer.reg(), Operand(right.reg()));
+      enter()->Branch(negative, &left, &right, not_taken);
+      __ xor_(answer.reg(), Operand(answer.reg()));  // Positive 0 is correct.
+      __ bind(&non_zero_result);
+      break;
+    }
+
+    case Token::DIV: {
+      // Left hand argument has been copied into answer, which is eax.
       // Sign extend eax into edx:eax.
       __ cdq();
       // Check for 0 divisor.
-      __ test(ebx, Operand(ebx));
-      enter()->Branch(zero, 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));
-  enter()->Branch(not_zero, 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).
-      __ sar(eax, kSmiTagSize);
-      // Do multiplication.
-      __ imul(eax, Operand(ebx));  // multiplication of smis; result in eax
-      // Go slow on overflows.
-      enter()->Branch(overflow, not_taken);
-      // Check for negative zero result.  Use ecx = x | y.
-      __ NegativeZeroTest(generator(), eax, ecx, enter());
-      break;
-
-    case Token::DIV:
+      __ test(right.reg(), Operand(right.reg()));
+      enter()->Branch(zero, &left, &right, not_taken);
       // Divide edx:eax by ebx.
-      __ idiv(ebx);
+      __ idiv(right.reg());
+      // Check for negative zero result.  If result is zero, and divisor
+      // is negative, return a floating point negative zero.
+      // The frame is unchanged in this block, so local control flow can
+      // use a Label rather than a JumpTarget.
+      Label non_zero_result;
+      __ test(left.reg(), Operand(left.reg()));
+      __ j(not_zero, &non_zero_result, taken);
+      __ test(right.reg(), Operand(right.reg()));
+      enter()->Branch(negative, &left, &right, not_taken);
+      __ bind(&non_zero_result);
       // 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);
-      enter()->Branch(equal);
-      // Check for negative zero result.  Use ecx = x | y.
-      __ NegativeZeroTest(generator(), eax, ecx, enter());
+      __ cmp(reg_eax.reg(), 0x40000000);
+      enter()->Branch(equal, &left, &right, not_taken);
       // Check that the remainder is zero.
-      __ test(edx, Operand(edx));
-      enter()->Branch(not_zero);
-      // Tag the result and store it in register eax.
+      __ test(reg_edx.reg(), Operand(reg_edx.reg()));
+      enter()->Branch(not_zero, &left, &right, not_taken);
+      // Tag the result and store it in register temp.

[Kevin Millikin (Chromium), 2009/01/15 15:47:03]

the answer register.

       ASSERT(kSmiTagSize == times_2);  // adjust code if not the case
-      __ lea(eax, Operand(eax, eax, times_1, kSmiTag));
-      break;
-
-    case Token::MOD:
+      __ lea(answer.reg(), Operand(eax, eax, times_1, kSmiTag));
+      break;
+    }
+
+    case Token::MOD: {
+      // Left hand argument has been copied into answer, which is eax.
+      // Sign extend eax into edx:eax.
+      __ cdq();
+      // Check for 0 divisor.
+      __ test(right.reg(), Operand(right.reg()));
+      enter()->Branch(zero, &left, &right, not_taken);
+
       // Divide edx:eax by ebx.
-      __ idiv(ebx);
-      // Check for negative zero result.  Use ecx = x | y.
-      __ NegativeZeroTest(generator(), edx, ecx, enter());
-      // Move remainder to register eax.
-      __ mov(eax, Operand(edx));
-      break;
+      __ idiv(right.reg());
+      // Check for negative zero result.  If result is zero, and divisor
+      // is negative, return a floating point negative zero.
+      // The frame is unchanged in this block, so local control flow can
+      // use a Label rather than a JumpTarget.
+      Label non_zero_result;
+      __ test(reg_edx.reg(), Operand(reg_edx.reg()));
+      __ j(not_zero, &non_zero_result, taken);
+      __ test(left.reg(), Operand(left.reg()));
+      enter()->Branch(negative, &left, &right, not_taken);
+      __ bind(&non_zero_result);
+      // The answer is in edx.
+      answer = reg_edx;
+      break;
+    }
 
     case Token::BIT_OR:
-      __ or_(eax, Operand(ebx));
+      __ or_(answer.reg(), Operand(right.reg()));
       break;
 
     case Token::BIT_AND:
-      __ and_(eax, Operand(ebx));
+      __ and_(answer.reg(), Operand(right.reg()));
       break;
 
     case Token::BIT_XOR:
-      __ xor_(eax, Operand(ebx));
+      __ xor_(answer.reg(), Operand(right.reg()));
       break;
 
     case Token::SHL:
     case Token::SHR:
     case Token::SAR:
-      // Move the second operand into register ecx.
-      __ mov(ecx, Operand(ebx));
+      // Move right into ecx.
+      // Left is in two registers already, so even if left or answer is ecx,
+      // we can move right to it, and use the other one.
+      // Right operand must be in register cl because x86 likes it that way.
+      if (right.reg().is(ecx)) {
+        // Right is already in the right place.  Left may be in the
+        // same register, which causes problems.  Use answer instead.
+        if (left.reg().is(ecx)) {
+          left = answer;
+        }
+      } else if (left.reg().is(ecx)) {
+        __ mov(left.reg(), Operand(right.reg()));
+        right = left;
+        left = answer;  // Use copy of left in answer as left.
+      } else if (answer.reg().is(ecx)) {
+        __ mov(answer.reg(), Operand(right.reg()));
+        right = answer;
+      } else {
+        Result reg_ecx = generator()->allocator()->Allocate(ecx);
+        ASSERT(reg_ecx.is_valid());
+        __ mov(reg_ecx.reg(), Operand(right.reg()));
+        right = reg_ecx;
+      }
+      ASSERT(left.reg().is_valid());
+      ASSERT(!left.reg().is(ecx));
+      ASSERT(right.reg().is(ecx));
+      answer.Unuse();  // Answer may now be being used for left or right.
+      // We will modify left and right, which we do not do in any other
+      // binary operation.  The exits to slow code need to restore the
+      // original values of left and right, or at least values that give
+      // the same answer.
+
+      // We are modifying left and right.  They must be spilled!
+      generator()->frame()->Spill(left.reg());
+      generator()->frame()->Spill(right.reg());
+
       // Remove tags from operands (but keep sign).
-      __ sar(eax, kSmiTagSize);
+      __ sar(left.reg(), kSmiTagSize);
       __ sar(ecx, kSmiTagSize);
       // Perform the operation.
       switch (op_) {
         case Token::SAR:
-          __ sar(eax);
+          __ sar(left.reg());
           // No checks of result necessary
           break;
-        case Token::SHR:
-          __ shr(eax);
+        case Token::SHR: {
+          __ shr(left.reg());
           // Check that the *unsigned* result fits in a smi.
           // Neither of the two high-order bits can be set:
           // - 0x80000000: high bit would be lost when smi tagging.
           // - 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));
-          enter()->Branch(not_zero, not_taken);
+          // If the answer cannot be represented by a SMI, restore
+          // the left and right arguments, and jump to slow case.
+          // The low bit of the left argument may be lost, but only
+          // in a case where it is dropped anyway.
+          JumpTarget result_ok(generator());
+          __ test(left.reg(), Immediate(0xc0000000));
+          result_ok.Branch(zero, &left, &right, taken);
+          __ shl(left.reg());
+          ASSERT(kSmiTag == 0);
+          __ shl(left.reg(), kSmiTagSize);
+          __ shl(right.reg(), kSmiTagSize);
+          enter()->Jump(&left, &right);
+          result_ok.Bind(&left, &right);
           break;
-        case Token::SHL:
-          __ shl(eax);
+        }
+        case Token::SHL: {
+          __ shl(left.reg());
           // Check that the *signed* result fits in a smi.
-          __ lea(ecx, Operand(eax, 0x40000000));
-          __ test(ecx, Immediate(0x80000000));
-          enter()->Branch(not_zero, not_taken);
+          // TODO(): Can reduce registers from 4 to 3 by preallocating ecx.
+          JumpTarget result_ok(generator());
+          Result smi_test_reg = generator()->allocator()->Allocate();
+          ASSERT(smi_test_reg.is_valid());
+          __ lea(smi_test_reg.reg(), Operand(left.reg(), 0x40000000));
+          __ test(smi_test_reg.reg(), Immediate(0x80000000));
+          smi_test_reg.Unuse();
+          result_ok.Branch(zero, &left, &right, taken);
+          __ shr(left.reg());
+          ASSERT(kSmiTag == 0);
+          __ shl(left.reg(), kSmiTagSize);
+          __ shl(right.reg(), kSmiTagSize);
+          enter()->Jump(&left, &right);
+          result_ok.Bind(&left, &right);
           break;
+        }
         default:
           UNREACHABLE();
       }
-      // Tag the result and store it in register eax.
+      // Smi-tag the result, in left, and make answer an alias for left.
+      answer = left;
+      answer.ToRegister();
       ASSERT(kSmiTagSize == times_2);  // adjust code if not the case
-      __ lea(eax, Operand(eax, eax, times_1, kSmiTag));
+      __ lea(answer.reg(),
+             Operand(answer.reg(), answer.reg(), times_1, kSmiTag));
       break;
 
     default:
       UNREACHABLE();
       break;
   }
+  return answer;
 }
 
 
 #undef __
 #define __ masm->
 
 void GenericBinaryOpStub::GenerateSmiCode(MacroAssembler* masm, Label* slow) {
   // Perform fast-case smi code for the operation (eax <op> ebx) and
   // leave result in register eax.
 
@@ skipping 1154 matching lines @@
 
   // Slow-case: Go through the JavaScript implementation.
   __ bind(&slow);
   __ InvokeBuiltin(Builtins::INSTANCE_OF, JUMP_FUNCTION);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal