From: kasperl@chromium.org Date: Wed, 24 Jun 2009 08:01:38 +0000 (+0000) Subject: Allocate arguments object on-demand instead of at function entry. X-Git-Tag: upstream/4.7.83~23830 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f66ea38c0beacc049e4b897035d0b64f880f784c;p=platform%2Fupstream%2Fv8.git Allocate arguments object on-demand instead of at function entry. This allows Function.prototype.apply to not allocate the objects and copy the arguments directly from the stack. Review URL: http://codereview.chromium.org/147075 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2256 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/accessors.cc b/src/accessors.cc index ac6cdf95a..82ae702fd 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -511,7 +511,10 @@ Object* Accessors::FunctionGetArguments(Object* object, void*) { // If there is an arguments variable in the stack, we return that. int index = ScopeInfo<>::StackSlotIndex(frame->code(), Heap::arguments_symbol()); - if (index >= 0) return frame->GetExpression(index); + if (index >= 0) { + Handle arguments = Handle(frame->GetExpression(index)); + if (!arguments->IsTheHole()) return *arguments; + } // If there isn't an arguments variable in the stack, we need to // find the frame that holds the actual arguments passed to the diff --git a/src/ast.cc b/src/ast.cc index eef8da715..d8a323267 100644 --- a/src/ast.cc +++ b/src/ast.cc @@ -68,7 +68,7 @@ VariableProxy::VariableProxy(Handle name, // names must be canonicalized for fast equality checks ASSERT(name->IsSymbol()); // at least one access, otherwise no need for a VariableProxy - var_uses_.RecordAccess(1); + var_uses_.RecordRead(1); } diff --git a/src/ast.h b/src/ast.h index 80a4aa5f2..15d762f05 100644 --- a/src/ast.h +++ b/src/ast.h @@ -802,13 +802,20 @@ class VariableProxy: public Expression { Variable* AsVariable() { return this == NULL || var_ == NULL ? NULL : var_->AsVariable(); } + virtual bool IsValidLeftHandSide() { return var_ == NULL ? true : var_->IsValidLeftHandSide(); } + bool IsVariable(Handle n) { return !is_this() && name().is_identical_to(n); } + bool IsArguments() { + Variable* variable = AsVariable(); + return (variable == NULL) ? false : variable->is_arguments(); + } + // If this assertion fails it means that some code has tried to // treat the special "this" variable as an ordinary variable with // the name "this". @@ -890,12 +897,13 @@ class Slot: public Expression { virtual void Accept(AstVisitor* v); // Type testing & conversion - virtual Slot* AsSlot() { return this; } + virtual Slot* AsSlot() { return this; } // Accessors - Variable* var() const { return var_; } - Type type() const { return type_; } - int index() const { return index_; } + Variable* var() const { return var_; } + Type type() const { return type_; } + int index() const { return index_; } + bool is_arguments() const { return var_->is_arguments(); } private: Variable* var_; diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index eed0b14c0..0228be938 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -175,18 +175,7 @@ void CodeGenerator::GenCode(FunctionLiteral* fun) { function_return_.set_direction(JumpTarget::BIDIRECTIONAL); function_return_is_shadowed_ = false; - // 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); - frame_->PushFunction(); - frame_->PushReceiverSlotAddress(); - frame_->Push(Smi::FromInt(scope_->num_parameters())); - Result answer = frame_->CallStub(&stub, 3); - frame_->Push(&answer); - } - + // Allocate the local context if needed. if (scope_->num_heap_slots() > 0) { Comment cmnt(masm_, "[ allocate local context"); // Allocate local context. @@ -247,27 +236,11 @@ void CodeGenerator::GenCode(FunctionLiteral* fun) { } } - // This section stores the pointer to the arguments object that - // was allocated and copied into above. If the address was not - // saved to TOS, we push ecx onto the stack. - // // Store the arguments object. This must happen after context - // initialization because the arguments object may be stored in the - // context. - if (scope_->arguments() != NULL) { - Comment cmnt(masm_, "[ store arguments object"); - { Reference shadow_ref(this, scope_->arguments_shadow()); - ASSERT(shadow_ref.is_slot()); - { Reference arguments_ref(this, scope_->arguments()); - ASSERT(arguments_ref.is_slot()); - // Here we rely on the convenient property that references to slot - // take up zero space in the frame (ie, it doesn't matter that the - // stored value is actually below the reference on the frame). - arguments_ref.SetValue(NOT_CONST_INIT); - } - shadow_ref.SetValue(NOT_CONST_INIT); - } - frame_->Drop(); // Value is no longer needed. + // initialization because the arguments object may be stored in + // the context. + if (ArgumentsMode() != NO_ARGUMENTS_ALLOCATION) { + StoreArgumentsObject(true); } // Generate code to 'execute' declarations and initialize functions @@ -591,6 +564,71 @@ void CodeGenerator::LoadTypeofExpression(Expression* x) { } +ArgumentsAllocationMode CodeGenerator::ArgumentsMode() const { + if (scope_->arguments() == NULL) return NO_ARGUMENTS_ALLOCATION; + ASSERT(scope_->arguments_shadow() != NULL); + // We don't want to do lazy arguments allocation for functions that + // have heap-allocated contexts, because it interfers with the + // uninitialized const tracking in the context objects. + return (scope_->num_heap_slots() > 0) + ? EAGER_ARGUMENTS_ALLOCATION + : LAZY_ARGUMENTS_ALLOCATION; +} + + +Result CodeGenerator::StoreArgumentsObject(bool initial) { + ArgumentsAllocationMode mode = ArgumentsMode(); + ASSERT(mode != NO_ARGUMENTS_ALLOCATION); + + Comment cmnt(masm_, "[ store arguments object"); + if (mode == LAZY_ARGUMENTS_ALLOCATION && initial) { + // When using lazy arguments allocation, we store the hole value + // as a sentinel indicating that the arguments object hasn't been + // allocated yet. + frame_->Push(Factory::the_hole_value()); + } else { + ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT); + frame_->PushFunction(); + frame_->PushReceiverSlotAddress(); + frame_->Push(Smi::FromInt(scope_->num_parameters())); + Result result = frame_->CallStub(&stub, 3); + frame_->Push(&result); + } + + { Reference shadow_ref(this, scope_->arguments_shadow()); + Reference arguments_ref(this, scope_->arguments()); + ASSERT(shadow_ref.is_slot() && arguments_ref.is_slot()); + // Here we rely on the convenient property that references to slot + // take up zero space in the frame (ie, it doesn't matter that the + // stored value is actually below the reference on the frame). + JumpTarget done; + bool skip_arguments = false; + if (mode == LAZY_ARGUMENTS_ALLOCATION && !initial) { + // We have to skip storing into the arguments slot if it has + // already been written to. This can happen if the a function + // has a local variable named 'arguments'. + LoadFromSlot(scope_->arguments()->var()->slot(), NOT_INSIDE_TYPEOF); + Result arguments = frame_->Pop(); + if (arguments.is_constant()) { + // We have to skip updating the arguments object if it has + // been assigned a proper value. + skip_arguments = !arguments.handle()->IsTheHole(); + } else { + __ cmp(Operand(arguments.reg()), Immediate(Factory::the_hole_value())); + arguments.Unuse(); + done.Branch(not_equal); + } + } + if (!skip_arguments) { + arguments_ref.SetValue(NOT_CONST_INIT); + if (mode == LAZY_ARGUMENTS_ALLOCATION) done.Bind(); + } + shadow_ref.SetValue(NOT_CONST_INIT); + } + return frame_->Pop(); +} + + Reference::Reference(CodeGenerator* cgen, Expression* expression) : cgen_(cgen), expression_(expression), type_(ILLEGAL) { cgen->LoadReference(this); @@ -2090,6 +2128,172 @@ void CodeGenerator::CallWithArguments(ZoneList* args, } +void CodeGenerator::CallApplyLazy(Property* apply, + Expression* receiver, + VariableProxy* arguments, + int position) { + ASSERT(ArgumentsMode() == LAZY_ARGUMENTS_ALLOCATION); + ASSERT(arguments->IsArguments()); + + JumpTarget slow, done; + + // Load the apply function onto the stack. This will usually + // give us a megamorphic load site. Not super, but it works. + Reference ref(this, apply); + ref.GetValue(NOT_INSIDE_TYPEOF); + ASSERT(ref.type() == Reference::NAMED); + + // Load the receiver and the existing arguments object onto the + // expression stack. Avoid allocating the arguments object here. + Load(receiver); + LoadFromSlot(scope_->arguments()->var()->slot(), NOT_INSIDE_TYPEOF); + + // Emit the source position information after having loaded the + // receiver and the arguments. + CodeForSourcePosition(position); + + // Check if the arguments object has been lazily allocated + // already. If so, just use that instead of copying the arguments + // from the stack. This also deals with cases where a local variable + // named 'arguments' has been introduced. + frame_->Dup(); + Result probe = frame_->Pop(); + bool try_lazy = true; + if (probe.is_constant()) { + try_lazy = probe.handle()->IsTheHole(); + } else { + __ cmp(Operand(probe.reg()), Immediate(Factory::the_hole_value())); + probe.Unuse(); + slow.Branch(not_equal); + } + + if (try_lazy) { + JumpTarget build_args; + + // Get rid of the arguments object probe. + frame_->Drop(); + + // Before messing with the execution stack, we sync all + // elements. This is bound to happen anyway because we're + // about to call a function. + frame_->SyncRange(0, frame_->element_count() - 1); + + // Check that the receiver really is a JavaScript object. + { frame_->PushElementAt(0); + Result receiver = frame_->Pop(); + receiver.ToRegister(); + __ test(receiver.reg(), Immediate(kSmiTagMask)); + build_args.Branch(zero); + Result tmp = allocator_->Allocate(); + __ CmpObjectType(receiver.reg(), FIRST_JS_OBJECT_TYPE, tmp.reg()); + build_args.Branch(less); + __ cmp(tmp.reg(), LAST_JS_OBJECT_TYPE); + build_args.Branch(greater); + } + + // Verify that we're invoking Function.prototype.apply. + { frame_->PushElementAt(1); + Result apply = frame_->Pop(); + apply.ToRegister(); + __ test(apply.reg(), Immediate(kSmiTagMask)); + build_args.Branch(zero); + Result tmp = allocator_->Allocate(); + __ CmpObjectType(apply.reg(), JS_FUNCTION_TYPE, tmp.reg()); + build_args.Branch(not_equal); + __ mov(tmp.reg(), + FieldOperand(apply.reg(), JSFunction::kSharedFunctionInfoOffset)); + Handle apply_code(Builtins::builtin(Builtins::FunctionApply)); + __ cmp(FieldOperand(tmp.reg(), SharedFunctionInfo::kCodeOffset), + Immediate(apply_code)); + build_args.Branch(not_equal); + } + + // Get the function receiver from the stack. Check that it + // really is a function. + __ mov(edi, Operand(esp, 2 * kPointerSize)); + __ test(edi, Immediate(kSmiTagMask)); + build_args.Branch(zero); + __ CmpObjectType(edi, JS_FUNCTION_TYPE, ecx); + build_args.Branch(not_equal); + + // Copy the arguments to this function possibly from the + // adaptor frame below it. + Label invoke, adapted; + __ mov(edx, Operand(ebp, StandardFrameConstants::kCallerFPOffset)); + __ mov(ecx, Operand(edx, StandardFrameConstants::kContextOffset)); + __ cmp(ecx, ArgumentsAdaptorFrame::SENTINEL); + __ j(equal, &adapted); + + // No arguments adaptor frame. Copy fixed number of arguments. + __ mov(eax, Immediate(scope_->num_parameters())); + for (int i = 0; i < scope_->num_parameters(); i++) { + __ push(frame_->ParameterAt(i)); + } + __ jmp(&invoke); + + // Arguments adaptor frame present. Copy arguments from there, but + // avoid copying too many arguments to avoid stack overflows. + __ bind(&adapted); + static const uint32_t kArgumentsLimit = 1 * KB; + __ mov(eax, Operand(edx, ArgumentsAdaptorFrameConstants::kLengthOffset)); + __ shr(eax, kSmiTagSize); + __ mov(ecx, Operand(eax)); + __ cmp(eax, kArgumentsLimit); + build_args.Branch(above); + + // Loop through the arguments pushing them onto the execution + // stack. We don't inform the virtual frame of the push, so we don't + // have to worry about getting rid of the elements from the virtual + // frame. + Label loop; + __ bind(&loop); + __ test(ecx, Operand(ecx)); + __ j(zero, &invoke); + __ push(Operand(edx, ecx, times_4, 1 * kPointerSize)); + __ dec(ecx); + __ jmp(&loop); + + // Invoke the function. The virtual frame knows about the receiver + // so make sure to forget that explicitly. + __ bind(&invoke); + ParameterCount actual(eax); + __ InvokeFunction(edi, actual, CALL_FUNCTION); + frame_->Forget(1); + Result result = allocator()->Allocate(eax); + frame_->SetElementAt(0, &result); + done.Jump(); + + // Slow-case: Allocate the arguments object since we know it isn't + // there, and fall-through to the slow-case where we call + // Function.prototype.apply. + build_args.Bind(); + Result arguments_object = StoreArgumentsObject(false); + frame_->Push(&arguments_object); + slow.Bind(); + } + + // Flip the apply function and the function to call on the stack, so + // the function looks like the receiver of the apply call. This way, + // the generic Function.prototype.apply implementation can deal with + // the call like it usually does. + Result a2 = frame_->Pop(); + Result a1 = frame_->Pop(); + Result ap = frame_->Pop(); + Result fn = frame_->Pop(); + frame_->Push(&ap); + frame_->Push(&fn); + frame_->Push(&a1); + frame_->Push(&a2); + CallFunctionStub call_function(2, NOT_IN_LOOP); + Result res = frame_->CallStub(&call_function, 3); + frame_->Push(&res); + + // All done. Restore context register after call. + if (try_lazy) done.Bind(); + frame_->RestoreContextRegister(); +} + + class DeferredStackCheck: public DeferredCode { public: DeferredStackCheck() { @@ -3615,6 +3819,44 @@ void CodeGenerator::LoadFromSlot(Slot* slot, TypeofState typeof_state) { } +void CodeGenerator::LoadFromSlotCheckForArguments(Slot* slot, + TypeofState state) { + LoadFromSlot(slot, state); + + // Bail out quickly if we're not using lazy arguments allocation. + if (ArgumentsMode() != LAZY_ARGUMENTS_ALLOCATION) return; + + // ... or if the slot isn't a non-parameter arguments slot. + if (slot->type() == Slot::PARAMETER || !slot->is_arguments()) return; + + // Pop the loaded value from the stack. + Result value = frame_->Pop(); + + // If the loaded value is a constant, we know if the arguments + // object has been lazily loaded yet. + if (value.is_constant()) { + if (value.handle()->IsTheHole()) { + Result arguments = StoreArgumentsObject(false); + frame_->Push(&arguments); + } else { + frame_->Push(&value); + } + return; + } + + // The loaded value is in a register. If it is the sentinel that + // indicates that we haven't loaded the arguments object yet, we + // need to do it now. + JumpTarget exit; + __ cmp(Operand(value.reg()), Immediate(Factory::the_hole_value())); + frame_->Push(&value); + exit.Branch(not_equal); + Result arguments = StoreArgumentsObject(false); + frame_->SetElementAt(0, &arguments); + exit.Bind(); +} + + Result CodeGenerator::LoadFromGlobalSlotCheckExtensions( Slot* slot, TypeofState typeof_state, @@ -3787,7 +4029,7 @@ void CodeGenerator::StoreToSlot(Slot* slot, InitState init_state) { void CodeGenerator::VisitSlot(Slot* node) { Comment cmnt(masm_, "[ Slot"); - LoadFromSlot(node, typeof_state()); + LoadFromSlotCheckForArguments(node, typeof_state()); } @@ -4349,23 +4591,40 @@ void CodeGenerator::VisitCall(Call* node) { // JavaScript example: 'object.foo(1, 2, 3)' or 'map["key"](1, 2, 3)' // ------------------------------------------------------------------ - // Push the name of the function and the receiver onto the stack. - frame_->Push(literal->handle()); - Load(property->obj()); + Handle name = Handle::cast(literal->handle()); - // Load the arguments. - int arg_count = args->length(); - for (int i = 0; i < arg_count; i++) { - Load(args->at(i)); - } + if (ArgumentsMode() == LAZY_ARGUMENTS_ALLOCATION && + name->IsEqualTo(CStrVector("apply")) && + args->length() == 2 && + args->at(1)->AsVariableProxy() != NULL && + args->at(1)->AsVariableProxy()->IsArguments()) { + // Use the optimized Function.prototype.apply that avoids + // allocating lazily allocated arguments objects. + CallApplyLazy(property, + args->at(0), + args->at(1)->AsVariableProxy(), + node->position()); - // Call the IC initialization code. - CodeForSourcePosition(node->position()); - Result result = - frame_->CallCallIC(RelocInfo::CODE_TARGET, arg_count, loop_nesting()); - frame_->RestoreContextRegister(); - // Replace the function on the stack with the result. - frame_->SetElementAt(0, &result); + } else { + // Push the name of the function and the receiver onto the stack. + frame_->Push(name); + Load(property->obj()); + + // Load the arguments. + int arg_count = args->length(); + for (int i = 0; i < arg_count; i++) { + Load(args->at(i)); + } + + // Call the IC initialization code. + CodeForSourcePosition(node->position()); + Result result = + frame_->CallCallIC(RelocInfo::CODE_TARGET, arg_count, + loop_nesting()); + frame_->RestoreContextRegister(); + // Replace the function on the stack with the result. + frame_->SetElementAt(0, &result); + } } else { // ------------------------------------------- @@ -5838,7 +6097,7 @@ void Reference::GetValue(TypeofState typeof_state) { Comment cmnt(masm, "[ Load from Slot"); Slot* slot = expression_->AsVariableProxy()->AsVariable()->slot(); ASSERT(slot != NULL); - cgen_->LoadFromSlot(slot, typeof_state); + cgen_->LoadFromSlotCheckForArguments(slot, typeof_state); break; } diff --git a/src/ia32/codegen-ia32.h b/src/ia32/codegen-ia32.h index 7bf1c63cb..d25d07c7e 100644 --- a/src/ia32/codegen-ia32.h +++ b/src/ia32/codegen-ia32.h @@ -273,6 +273,14 @@ class CodeGenState BASE_EMBEDDED { }; +// ------------------------------------------------------------------------- +// Arguments allocation mode + +enum ArgumentsAllocationMode { + NO_ARGUMENTS_ALLOCATION, + EAGER_ARGUMENTS_ALLOCATION, + LAZY_ARGUMENTS_ALLOCATION +}; // ------------------------------------------------------------------------- @@ -332,12 +340,11 @@ class CodeGenerator: public AstVisitor { // Accessors Scope* scope() const { return scope_; } + bool is_eval() { return is_eval_; } // Generating deferred code. void ProcessDeferred(); - bool is_eval() { return is_eval_; } - // State TypeofState typeof_state() const { return state_->typeof_state(); } ControlDestination* destination() const { return state_->destination(); } @@ -373,6 +380,12 @@ class CodeGenerator: public AstVisitor { // target (which can not be done more than once). void GenerateReturnSequence(Result* return_value); + // Returns the arguments allocation mode. + ArgumentsAllocationMode ArgumentsMode() const; + + // Store the arguments object and allocate it if necessary. + Result StoreArgumentsObject(bool initial); + // The following are used by class Reference. void LoadReference(Reference* ref); void UnloadReference(Reference* ref); @@ -408,6 +421,7 @@ class CodeGenerator: public AstVisitor { // Read a value from a slot and leave it on top of the expression stack. void LoadFromSlot(Slot* slot, TypeofState typeof_state); + void LoadFromSlotCheckForArguments(Slot* slot, TypeofState typeof_state); Result LoadFromGlobalSlotCheckExtensions(Slot* slot, TypeofState typeof_state, JumpTarget* slow); @@ -470,6 +484,14 @@ class CodeGenerator: public AstVisitor { void CallWithArguments(ZoneList* arguments, int position); + // Use an optimized version of Function.prototype.apply that avoid + // allocating the arguments object and just copies the arguments + // from the stack. + void CallApplyLazy(Property* apply, + Expression* receiver, + VariableProxy* arguments, + int position); + void CheckStack(); struct InlineRuntimeLUT { diff --git a/src/parser.cc b/src/parser.cc index c2ec499a8..2b4be79a0 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -1582,7 +1582,8 @@ VariableProxy* AstBuildingParser::Declare(Handle name, // For global const variables we bind the proxy to a variable. if (mode == Variable::CONST && top_scope_->is_global_scope()) { ASSERT(resolve); // should be set by all callers - var = NEW(Variable(top_scope_, name, Variable::CONST, true, false)); + Variable::Kind kind = Variable::NORMAL; + var = NEW(Variable(top_scope_, name, Variable::CONST, true, kind)); } // If requested and we have a local variable, bind the proxy to the variable diff --git a/src/runtime.js b/src/runtime.js index d4b49704a..df26b8894 100644 --- a/src/runtime.js +++ b/src/runtime.js @@ -391,8 +391,9 @@ function CALL_NON_FUNCTION_AS_CONSTRUCTOR() { function APPLY_PREPARE(args) { var length; - // First check whether length is a positive Smi and args is an array. This is the - // fast case. If this fails, we do the slow case that takes care of more eventualities + // First check whether length is a positive Smi and args is an + // array. This is the fast case. If this fails, we do the slow case + // that takes care of more eventualities. if (%_IsArray(args)) { length = args.length; if (%_IsSmi(length) && length >= 0 && length < 0x800000 && IS_FUNCTION(this)) { diff --git a/src/scopes.cc b/src/scopes.cc index 7122eb03c..88b1c66f3 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -81,12 +81,12 @@ Variable* LocalsMap::Declare(Scope* scope, Handle name, Variable::Mode mode, bool is_valid_LHS, - bool is_this) { + Variable::Kind kind) { HashMap::Entry* p = HashMap::Lookup(name.location(), name->Hash(), true); if (p->value == NULL) { // The variable has not been declared yet -> insert it. ASSERT(p->key == name.location()); - p->value = new Variable(scope, name, mode, is_valid_LHS, is_this); + p->value = new Variable(scope, name, mode, is_valid_LHS, kind); } return reinterpret_cast(p->value); } @@ -169,7 +169,8 @@ void Scope::Initialize(bool inside_with) { // such parameter is 'this' which is passed on the stack when // invoking scripts { Variable* var = - locals_.Declare(this, Factory::this_symbol(), Variable::VAR, false, true); + locals_.Declare(this, Factory::this_symbol(), Variable::VAR, + false, Variable::THIS); var->rewrite_ = new Slot(var, Slot::PARAMETER, -1); receiver_ = new VariableProxy(Factory::this_symbol(), true, false); receiver_->BindTo(var); @@ -179,7 +180,8 @@ void Scope::Initialize(bool inside_with) { // Declare 'arguments' variable which exists in all functions. // Note that it may never be accessed, in which case it won't // be allocated during variable allocation. - Declare(Factory::arguments_symbol(), Variable::VAR); + locals_.Declare(this, Factory::arguments_symbol(), Variable::VAR, + true, Variable::ARGUMENTS); } } @@ -203,7 +205,7 @@ Variable* Scope::Lookup(Handle name) { Variable* Scope::DeclareFunctionVar(Handle name) { ASSERT(is_function_scope() && function_ == NULL); - function_ = new Variable(this, name, Variable::CONST, true, false); + function_ = new Variable(this, name, Variable::CONST, true, Variable::NORMAL); return function_; } @@ -213,7 +215,7 @@ Variable* Scope::Declare(Handle name, Variable::Mode mode) { // INTERNAL variables are allocated explicitly, and TEMPORARY // variables are allocated via NewTemporary(). ASSERT(mode == Variable::VAR || mode == Variable::CONST); - return locals_.Declare(this, name, mode, true, false); + return locals_.Declare(this, name, mode, true, Variable::NORMAL); } @@ -247,7 +249,8 @@ void Scope::RemoveUnresolved(VariableProxy* var) { VariableProxy* Scope::NewTemporary(Handle name) { - Variable* var = new Variable(this, name, Variable::TEMPORARY, true, false); + Variable* var = new Variable(this, name, Variable::TEMPORARY, true, + Variable::NORMAL); VariableProxy* tmp = new VariableProxy(name, false, false); tmp->BindTo(var); temps_.Add(var); @@ -503,7 +506,7 @@ Variable* Scope::NonLocal(Handle name, Variable::Mode mode) { Variable* var = map->Lookup(name); if (var == NULL) { // Declare a new non-local. - var = map->Declare(NULL, name, mode, true, false); + var = map->Declare(NULL, name, mode, true, Variable::NORMAL); // Allocate it by giving it a dynamic lookup. var->rewrite_ = new Slot(var, Slot::LOOKUP, -1); } @@ -619,7 +622,7 @@ void Scope::ResolveVariable(Scope* global_scope, // We must have a global variable. ASSERT(global_scope != NULL); var = new Variable(global_scope, proxy->name(), - Variable::DYNAMIC, true, false); + Variable::DYNAMIC, true, Variable::NORMAL); } else if (scope_inside_with_) { // If we are inside a with statement we give up and look up @@ -797,7 +800,7 @@ void Scope::AllocateParameterLocals() { // are never allocated in the context). Variable* arguments_shadow = new Variable(this, Factory::arguments_shadow_symbol(), - Variable::INTERNAL, true, false); + Variable::INTERNAL, true, Variable::ARGUMENTS); arguments_shadow_ = new VariableProxy(Factory::arguments_shadow_symbol(), false, false); arguments_shadow_->BindTo(arguments_shadow); diff --git a/src/scopes.h b/src/scopes.h index b2f61ef66..ea4e0f746 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -47,7 +47,7 @@ class LocalsMap: public HashMap { virtual ~LocalsMap(); Variable* Declare(Scope* scope, Handle name, Variable::Mode mode, - bool is_valid_LHS, bool is_this); + bool is_valid_LHS, Variable::Kind kind); Variable* Lookup(Handle name); }; diff --git a/src/variables.cc b/src/variables.cc index 6c9f82f08..d9a78a5e7 100644 --- a/src/variables.cc +++ b/src/variables.cc @@ -140,12 +140,12 @@ Variable::Variable(Scope* scope, Handle name, Mode mode, bool is_valid_LHS, - bool is_this) + Kind kind) : scope_(scope), name_(name), mode_(mode), is_valid_LHS_(is_valid_LHS), - is_this_(is_this), + kind_(kind), local_if_not_shadowed_(NULL), is_accessed_from_inner_scope_(false), rewrite_(NULL) { diff --git a/src/variables.h b/src/variables.h index 50620718c..c0d14356f 100644 --- a/src/variables.h +++ b/src/variables.h @@ -137,6 +137,12 @@ class Variable: public ZoneObject { // in a context }; + enum Kind { + NORMAL, + THIS, + ARGUMENTS + }; + // Printing support static const char* Mode2String(Mode mode); @@ -172,7 +178,8 @@ class Variable: public ZoneObject { } bool is_global() const; - bool is_this() const { return is_this_; } + bool is_this() const { return kind_ == THIS; } + bool is_arguments() const { return kind_ == ARGUMENTS; } Variable* local_if_not_shadowed() const { ASSERT(mode_ == DYNAMIC_LOCAL && local_if_not_shadowed_ != NULL); @@ -190,13 +197,13 @@ class Variable: public ZoneObject { private: Variable(Scope* scope, Handle name, Mode mode, bool is_valid_LHS, - bool is_this); + Kind kind); Scope* scope_; Handle name_; Mode mode_; bool is_valid_LHS_; - bool is_this_; + Kind kind_; Variable* local_if_not_shadowed_; diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index e48a1b659..1a78895a5 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -1257,6 +1257,10 @@ void CodeGenerator::VisitCall(Call* node) { // JavaScript example: 'object.foo(1, 2, 3)' or 'map["key"](1, 2, 3)' // ------------------------------------------------------------------ + // TODO(X64): Consider optimizing Function.prototype.apply calls + // with arguments object. Requires lazy arguments allocation; + // see http://codereview.chromium.org/147075. + // Push the name of the function and the receiver onto the stack. frame_->Push(literal->handle()); Load(property->obj()); diff --git a/test/mjsunit/arguments-apply.js b/test/mjsunit/arguments-apply.js new file mode 100644 index 000000000..d8172ccf1 --- /dev/null +++ b/test/mjsunit/arguments-apply.js @@ -0,0 +1,127 @@ +// Copyright 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 +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +function ReturnArguments() { + return arguments; +} + +function ReturnReceiver() { + return this; +} + + +function Global() { + return ReturnArguments.apply(this, arguments); +} + +assertEquals(0, Global().length); +assertEquals(1, Global(1).length); +assertEquals(2, Global(2)[0]); +assertEquals(2, Global(3, 4).length); +assertEquals(3, Global(3, 4)[0]); +assertEquals(4, Global(3, 4)[1]); + + +function Local() { + var object = { f: ReturnArguments }; + return object.f.apply(this, arguments); +} + +assertEquals(0, Local().length); +assertEquals(1, Local(1).length); +assertEquals(2, Local(2)[0]); +assertEquals(2, Local(3, 4).length); +assertEquals(3, Local(3, 4)[0]); +assertEquals(4, Local(3, 4)[1]); + + +function ShadowArguments() { + var arguments = [3, 4]; + return ReturnArguments.apply(this, arguments); +} + +assertEquals(2, ShadowArguments().length); +assertEquals(3, ShadowArguments()[0]); +assertEquals(4, ShadowArguments()[1]); + + +function NonObjectReceiver(receiver) { + return ReturnReceiver.apply(receiver, arguments); +} + +assertEquals(42, NonObjectReceiver(42)); +assertEquals("object", typeof NonObjectReceiver(42)); +assertTrue(NonObjectReceiver(42) instanceof Number); +assertTrue(this === NonObjectReceiver(null)); +assertTrue(this === NonObjectReceiver(void 0)); + + +function ShadowApply() { + function f() { return 42; } + f.apply = function() { return 87; } + return f.apply(this, arguments); +} + +assertEquals(87, ShadowApply()); +assertEquals(87, ShadowApply(1)); +assertEquals(87, ShadowApply(1, 2)); + + +function CallNonFunction() { + var object = { apply: Function.prototype.apply }; + return object.apply(this, arguments); +} + +assertThrows(CallNonFunction, TypeError); + + +// Make sure that the stack after the apply optimization is +// in a valid state. +function SimpleStackCheck() { + var sentinel = 42; + var result = ReturnArguments.apply(this, arguments); + assertTrue(result != null); + assertEquals(42, sentinel); +} + +SimpleStackCheck(); + + +function ShadowArgumentsWithConstant() { + var arguments = null; + return ReturnArguments.apply(this, arguments); +} + +assertEquals(0, ShadowArgumentsWithConstant().length); +assertEquals(0, ShadowArgumentsWithConstant(1).length); +assertEquals(0, ShadowArgumentsWithConstant(1, 2).length); + + +// Make sure we can deal with unfolding lots of arguments on the +// stack even in the presence of the apply optimizations. +var array = new Array(2048); +assertEquals(2048, Global.apply(this, array).length); diff --git a/test/mjsunit/arguments-lazy.js b/test/mjsunit/arguments-lazy.js new file mode 100644 index 000000000..794afc36b --- /dev/null +++ b/test/mjsunit/arguments-lazy.js @@ -0,0 +1,47 @@ +// Copyright 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 +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Make sure we don't allocate the arguments object over and +// over again. +function SharedLazyArguments() { + return arguments === arguments; +} + +assertTrue(SharedLazyArguments()); + + +// Make sure that accessing arguments doesn't clobber any +// local variables called arguments. +function ArgumentsOverride(x) { + var arguments = 42; + x = x ? x : 0; + return x + arguments; +} + +assertEquals(42, ArgumentsOverride()); +assertEquals(43, ArgumentsOverride(1)); +assertEquals(44, ArgumentsOverride(2,3));