Rewrite of VisitCountOperation that should speed it up

src/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 4706 matching lines @@
     }
   }
 }
 
 
 class DeferredCountOperation: public DeferredCode {
  public:
   DeferredCountOperation(CodeGenerator* generator,
                          bool is_postfix,
                          bool is_increment,
-                         int result_offset)
+                         int target_size)
       : DeferredCode(generator),
         is_postfix_(is_postfix),
         is_increment_(is_increment),
-        result_offset_(result_offset) {
+        target_size_(target_size) {
     set_comment("[ DeferredCountOperation");
   }
 
   virtual void Generate();
 
  private:
   bool is_postfix_;
   bool is_increment_;
-  int result_offset_;
-};
-
-
-class RevertToNumberStub: public CodeStub {
- public:
-  explicit RevertToNumberStub(bool is_increment)
-     :  is_increment_(is_increment) { }
-
- private:
-  bool is_increment_;
-
-  Major MajorKey() { return RevertToNumber; }
-  int MinorKey() { return is_increment_ ? 1 : 0; }
-  void Generate(MacroAssembler* masm);
-
-#ifdef DEBUG
-  void Print() {
-    PrintF("RevertToNumberStub (is_increment %s)\n",
-           is_increment_ ? "true" : "false");
-  }
-#endif
-};
-
-
-class CounterOpStub: public CodeStub {
- public:
-  CounterOpStub(int result_offset, bool is_postfix, bool is_increment)
-     :  result_offset_(result_offset),
-        is_postfix_(is_postfix),
-        is_increment_(is_increment) { }
-
- private:
-  int result_offset_;
-  bool is_postfix_;
-  bool is_increment_;
-
-  Major MajorKey() { return CounterOp; }
-  int MinorKey() {
-    return ((result_offset_ << 2) |
-            (is_postfix_ ? 2 : 0) |
-            (is_increment_ ? 1 : 0));
-  }
-  void Generate(MacroAssembler* masm);
-
-#ifdef DEBUG
-  void Print() {
-    PrintF("CounterOpStub (result_offset %d), (is_postfix %s),"
-           " (is_increment %s)\n",
-           result_offset_,
-           is_postfix_ ? "true" : "false",
-           is_increment_ ? "true" : "false");
-  }
-#endif
+  int target_size_;
 };
 
 
 void DeferredCountOperation::Generate() {
   CodeGenerator* cgen = generator();
-
   Result value(cgen);
   enter()->Bind(&value);
-  if (is_postfix_) {
-    RevertToNumberStub to_number_stub(is_increment_);
-    value = generator()->frame()->CallStub(&to_number_stub, &value);
+  VirtualFrame* frame = cgen->frame();
+  // Undo the optimistic smi operation.
+  value.ToRegister();
+  frame->Spill(value.reg());
+  if (is_increment_) {
+    __ sub(Operand(value.reg()), Immediate(Smi::FromInt(1)));
+  } else {
+    __ add(Operand(value.reg()), Immediate(Smi::FromInt(1)));
   }
-
-  CounterOpStub stub(result_offset_, is_postfix_, is_increment_);
-  value = generator()->frame()->CallStub(&stub, &value);
+  frame->Push(&value);
+  value = frame->InvokeBuiltin(Builtins::TO_NUMBER, CALL_FUNCTION, 1);
+  frame->Push(&value);
+  if (is_postfix_) {  // Fix up copy of old value with ToNumber(value).
+    // This is only safe because VisitCountOperation makes this frame slot
+    // beneath the reference a register, which is spilled at the above call.
+    // We cannot safely write to constants or copies below the water line.
+    frame->StoreToElementAt(target_size_ + 1);
+  }
+  frame->Push(Smi::FromInt(1));
+  if (is_increment_) {
+    value = frame->CallRuntime(Runtime::kNumberAdd, 2);
+  } else {
+    value = frame->CallRuntime(Runtime::kNumberSub, 2);
+  }
   exit_.Jump(&value);
 }
 
 
 void CodeGenerator::VisitCountOperation(CountOperation* node) {
   Comment cmnt(masm_, "[ CountOperation");
 
   bool is_postfix = node->is_postfix();
   bool is_increment = node->op() == Token::INC;
 
   Variable* var = node->expression()->AsVariableProxy()->AsVariable();
   bool is_const = (var != NULL && var->mode() == Variable::CONST);
 
-  // Postfix: Make room for the result.
+  // Postfix operators need a stack slot under the reference to hold
+  // the old value while the new one is being stored.
   if (is_postfix) {
     frame_->Push(Smi::FromInt(0));
   }
 
   { Reference target(this, node->expression());
     if (target.is_illegal()) {
       // Spoof the virtual frame to have the expected height (one higher
       // than on entry).
       if (!is_postfix) {
         frame_->Push(Smi::FromInt(0));
       }
       return;
     }
     target.TakeValue(NOT_INSIDE_TYPEOF);
 
     DeferredCountOperation* deferred =
-        new DeferredCountOperation(this, is_postfix, is_increment,
-                                   target.size() * kPointerSize);
+        new DeferredCountOperation(this, is_postfix,
+                                   is_increment, target.size());
 
     Result value = frame_->Pop();
     value.ToRegister();
-    ASSERT(value.is_valid());
 
     // Postfix: Store the old value as the result.
     if (is_postfix) {
-      Result old_value = value;
+      // Explicitly back the slot for the old value with a new register.
+      // This improves performance in some cases.
+      Result old_value = allocator_->Allocate();
+      ASSERT(old_value.is_valid());
+      __ mov(old_value.reg(), value.reg());
+      // SetElement must not create a constant element or a copy in this slot,
+      // since we will write to it, below the waterline, in deferred code.
       frame_->SetElementAt(target.size(), &old_value);
     }
 
     // Perform optimistic increment/decrement.  Ensure the value is
     // writable.
     frame_->Spill(value.reg());
     ASSERT(allocator_->count(value.reg()) == 1);
 
     // In order to combine the overflow and the smi check, we need to
     // be able to allocate a byte register.  We attempt to do so
@@ skipping 19 matching lines @@
     // slow-case code.
     //
     // We combine the overflow and the smi check if we could
     // successfully allocate a temporary byte register.
     if (tmp.is_valid()) {
       __ setcc(overflow, tmp.reg());
       __ or_(Operand(value.reg()), tmp.reg());
       tmp.Unuse();
       __ test(value.reg(), Immediate(kSmiTagMask));
       deferred->enter()->Branch(not_zero, &value, not_taken);
-    } else {
+    } else {  // Otherwise we test separately for overflow and smi check.
       deferred->enter()->Branch(overflow, &value, not_taken);
       __ test(value.reg(), Immediate(kSmiTagMask));
       deferred->enter()->Branch(not_zero, &value, not_taken);
     }
 
     // Store the new value in the target if not const.
     deferred->BindExit(&value);
     frame_->Push(&value);
     if (!is_const) {
       target.SetValue(NOT_CONST_INIT);
@@ skipping 1871 matching lines @@
   // Slow-case: Non-function called.
   __ bind(&slow);
   __ Set(eax, Immediate(argc_));
   __ Set(ebx, Immediate(0));
   __ GetBuiltinEntry(edx, Builtins::CALL_NON_FUNCTION);
   Handle<Code> adaptor(Builtins::builtin(Builtins::ArgumentsAdaptorTrampoline));
   __ jmp(adaptor, RelocInfo::CODE_TARGET);
 }
 
 
-void RevertToNumberStub::Generate(MacroAssembler* masm) {
-  // Revert optimistic increment/decrement.
-  if (is_increment_) {
-    __ sub(Operand(eax), Immediate(Smi::FromInt(1)));
-  } else {
-    __ add(Operand(eax), Immediate(Smi::FromInt(1)));
-  }
-
-  __ pop(ecx);
-  __ push(eax);
-  __ push(ecx);
-  __ InvokeBuiltin(Builtins::TO_NUMBER, JUMP_FUNCTION);
-  // Code never returns due to JUMP_FUNCTION.
-}
-
-
-void CounterOpStub::Generate(MacroAssembler* masm) {
-  // Store to the result on the stack (skip return address) before
-  // performing the count operation.
-  if (is_postfix_) {
-    __ mov(Operand(esp, result_offset_ + kPointerSize), eax);
-  }
-
-  // Revert optimistic increment/decrement but only for prefix
-  // counts. For postfix counts it has already been reverted before
-  // the conversion to numbers.
-  if (!is_postfix_) {
-    if (is_increment_) {
-      __ sub(Operand(eax), Immediate(Smi::FromInt(1)));
-    } else {
-      __ add(Operand(eax), Immediate(Smi::FromInt(1)));
-    }
-  }
-
-  // Compute the new value by calling the right JavaScript native.
-  __ pop(ecx);
-  __ push(eax);
-  __ push(ecx);
-  Builtins::JavaScript builtin = is_increment_ ? Builtins::INC : Builtins::DEC;
-  __ InvokeBuiltin(builtin, JUMP_FUNCTION);
-  // Code never returns due to JUMP_FUNCTION.
-}
-
 
 void CEntryStub::GenerateThrowTOS(MacroAssembler* masm) {
   ASSERT(StackHandlerConstants::kSize == 6 * kPointerSize);  // adjust this code
   ExternalReference handler_address(Top::k_handler_address);
   __ mov(edx, Operand::StaticVariable(handler_address));
   __ mov(ecx, Operand(edx, -1 * kPointerSize));  // get next in chain
   __ mov(Operand::StaticVariable(handler_address), ecx);
   __ mov(esp, Operand(edx));
   __ pop(edi);
   __ pop(ebp);
@@ skipping 342 matching lines @@
 
   // Slow-case: Go through the JavaScript implementation.
   __ bind(&slow);
   __ InvokeBuiltin(Builtins::INSTANCE_OF, JUMP_FUNCTION);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal