Kevin, Do you have any explanation why the Ia32 code is so much more convoluted setting up the arguments object when compared to ARM?
Thanks, -Ivan On Thu, Oct 2, 2008 at 05:43, Kevin Millikin <[EMAIL PROTECTED]> wrote: > It does now :) > > On Thu, Oct 2, 2008 at 2:37 PM, Kasper Lund <[EMAIL PROTECTED]> wrote: >> >> LGTM. Does it lint? >> >> On Thu, Oct 2, 2008 at 2:16 PM, <[EMAIL PROTECTED]> wrote: >> > Reviewers: Kasper Lund, >> > >> > Description: >> > In the code generator, avoid loading the arguments object to the >> > expression stack when it is already there. Also, cleanup up the >> > (two!) extra copies of the arguments object left on the stack. >> > >> > Please review this at http://codereview.chromium.org/5667 >> > >> > Affected files: >> > M src/codegen-arm.cc >> > M src/codegen-ia32.cc >> > >> > >> > Index: src/codegen-arm.cc >> > =================================================================== >> > --- src/codegen-arm.cc (revision 409) >> > +++ src/codegen-arm.cc (working copy) >> > @@ -576,22 +576,18 @@ >> > if (scope->arguments() != NULL) { >> > ASSERT(scope->arguments_shadow() != NULL); >> > Comment cmnt(masm_, "[ allocate arguments object"); >> > - { >> > - Reference target(this, scope->arguments()); >> > - __ ldr(r0, FunctionOperand()); >> > - __ push(r0); >> > - __ CallRuntime(Runtime::kNewArguments, 1); >> > - __ push(r0); >> > - SetValue(&target); >> > + { Reference shadow_ref(this, scope->arguments_shadow()); >> > + { Reference arguments_ref(this, scope->arguments()); >> > + >> > + __ ldr(r0, FunctionOperand()); >> > + __ push(r0); >> > + __ CallRuntime(Runtime::kNewArguments, 1); >> > + __ push(r0); >> > + SetValue(&arguments_ref); >> > + } >> > + SetValue(&shadow_ref); >> > } >> > - // The value of arguments must also be stored in .arguments. >> > - // TODO(1241813): This code can probably be improved by fusing it >> > with >> > - // the code that stores the arguments object above. >> > - { >> > - Reference target(this, scope->arguments_shadow()); >> > - Load(scope->arguments()); >> > - SetValue(&target); >> > - } >> > + __ pop(r0); // Value is no longer needed. >> > } >> > >> > // Generate code to 'execute' declarations and initialize >> > Index: src/codegen-ia32.cc >> > =================================================================== >> > --- src/codegen-ia32.cc (revision 409) >> > +++ src/codegen-ia32.cc (working copy) >> > @@ -661,21 +661,27 @@ >> > ASSERT(scope->arguments() != NULL); >> > ASSERT(scope->arguments_shadow() != NULL); >> > Comment cmnt(masm_, "[ store arguments object"); >> > - { >> > - Reference target(this, scope->arguments()); >> > - if (!arguments_object_saved) { >> > - __ push(Operand(ecx)); >> > + { Reference shadow_ref(this, scope->arguments_shadow()); >> > + { Reference arguments_ref(this, scope->arguments()); >> > + // If the newly-allocated arguments object is already on the >> > + // stack, we make use of the property that references >> > representing >> > + // variables take up no space on the expression stack (ie, it >> > + // doesn't matter that the stored value is actually below the >> > + // reference). >> > + ASSERT(arguments_ref.size() == 0); >> > + ASSERT(shadow_ref.size() == 0); >> > + >> > + // If the newly-allocated argument object is not already on >> > the >> > + // stack, we rely on the property that loading a >> > + // (zero-sized) reference will not clobber the ecx register. >> > + if (!arguments_object_saved) { >> > + __ push(ecx); >> > + } >> > + SetValue(&arguments_ref); >> > } >> > - SetValue(&target); >> > + SetValue(&shadow_ref); >> > } >> > - // The value of arguments must also be stored in .arguments. >> > - // TODO(1241813): This code can probably be improved by fusing it >> > with >> > - // the code that stores the arguments object above. >> > - { >> > - Reference target(this, scope->arguments_shadow()); >> > - Load(scope->arguments()); >> > - SetValue(&target); >> > - } >> > + __ pop(eax); // Value is no longer needed. >> > } >> > >> > // Generate code to 'execute' declarations and initialize >> > >> > >> > > > > > -- > Google Denmark ApS > CVR nr. 28 86 69 84 > c/o Philip & Partners, 7 Vognmagergade, P.O. Box 2227, DK-1018 Copenhagen K, > Denmark > > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
