LGTM, Lars On Fri, Oct 24, 2008 at 2:38 PM, <[EMAIL PROTECTED]> wrote: > Lars, > > I'd like you to do a code review. To review this change, run > > gvn review --project https://v8.googlecode.com/svn [EMAIL PROTECTED]/[EMAIL > PROTECTED] > > Alternatively, to review the latest snapshot of this change > branch, run > > gvn --project https://v8.googlecode.com/svn review [EMAIL > PROTECTED]/prepare-120 > > to review the following change: > > [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-10-24 13:37:53 > +-100 (Fri, 24 Oct 2008) > > Description: > > Get ready for fixing issue 120: Pin point the places > where the receiver needs to be patched with the proxy > and get ready of unnecessary function patching on ARM. > > > > Affected Paths: > M //branches/bleeding_edge/src/codegen-arm.cc > M //branches/bleeding_edge/src/codegen-ia32.cc > M //branches/bleeding_edge/src/ic-arm.cc > M //branches/bleeding_edge/src/ic-ia32.cc > M //branches/bleeding_edge/src/stub-cache-arm.cc > M //branches/bleeding_edge/src/stub-cache-ia32.cc > > > This is a semiautomated message from "gvn mail". See > <http://code.google.com/p/gvn/> to learn more. > > Index: src/codegen-arm.cc > =================================================================== > --- src/codegen-arm.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/codegen-arm.cc (^/changes/[EMAIL > PROTECTED]/prepare-120/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -2240,8 +2240,8 @@ void CodeGenerator::VisitCall(Call* node) { > __ mov(r0, Operand(var->name())); > __ push(r0); > > - // TODO(120): use JSGlobalObject for function lookup and inline cache, > - // and use global proxy as 'this' for invocation. > + // TODO(120): Use global object for function lookup and inline > + // cache, and use global proxy as 'this' for invocation. > LoadGlobalReceiver(r0); > > // Load the arguments. > @@ -2329,11 +2329,10 @@ void CodeGenerator::VisitCall(Call* node) { > > // Load the function. > Load(function); > - // Pass the global object as the receiver. > > - // TODO(120): use JSGlobalObject for function lookup and inline cache, > - // and use global proxy as 'this' for invocation. > + // Pass the global proxy as the receiver. > LoadGlobalReceiver(r0); > + > // Call the function. > CallWithArguments(args, node->position()); > __ push(r0); > @@ -2351,9 +2350,10 @@ void CodeGenerator::VisitCallNew(CallNew* node) { > // evaluated. > > // Compute function to call and use the global object as the > - // receiver. > + // receiver. There is no need to use the global proxy here because > + // it will always be replaced with a newly allocated object. > Load(node->expression()); > - LoadGlobalReceiver(r0); > + LoadGlobal(); > > // Push the arguments ("left-to-right") on the stack. > ZoneList<Expression*>* args = node->arguments(); > Index: src/codegen-ia32.cc > =================================================================== > --- src/codegen-ia32.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/codegen-ia32.cc (^/changes/[EMAIL > PROTECTED]/prepare-120/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -2660,8 +2660,8 @@ void CodeGenerator::VisitCall(Call* node) { > // Push the name of the function and the receiver onto the stack. > frame_->Push(Immediate(var->name())); > > - // TODO(120): use JSGlobalObject for function lookup and inline cache, > - // and use global proxy as 'this' for invocation. > + // TODO(120): Use global object for function lookup and inline > + // cache, and use global proxy as 'this' for invocation. > LoadGlobalReceiver(eax); > > // Load the arguments. > @@ -2747,10 +2747,7 @@ void CodeGenerator::VisitCall(Call* node) { > // Load the function. > Load(function); > > - // Pass the global object as the receiver. > - > - // TODO(120): use JSGlobalObject for function lookup and inline cache, > - // and use global proxy as 'this' for invocation. > + // Pass the global proxy as the receiver. > LoadGlobalReceiver(eax); > > // Call the function. > @@ -2769,9 +2766,10 @@ void CodeGenerator::VisitCallNew(CallNew* node) { > // evaluated. > > // Compute function to call and use the global object as the > - // receiver. > + // receiver. There is no need to use the global proxy here because > + // it will always be replaced with a newly allocated object. > Load(node->expression()); > - LoadGlobalReceiver(eax); > + LoadGlobal(); > > // Push the arguments ("left-to-right") on the stack. > ZoneList<Expression*>* args = node->arguments(); > Index: src/ic-arm.cc > =================================================================== > --- src/ic-arm.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/ic-arm.cc (^/changes/[EMAIL > PROTECTED]/prepare-120/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -365,7 +365,7 @@ void CallIC::GenerateNormal(MacroAssembler* masm, > // If this assert fails, we have to check upper bound too. > ASSERT(LAST_TYPE == JS_FUNCTION_TYPE); > > - // Check for access to global object (unlikely). > + // Check for access to global proxy. > __ cmp(r0, Operand(JS_GLOBAL_PROXY_TYPE)); > __ b(eq, &global); > > @@ -383,8 +383,8 @@ void CallIC::GenerateNormal(MacroAssembler* masm, > __ cmp(r0, Operand(JS_FUNCTION_TYPE)); > __ b(ne, &miss); > > - // Patch the function on the stack; 1 ~ receiver. > - __ str(r1, MemOperand(sp, (argc + 1) * kPointerSize)); > + // TODO(120): Check for access to global object. Needs patching of > + // receiver but no security check. > > // Invoke the function. > ParameterCount actual(argc); > @@ -425,13 +425,12 @@ void CallIC::Generate(MacroAssembler* masm, > CEntryStub stub; > __ CallStub(&stub); > > - // Move result to r1. > + // Move result to r1 and leave the internal frame. > __ mov(r1, Operand(r0)); > - > __ LeaveInternalFrame(); > > - // Patch the function on the stack; 1 ~ receiver. > - __ str(r1, MemOperand(sp, (argc + 1) * kPointerSize)); > + // TODO(120): Check for access to to global object. Needs patching > + // of receiver but no security check. > > // Invoke the function. > ParameterCount actual(argc); > @@ -485,7 +484,6 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) > __ cmp(r1, Operand(JS_GLOBAL_PROXY_TYPE)); > __ b(eq, &global); > > - > __ bind(&probe); > GenerateDictionaryLoad(masm, &done, &miss, r1, r0); > __ Ret(); > Index: src/ic-ia32.cc > =================================================================== > --- src/ic-ia32.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/ic-ia32.cc (^/changes/[EMAIL > PROTECTED]/prepare-120/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -479,7 +479,7 @@ void CallIC::GenerateNormal(MacroAssembler* masm, > // If this assert fails, we have to check upper bound too. > ASSERT(LAST_TYPE == JS_FUNCTION_TYPE); > > - // Check for access to global object. > + // Check for access to global proxy. > __ cmp(eax, JS_GLOBAL_PROXY_TYPE); > __ j(equal, &global, not_taken); > > @@ -498,11 +498,14 @@ void CallIC::GenerateNormal(MacroAssembler* masm, > __ cmp(edx, JS_FUNCTION_TYPE); > __ j(not_equal, &miss, not_taken); > > + // TODO(120): Check for access to global object. Needs patching of > + // receiver but no security check. > + > // Invoke the function. > ParameterCount actual(argc); > __ InvokeFunction(edi, actual, JUMP_FUNCTION); > > - // Global object access: Check access rights. > + // Global object proxy access: Check access rights. > __ bind(&global); > __ CheckAccessGlobalProxy(edx, eax, &miss); > __ jmp(&probe); > @@ -542,6 +545,9 @@ void CallIC::Generate(MacroAssembler* masm, > __ mov(Operand(edi), eax); > __ LeaveInternalFrame(); > > + // TODO(120): Check for access to to global object. Needs patching > + // of receiver but no security check. > + > // Invoke the function. > ParameterCount actual(argc); > __ InvokeFunction(edi, actual, JUMP_FUNCTION); > Index: src/stub-cache-arm.cc > =================================================================== > --- src/stub-cache-arm.cc (^/branches/bleeding_edge/src/[EMAIL > PROTECTED]) > +++ src/stub-cache-arm.cc (^/changes/[EMAIL > PROTECTED]/prepare-120/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -240,8 +240,9 @@ Object* CallStubCompiler::CompileCallField(Object* > __ cmp(r2, Operand(JS_FUNCTION_TYPE)); > __ b(ne, &miss); > > - // Patch the function on the stack; 1 ~ receiver. > - __ str(r1, MemOperand(sp, (argc + 1) * kPointerSize)); > + if (object->IsGlobalObject()) { > + // TODO(120): Patch receiver with the global proxy. > + } > > // Invoke the function. > __ InvokeFunction(r1, arguments(), JUMP_FUNCTION); > @@ -352,8 +353,9 @@ Object* CallStubCompiler::CompileCallConstant(Obje > __ mov(r1, Operand(Handle<JSFunction>(function))); > __ ldr(cp, FieldMemOperand(r1, JSFunction::kContextOffset)); > > - // Patch the function on the stack; 1 ~ receiver. > - __ str(r1, MemOperand(sp, (argc + 1) * kPointerSize)); > + if (object->IsGlobalObject()) { > + // TODO(120): Patch receiver with the global proxy. > + } > > // Jump to the cached code (tail call). > Handle<Code> code(function->code()); > Index: src/stub-cache-ia32.cc > =================================================================== > --- src/stub-cache-ia32.cc (^/branches/bleeding_edge/src/[EMAIL > PROTECTED]) > +++ src/stub-cache-ia32.cc (^/changes/[EMAIL > PROTECTED]/prepare-120/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -478,6 +478,7 @@ Object* StubCompiler::CompileLazyCompile(Code::Fla > __ CallRuntime(Runtime::kLazyCompile, 1); > __ pop(edi); > > + // Tear down temporary frame. > __ LeaveInternalFrame(); > > // Do a tail-call of the compiled function. > @@ -519,6 +520,10 @@ Object* CallStubCompiler::CompileCallField(Object* > __ cmp(ebx, JS_FUNCTION_TYPE); > __ j(not_equal, &miss, not_taken); > > + if (object->IsGlobalObject()) { > + // TODO(120): Patch receiver with the global proxy. > + } > + > // Invoke the function. > __ InvokeFunction(edi, arguments(), JUMP_FUNCTION); > > @@ -627,6 +632,10 @@ Object* CallStubCompiler::CompileCallConstant(Obje > __ mov(Operand(edi), Immediate(Handle<JSFunction>(function))); > __ mov(esi, FieldOperand(edi, JSFunction::kContextOffset)); > > + if (object->IsGlobalObject()) { > + // TODO(120): Patch receiver with the global proxy. > + } > + > // Jump to the cached code (tail call). > Handle<Code> code(function->code()); > ParameterCount expected(function->shared()->formal_parameter_count()); > @@ -697,6 +706,10 @@ Object* CallStubCompiler::CompileCallInterceptor(O > __ cmp(ebx, JS_FUNCTION_TYPE); > __ j(not_equal, &miss, not_taken); > > + if (object->IsGlobalObject()) { > + // TODO(120): Patch receiver with the global proxy. > + } > + > // Invoke the function. > __ InvokeFunction(edi, arguments(), JUMP_FUNCTION); > > >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
