I know how it's different but I don't really know why it's different. On IA32 we allocate the arguments object right away if needed, then allocate and initialize the locals. There is some pretty ugly code (that will go away once we make smarter use of registers) to try to keep the arguments object in ecx unless it has to spill it for a runtime call.
On ARM we allocate the arguments object later, after context creation in the heap, so there is no gyration involved in trying to avoid spilling it. I'm not sure if either of them contains a bug. On Thu, Oct 2, 2008 at 2:47 PM, Ivan Posva <[EMAIL PROTECTED]> wrote: > 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 > > > > > > > > > -- 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 -~----------~----~----~----~------~----~------~--~---
