Lasse, please also create a compiler test of the same 'arguments' parameter being used in different contexts (eg, effect+value, effect+test, value+test, etc.) to verify that this approach works.
On Thu, Nov 12, 2009 at 6:38 AM, <[email protected]> wrote: > > Revision: 3289 > Author: [email protected] > Date: Thu Nov 12 03:38:01 2009 > Log: Fast-codegen: Added support for arguments in functions. > Functions using "arguments" have their arguments object created on entry. > Also added support for variables rewritten into argument object property > access. > > Review URL: http://codereview.chromium.org/384078 > > http://code.google.com/p/v8/source/detail?r=3289 > > Modified: > /branches/bleeding_edge/src/arm/fast-codegen-arm.cc > /branches/bleeding_edge/src/compiler.cc > /branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc > /branches/bleeding_edge/src/x64/fast-codegen-x64.cc > > ======================================= > --- /branches/bleeding_edge/src/arm/fast-codegen-arm.cc Mon Nov 9 01:56:57 > 2009 > +++ /branches/bleeding_edge/src/arm/fast-codegen-arm.cc Thu Nov 12 03:38:01 > 2009 > @@ -55,26 +55,56 @@ > void FastCodeGenerator::Generate(FunctionLiteral* fun) { > function_ = fun; > SetFunctionPosition(fun); > + int locals_count = fun->scope()->num_stack_slots(); > > __ stm(db_w, sp, r1.bit() | cp.bit() | fp.bit() | lr.bit()); > + if (locals_count > 0) { > + // Load undefined value here, so the value is ready for the loop > below. > + __ LoadRoot(ip, Heap::kUndefinedValueRootIndex); > + } > // Adjust fp to point to caller's fp. > __ add(fp, sp, Operand(2 * kPointerSize)); > > { Comment cmnt(masm_, "[ Allocate locals"); > - int locals_count = fun->scope()->num_stack_slots(); > - if (locals_count > 0) { > - __ LoadRoot(ip, Heap::kUndefinedValueRootIndex); > - } > - __ LoadRoot(r2, Heap::kStackLimitRootIndex); > for (int i = 0; i < locals_count; i++) { > __ push(ip); > } > } > + > + bool function_in_register = true; > + > + Variable* arguments = fun->scope()->arguments()->AsVariable(); > + if (arguments != NULL) { > + // Function uses arguments object. > + Comment cmnt(masm_, "[ Allocate arguments object"); > + __ mov(r3, r1); > + // Receiver is just before the parameters on the caller's stack. > + __ add(r2, fp, Operand(StandardFrameConstants::kCallerSPOffset + > + fun->num_parameters() * kPointerSize)); > + __ mov(r1, Operand(Smi::FromInt(fun->num_parameters()))); > + __ stm(db_w, sp, r1.bit() | r2.bit() | r3.bit()); > + > + // Arguments to ArgumentsAccessStub: > + // function, receiver address, parameter count. > + // The stub will rewrite receiever and parameter count if the previous > + // stack frame was an arguments adapter frame. > + ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT); > + __ CallStub(&stub); > + __ str(r0, MemOperand(fp, SlotOffset(arguments->slot()))); > + Slot* dot_arguments_slot = > + fun->scope()->arguments_shadow()->AsVariable()->slot(); > + __ str(r0, MemOperand(fp, SlotOffset(dot_arguments_slot))); > + function_in_register = false; > + } > > // Possibly allocate a local context. > if (fun->scope()->num_heap_slots() > 0) { > Comment cmnt(masm_, "[ Allocate local context"); > - // Argument to NewContext is the function, still in r1. > + if (!function_in_register) { > + // Load this again, if it's used by the local context below. > + __ ldr(r1, MemOperand(fp, > JavaScriptFrameConstants::kFunctionOffset)); > + } > + // Argument to NewContext is the function, which is in r1. > __ push(r1); > __ CallRuntime(Runtime::kNewContext, 1); > // Context is returned in both r0 and cp. It replaces the context > @@ -94,6 +124,7 @@ > // added to the implicit 8 byte offset that always applies to operations > // with pc and gives a return address 12 bytes down. > Comment cmnt(masm_, "[ Stack check"); > + __ LoadRoot(r2, Heap::kStackLimitRootIndex); > __ add(lr, pc, Operand(Assembler::kInstrSize)); > __ cmp(sp, Operand(r2)); > StackCheckStub stub; > @@ -432,7 +463,7 @@ > Handle<Code> ic(Builtins::builtin(Builtins::LoadIC_Initialize)); > __ Call(ic, RelocInfo::CODE_TARGET_CONTEXT); > DropAndMove(expr->context(), r0); > - } else { > + } else if (rewrite->AsSlot() != NULL) { > Slot* slot = rewrite->AsSlot(); > ASSERT_NE(NULL, slot); > switch (slot->type()) { > @@ -474,6 +505,13 @@ > UNREACHABLE(); > break; > } > + } else { > + // The parameter variable has been rewritten into an explict access to > + // the arguments object. > + Property* property = rewrite->AsProperty(); > + ASSERT_NOT_NULL(property); > + ASSERT_EQ(expr->context(), property->context()); > + Visit(property); > } > } > > ======================================= > --- /branches/bleeding_edge/src/compiler.cc Wed Nov 11 01:50:06 2009 > +++ /branches/bleeding_edge/src/compiler.cc Thu Nov 12 03:38:01 2009 > @@ -598,11 +598,6 @@ > } > } > } > - > - if (scope->arguments() != NULL) { > - if (FLAG_trace_bailout) PrintF("function uses 'arguments'\n"); > - return NORMAL; > - } > > has_supported_syntax_ = true; > VisitDeclarations(scope->declarations()); > @@ -796,17 +791,21 @@ > if (rewrite != NULL) { > // Non-global. > Slot* slot = rewrite->AsSlot(); > - if (slot == NULL) { > - // This is a variable rewritten to an explicit property access > - // on the arguments object. > - BAILOUT("non-global/non-slot variable reference"); > - } > - > - Slot::Type type = slot->type(); > - // When LOOKUP slots are enabled, some currently dead code > - // implementing unary typeof will become live. > - if (type == Slot::LOOKUP) { > - BAILOUT("Lookup slot"); > + if (slot != NULL) { > + Slot::Type type = slot->type(); > + // When LOOKUP slots are enabled, some currently dead code > + // implementing unary typeof will become live. > + if (type == Slot::LOOKUP) { > + BAILOUT("Lookup slot"); > + } > + } else { > + Property* property = rewrite->AsProperty(); > + // In the presence of an arguments object, parameter variables > + // are rewritten into property accesses on that object. > + ASSERT_NOT_NULL(property); > + ASSERT_NE(Expression::kUninitialized, context_); > + Visit(property); > + property->set_context(context_); > } > } > } > ======================================= > --- /branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc Mon Nov 9 > 01:56:57 2009 > +++ /branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc Thu Nov 12 > 03:38:01 2009 > @@ -66,12 +66,43 @@ > __ push(Immediate(Factory::undefined_value())); > } > } > + > + bool function_in_register = true; > + > + Variable* arguments = fun->scope()->arguments()->AsVariable(); > + if (arguments != NULL) { > + // Function uses arguments object. > + Comment cmnt(masm_, "[ Allocate arguments object"); > + __ push(edi); > + // Receiver is just before the parameters on the caller's stack. > + __ lea(edx, Operand(ebp, StandardFrameConstants::kCallerSPOffset + > + fun->num_parameters() * kPointerSize)); > + __ push(edx); > + __ push(Immediate(Smi::FromInt(fun->num_parameters()))); > + // Arguments to ArgumentsAccessStub: > + // function, receiver address, parameter count. > + // The stub will rewrite receiever and parameter count if the previous > + // stack frame was an arguments adapter frame. > + ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT); > + __ CallStub(&stub); > + __ mov(Operand(ebp, SlotOffset(arguments->slot())), eax); > + Slot* dot_arguments_slot = > + fun->scope()->arguments_shadow()->AsVariable()->slot(); > + __ mov(Operand(ebp, SlotOffset(dot_arguments_slot)), eax); > + > + function_in_register = false; > + } > > // Possibly allocate a local context. > if (fun->scope()->num_heap_slots() > 0) { > Comment cmnt(masm_, "[ Allocate local context"); > - // Argument to NewContext is the function, still in edi. > - __ push(edi); > + if (function_in_register) { > + // Argument to NewContext is the function, still in edi. > + __ push(edi); > + } else { > + // Argument to NewContext is the function, no longer in edi. > + __ push(Operand(ebp, JavaScriptFrameConstants::kFunctionOffset)); > + } > __ CallRuntime(Runtime::kNewContext, 1); > // Context is returned in both eax and esi. It replaces the context > // passed to us. It's saved in the stack and kept live in esi. > @@ -425,9 +456,8 @@ > __ nop(); > > DropAndMove(expr->context(), eax); > - } else { > + } else if (rewrite->AsSlot() != NULL) { > Slot* slot = rewrite->AsSlot(); > - ASSERT_NE(NULL, slot); > switch (slot->type()) { > case Slot::LOCAL: > case Slot::PARAMETER: { > @@ -468,6 +498,13 @@ > UNREACHABLE(); > break; > } > + } else { > + // The parameter variable has been rewritten into an explict access to > + // the arguments object. > + Property* property = rewrite->AsProperty(); > + ASSERT_NOT_NULL(property); > + ASSERT_EQ(expr->context(), property->context()); > + Visit(property); > } > } > > ======================================= > --- /branches/bleeding_edge/src/x64/fast-codegen-x64.cc Wed Nov 11 01:50:06 > 2009 > +++ /branches/bleeding_edge/src/x64/fast-codegen-x64.cc Thu Nov 12 03:38:01 > 2009 > @@ -62,16 +62,54 @@ > > { Comment cmnt(masm_, "[ Allocate locals"); > int locals_count = fun->scope()->num_stack_slots(); > - for (int i = 0; i < locals_count; i++) { > - __ PushRoot(Heap::kUndefinedValueRootIndex); > + if (locals_count <= 1) { > + if (locals_count > 0) { > + __ PushRoot(Heap::kUndefinedValueRootIndex); > + } > + } else { > + __ LoadRoot(rdx, Heap::kUndefinedValueRootIndex); > + for (int i = 0; i < locals_count; i++) { > + __ push(rdx); > + } > } > } > + > + bool function_in_register = true; > + > + Variable* arguments = fun->scope()->arguments()->AsVariable(); > + if (arguments != NULL) { > + // Function uses arguments object. > + Comment cmnt(masm_, "[ Allocate arguments object"); > + __ push(rdi); > + // The receiver is just before the parameters on the caller's stack. > + __ lea(rdx, Operand(rbp, StandardFrameConstants::kCallerSPOffset + > + fun->num_parameters() * kPointerSize)); > + __ push(rdx); > + __ Push(Smi::FromInt(fun->num_parameters())); > + // Arguments to ArgumentsAccessStub: > + // function, receiver address, parameter count. > + // The stub will rewrite receiver and parameter count if the previous > + // stack frame was an arguments adapter frame. > + ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT); > + __ CallStub(&stub); > + // Store new arguments object in both "arguments" and ".arguments" > slots. > + __ movq(Operand(rbp, SlotOffset(arguments->slot())), rax); > + Slot* dot_arguments_slot = > + fun->scope()->arguments_shadow()->AsVariable()->slot(); > + __ movq(Operand(rbp, SlotOffset(dot_arguments_slot)), rax); > + function_in_register = false; > + } > > // Possibly allocate a local context. > if (fun->scope()->num_heap_slots() > 0) { > Comment cmnt(masm_, "[ Allocate local context"); > - // Argument to NewContext is the function, still in rdi. > - __ push(rdi); > + if (function_in_register) { > + // Argument to NewContext is the function, still in rdi. > + __ push(rdi); > + } else { > + // Argument to NewContext is the function, no longer in rdi. > + __ push(Operand(rbp, JavaScriptFrameConstants::kFunctionOffset)); > + } > __ CallRuntime(Runtime::kNewContext, 1); > // Context is returned in both rax and rsi. It replaces the context > // passed to us. It's saved in the stack and kept live in rsi. > @@ -432,9 +470,8 @@ > __ nop(); > > DropAndMove(expr->context(), rax); > - } else { > + } else if (rewrite->AsSlot() != NULL) { > Slot* slot = rewrite->AsSlot(); > - ASSERT_NE(NULL, slot); > switch (slot->type()) { > case Slot::LOCAL: > case Slot::PARAMETER: { > @@ -475,6 +512,13 @@ > UNREACHABLE(); > break; > } > + } else { > + // The parameter variable has been rewritten into an explict access to > + // the arguments object. > + Property* property = rewrite->AsProperty(); > + ASSERT_NOT_NULL(property); > + ASSERT_EQ(expr->context(), property->context()); > + Visit(property); > } > } > > > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
