From 5458eac183c2affc4d2b593ed13ba9563fdaefa6 Mon Sep 17 00:00:00 2001 From: "kasperl@chromium.org" Date: Mon, 6 Oct 2008 06:08:15 +0000 Subject: [PATCH] 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 | 87 ++++++++++++++++++++++---------------------- src/codegen-ia32.cc | 84 ++++++++++++++++++++---------------------- src/codegen.h | 27 ++++++++++++++ src/runtime.cc | 23 ++++++++++++ src/runtime.h | 1 + test/mjsunit/fuzz-natives.js | 1 + 6 files changed, 136 insertions(+), 87 deletions(-) diff --git a/src/codegen-arm.cc b/src/codegen-arm.cc index 5f7c23d..89fc0db 100644 --- a/src/codegen-arm.cc +++ b/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* 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* 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); diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc index e8359c3..9f97626 100644 --- a/src/codegen-ia32.cc +++ b/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* 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* 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); } diff --git a/src/codegen.h b/src/codegen.h index 894e03c..2367aef 100644 --- a/src/codegen.h +++ b/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 diff --git a/src/runtime.cc b/src/runtime.cc index f75a5bc..25ac6f3 100644 --- a/src/runtime.cc +++ b/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(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); diff --git a/src/runtime.h b/src/runtime.h index b7c5717..fcded39 100644 --- a/src/runtime.h +++ b/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) \ \ diff --git a/test/mjsunit/fuzz-natives.js b/test/mjsunit/fuzz-natives.js index fbebfb9..315270c 100644 --- a/test/mjsunit/fuzz-natives.js +++ b/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, -- 2.7.4