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 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
