Improve performance of arguments object allocation by taking

care of arguments adaptor frames in the generated code.
Review URL: http://codereview.chromium.org/6262

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

src/codegen-arm.cc

@@ -587,9 +587,15 @@ void ArmCodeGenerator::GenCode(FunctionLiteral* fun) {
       Comment cmnt(masm_, "[ allocate arguments object");
       { Reference shadow_ref(this, scope->arguments_shadow());
         { Reference arguments_ref(this, scope->arguments());
-          __ ldr(r0, FunctionOperand());
-          __ push(r0);
-          __ CallRuntime(Runtime::kNewArguments, 1);
+          ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT);
+          __ ldr(r2, FunctionOperand());
+          // The receiver is below the arguments, the return address,
+          // and the frame pointer on the stack.
+          const int kReceiverDisplacement = 2 + scope->num_parameters();
+          __ add(r1, fp, Operand(kReceiverDisplacement * kPointerSize));
+          __ mov(r0, Operand(Smi::FromInt(scope->num_parameters())));
+          __ stm(db_w, sp, r0.bit() | r1.bit() | r2.bit());
+          __ CallStub(&stub);
           __ push(r0);
           SetValue(&arguments_ref);
         }
@@ -963,28 +969,6 @@ class InvokeBuiltinStub : public CodeStub {
 };
 
 
-class ArgumentsAccessStub: public CodeStub {
- public:
-  explicit ArgumentsAccessStub(bool is_length) : is_length_(is_length) { }
-
- private:
-  bool is_length_;
-
-  Major MajorKey() { return ArgumentsAccess; }
-  int MinorKey() { return is_length_ ? 1 : 0; }
-  void Generate(MacroAssembler* masm);
-
-  const char* GetName() { return "ArgumentsAccessStub"; }
-
-#ifdef DEBUG
-  void Print() {
-    PrintF("ArgumentsAccessStub (is_length %s)\n",
-           is_length_ ? "true" : "false");
-  }
-#endif
-};
-
-
 void ArmCodeGenerator::GetReferenceProperty(Expression* key) {
   ASSERT(!ref()->is_illegal());
   Reference::Type type = ref()->type();
@@ -2857,7 +2841,7 @@ void ArmCodeGenerator::GenerateArgumentsLength(ZoneList<Expression*>* args) {
   __ mov(r0, Operand(Smi::FromInt(scope_->num_parameters())));
 
   // Call the shared stub to get to the arguments.length.
-  ArgumentsAccessStub stub(true);
+  ArgumentsAccessStub stub(ArgumentsAccessStub::READ_LENGTH);
   __ CallStub(&stub);
   __ push(r0);
 }
@@ -2873,7 +2857,7 @@ void ArmCodeGenerator::GenerateArgumentsAccess(ZoneList<Expression*>* args) {
   __ mov(r0, Operand(Smi::FromInt(scope_->num_parameters())));
 
   // Call the shared stub to get to arguments[key].
-  ArgumentsAccessStub stub(false);
+  ArgumentsAccessStub stub(ArgumentsAccessStub::READ_ELEMENT);
   __ CallStub(&stub);
   __ push(r0);
 }
@@ -4426,9 +4410,9 @@ void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
   //  -- lr: return address
   // -----------------------------------
 
-  // Check that the key is a smi for non-length accesses.
+  // If we're reading an element we need to check that the key is a smi.
   Label slow;
-  if (!is_length_) {
+  if (type_ == READ_ELEMENT) {
     __ tst(r1, Operand(kSmiTagMask));
     __ b(ne, &slow);
   }
@@ -4440,15 +4424,20 @@ void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
   __ ldr(r2, MemOperand(fp, StandardFrameConstants::kCallerFPOffset));
   __ ldr(r3, MemOperand(r2, StandardFrameConstants::kContextOffset));
   __ cmp(r3, Operand(ArgumentsAdaptorFrame::SENTINEL));
-  __ b(eq, &adaptor);
+  if (type_ == NEW_OBJECT) {
+    __ b(ne, &slow);
+  } else {
+    __ b(eq, &adaptor);
+  }
 
   static const int kParamDisplacement =
       StandardFrameConstants::kCallerSPOffset - kPointerSize;
 
-  if (is_length_) {
-    // Nothing to do: the formal length of parameters has been passed in r0
-    // by the calling function.
-  } else {
+  if (type_ == READ_LENGTH) {
+    // Nothing to do: The formal number of parameters has already been
+    // passed in register r0 by calling function. Just return it.
+    __ mov(pc, lr);
+  } else if (type_ == READ_ELEMENT) {
     // Check index against formal parameter count. Use unsigned comparison to
     // get the negative check for free.
     // r0: formal number of parameters
@@ -4456,15 +4445,16 @@ void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
     __ cmp(r1, r0);
     __ b(cs, &slow);
 
-    // Read the argument from the current frame.
+    // Read the argument from the current frame and return it.
     __ sub(r3, r0, r1);
     __ add(r3, fp, Operand(r3, LSL, kPointerSizeLog2 - kSmiTagSize));
     __ ldr(r0, MemOperand(r3, kParamDisplacement));
+    __ mov(pc, lr);
+  } else {
+    ASSERT(type_ == NEW_OBJECT);
+    // Do nothing here.
   }
 
-  // Return to the calling function.
-  __ mov(pc, lr);
-
   // An arguments adaptor frame is present. Find the length or the actual
   // argument in the calling frame.
   // r0: formal number of parameters
@@ -4475,7 +4465,10 @@ void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
   // only accessing the length, otherwise it is used in accessing the value
   __ ldr(r0, MemOperand(r2, ArgumentsAdaptorFrameConstants::kLengthOffset));
 
-  if (!is_length_) {
+  if (type_ == READ_LENGTH) {
+    // Return the length in r0.
+    __ mov(pc, lr);
+  } else if (type_ == READ_ELEMENT) {
     // Check index against actual arguments count. Use unsigned comparison to
     // get the negative check for free.
     // r0: actual number of parameter
@@ -4484,16 +4477,24 @@ void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
     __ cmp(r1, r0);
     __ b(cs, &slow);
 
-    // Read the argument from the adaptor frame.
+    // Read the argument from the adaptor frame and return it.
     __ sub(r3, r0, r1);
     __ add(r3, r2, Operand(r3, LSL, kPointerSizeLog2 - kSmiTagSize));
     __ ldr(r0, MemOperand(r3, kParamDisplacement));
+    __ mov(pc, lr);
+  } else {
+    ASSERT(type_ == NEW_OBJECT);
+    // Patch the arguments.length and the parameters pointer.
+    __ str(r0, MemOperand(sp, 0 * kPointerSize));
+    __ add(r3, r2, Operand(r0, LSL, kPointerSizeLog2 - kSmiTagSize));
+    __ add(r3, r3, Operand(kParamDisplacement + 1 * kPointerSize));
+    __ str(r3, MemOperand(sp, 1 * kPointerSize));
+    __ bind(&slow);
+    __ TailCallRuntime(ExternalReference(Runtime::kNewArgumentsFast), 3);
   }
 
   // Return to the calling function.
-  __ mov(pc, lr);
-
-  if (!is_length_) {
+  if (type_ == READ_ELEMENT) {
     __ bind(&slow);
     __ push(r1);
     __ TailCallRuntime(ExternalReference(Runtime::kGetArgumentsProperty), 1);

src/codegen-ia32.cc

@@ -518,7 +518,6 @@ Ia32CodeGenerator::Ia32CodeGenerator(int buffer_size,
 // edi: caller's parameter pointer
 // esi: callee's context
 
-
 void Ia32CodeGenerator::GenCode(FunctionLiteral* fun) {
   // Record the position for debugging purposes.
   __ RecordPosition(fun->start_position());
@@ -565,8 +564,12 @@ void Ia32CodeGenerator::GenCode(FunctionLiteral* fun) {
     if (scope->arguments() != NULL) {
       ASSERT(scope->arguments_shadow() != NULL);
       Comment cmnt(masm_, "[ allocate arguments object");
+      ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT);
+      __ lea(eax, ReceiverOperand());
       __ push(FunctionOperand());
-      __ CallRuntime(Runtime::kNewArguments, 1);
+      __ push(eax);
+      __ push(Immediate(Smi::FromInt(scope->num_parameters())));
+      __ CallStub(&stub);
       __ mov(ecx, Operand(eax));
       arguments_object_allocated = true;
     }
@@ -1068,28 +1071,6 @@ const char* GenericBinaryOpStub::GetName() {
 }
 
 
-class ArgumentsAccessStub: public CodeStub {
- public:
-  explicit ArgumentsAccessStub(bool is_length) : is_length_(is_length) { }
-
- private:
-  bool is_length_;
-
-  Major MajorKey() { return ArgumentsAccess; }
-  int MinorKey() { return is_length_ ? 1 : 0; }
-  void Generate(MacroAssembler* masm);
-
-  const char* GetName() { return "ArgumentsAccessStub"; }
-
-#ifdef DEBUG
-  void Print() {
-    PrintF("ArgumentsAccessStub (is_length %s)\n",
-           is_length_ ? "true" : "false");
-  }
-#endif
-};
-
-
 void Ia32CodeGenerator::GenericBinaryOperation(Token::Value op,
                                                OverwriteMode overwrite_mode) {
   Comment cmnt(masm_, "[ BinaryOperation");
@@ -3313,7 +3294,7 @@ void Ia32CodeGenerator::GenerateArgumentsLength(ZoneList<Expression*>* args) {
   __ Set(eax, Immediate(Smi::FromInt(scope_->num_parameters())));
 
   // Call the shared stub to get to the arguments.length.
-  ArgumentsAccessStub stub(true);
+  ArgumentsAccessStub stub(ArgumentsAccessStub::READ_LENGTH);
   __ CallStub(&stub);
   __ push(eax);
 }
@@ -3376,7 +3357,7 @@ void Ia32CodeGenerator::GenerateArgumentsAccess(ZoneList<Expression*>* args) {
   __ Set(eax, Immediate(Smi::FromInt(scope_->num_parameters())));
 
   // Call the shared stub to get to arguments[key].
-  ArgumentsAccessStub stub(false);
+  ArgumentsAccessStub stub(ArgumentsAccessStub::READ_ELEMENT);
   __ CallStub(&stub);
   __ mov(TOS, eax);
 }
@@ -4815,9 +4796,9 @@ void UnarySubStub::Generate(MacroAssembler* masm) {
 
 
 void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
-  // Check that the key is a smi for non-length access.
+  // If we're reading an element we need to check that the key is a smi.
   Label slow;
-  if (!is_length_) {
+  if (type_ == READ_ELEMENT) {
     __ mov(ebx, Operand(esp, 1 * kPointerSize));  // skip return address
     __ test(ebx, Immediate(kSmiTagMask));
     __ j(not_zero, &slow, not_taken);
@@ -4828,7 +4809,11 @@ void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
   __ mov(edx, Operand(ebp, StandardFrameConstants::kCallerFPOffset));
   __ mov(ecx, Operand(edx, StandardFrameConstants::kContextOffset));
   __ cmp(ecx, ArgumentsAdaptorFrame::SENTINEL);
-  __ j(equal, &adaptor);
+  if (type_ == NEW_OBJECT) {
+    __ j(not_equal, &slow);
+  } else {
+    __ j(equal, &adaptor);
+  }
 
   // The displacement is used for skipping the return address on the
   // stack. It is the offset of the last parameter (if any) relative
@@ -4836,31 +4821,35 @@ void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
   static const int kDisplacement = 1 * kPointerSize;
   ASSERT(kSmiTagSize == 1 && kSmiTag == 0);  // shifting code depends on this
 
-  if (is_length_) {
-    // Do nothing. The length is already in register eax.
-  } else {
+  if (type_ == READ_LENGTH) {
+    // Nothing to do: The formal number of parameters has already been
+    // passed in register eax by calling function. Just return it.
+    __ ret(0);
+  } else if (type_ == READ_ELEMENT) {
     // Check index against formal parameters count limit passed in
     // through register eax. Use unsigned comparison to get negative
     // check for free.
     __ cmp(ebx, Operand(eax));
     __ j(above_equal, &slow, not_taken);
 
-    // Read the argument from the stack.
+    // Read the argument from the stack and return it.
     __ lea(edx, Operand(ebp, eax, times_2, 0));
     __ neg(ebx);
     __ mov(eax, Operand(edx, ebx, times_2, kDisplacement));
+    __ ret(0);
+  } else {
+    ASSERT(type_ == NEW_OBJECT);
+    // Do nothing here.
   }
 
-  // Return the length or the argument.
-  __ ret(0);
-
   // Arguments adaptor case: Find the length or the actual argument in
   // the calling frame.
   __ bind(&adaptor);
-  if (is_length_) {
-    // Read the arguments length from the adaptor frame.
+  if (type_ == READ_LENGTH) {
+    // Read the arguments length from the adaptor frame and return it.
     __ mov(eax, Operand(edx, ArgumentsAdaptorFrameConstants::kLengthOffset));
-  } else {
+    __ ret(0);
+  } else if (type_ == READ_ELEMENT) {
     // Check index against actual arguments limit found in the
     // arguments adaptor frame. Use unsigned comparison to get
     // negative check for free.
@@ -4868,18 +4857,25 @@ void ArgumentsAccessStub::Generate(MacroAssembler* masm) {
     __ cmp(ebx, Operand(ecx));
     __ j(above_equal, &slow, not_taken);
 
-    // Read the argument from the stack.
+    // Read the argument from the stack and return it.
     __ lea(edx, Operand(edx, ecx, times_2, 0));
     __ neg(ebx);
     __ mov(eax, Operand(edx, ebx, times_2, kDisplacement));
+    __ ret(0);
+  } else {
+    ASSERT(type_ == NEW_OBJECT);
+    // Patch the arguments.length and the parameters pointer.
+    __ mov(ecx, Operand(edx, ArgumentsAdaptorFrameConstants::kLengthOffset));
+    __ mov(Operand(esp, 1 * kPointerSize), ecx);
+    __ lea(edx, Operand(edx, ecx, times_2, kDisplacement + 1 * kPointerSize));
+    __ mov(Operand(esp, 2 * kPointerSize), edx);
+    __ bind(&slow);
+    __ TailCallRuntime(ExternalReference(Runtime::kNewArgumentsFast), 3);
   }
 
-  // Return the length or the argument.
-  __ ret(0);
-
   // Slow-case: Handle non-smi or out-of-bounds access to arguments
   // by calling the runtime system.
-  if (!is_length_) {
+  if (type_ == READ_ELEMENT) {
     __ bind(&slow);
     __ TailCallRuntime(ExternalReference(Runtime::kGetArgumentsProperty), 1);
   }

src/codegen.h

@@ -342,6 +342,33 @@ class JSConstructEntryStub : public JSEntryStub {
 };
 
 
+class ArgumentsAccessStub: public CodeStub {
+ public:
+  enum Type {
+    READ_LENGTH,
+    READ_ELEMENT,
+    NEW_OBJECT
+  };
+
+  explicit ArgumentsAccessStub(Type type) : type_(type) { }
+
+ private:
+  Type type_;
+
+  Major MajorKey() { return ArgumentsAccess; }
+  int MinorKey() { return type_; }
+  void Generate(MacroAssembler* masm);
+
+  const char* GetName() { return "ArgumentsAccessStub"; }
+
+#ifdef DEBUG
+  void Print() {
+    PrintF("ArgumentsAccessStub (type %d)\n", type_);
+  }
+#endif
+};
+
+
 }  // namespace internal
 }  // namespace v8
 

src/runtime.cc

@@ -2703,6 +2703,9 @@ static Object* Runtime_Math_tan(Arguments args) {
 }
 
 
+// The NewArguments function is only used when constructing the
+// arguments array when calling non-functions from JavaScript in
+// runtime.js:CALL_NON_FUNCTION.
 static Object* Runtime_NewArguments(Arguments args) {
   NoHandleAllocation ha;
   ASSERT(args.length() == 1);
@@ -2727,6 +2730,26 @@ static Object* Runtime_NewArguments(Arguments args) {
 }
 
 
+static Object* Runtime_NewArgumentsFast(Arguments args) {
+  NoHandleAllocation ha;
+  ASSERT(args.length() == 3);
+
+  JSFunction* callee = JSFunction::cast(args[0]);
+  Object** parameters = reinterpret_cast<Object**>(args[1]);
+  const int length = Smi::cast(args[2])->value();
+
+  Object* result = Heap::AllocateArgumentsObject(callee, length);
+  if (result->IsFailure()) return result;
+  FixedArray* array = FixedArray::cast(JSObject::cast(result)->elements());
+  ASSERT(array->length() == length);
+  FixedArray::WriteBarrierMode mode = array->GetWriteBarrierMode();
+  for (int i = 0; i < length; i++) {
+    array->set(i, *--parameters, mode);
+  }
+  return result;
+}
+
+
 static Object* Runtime_NewClosure(Arguments args) {
   HandleScope scope;
   ASSERT(args.length() == 2);

src/runtime.h

@@ -57,6 +57,7 @@ namespace v8 { namespace internal {
   F(GetCalledFunction, 0) \
   F(GetFunctionDelegate, 1) \
   F(NewArguments, 1) \
+  F(NewArgumentsFast, 3) \
   F(LazyCompile, 1) \
   F(SetNewFunctionAttributes, 1) \
   \

test/mjsunit/fuzz-natives.js

@@ -109,6 +109,7 @@ var knownProblems = {
   
   // These functions should not be callable as runtime functions.
   "NewContext": true,
+  "NewArgumentsFast": true,
   "PushContext": true,
   "LazyCompile": true,
   "CreateObjectLiteralBoilerplate": true,