LGTM. Maybe change 'skipped_write' to 'may_skip_write'? On Thu, Sep 11, 2008 at 4:58 PM, <[EMAIL PROTECTED]> wrote: > Reviewers: Kasper Lund, > > Description: > Fix performace regression due to missed peephole optimization > opportunity. > > Please review this at http://codereview.chromium.org/2002 > > Affected files: > M src/codegen-arm.cc > M src/codegen-ia32.cc > > > Index: src/codegen-arm.cc > =================================================================== > --- src/codegen-arm.cc (revision 282) > +++ src/codegen-arm.cc (working copy) > @@ -948,6 +948,7 @@ > ASSERT(var()->mode() != Variable::DYNAMIC); > > Label exit; > + bool skipped_write = false; > if (init_state == CONST_INIT) { > ASSERT(var()->mode() == Variable::CONST); > // Only the first const initialization must be executed (the slot > @@ -957,6 +958,7 @@ > masm->ldr(r2, ArmCodeGenerator::SlotOperand(masm, scope, this, r2)); > masm->cmp(r2, Operand(Factory::the_hole_value())); > masm->b(ne, &exit); > + skipped_write = true; > } > > // We must execute the store. > @@ -981,7 +983,9 @@ > masm->mov(r3, Operand(offset)); > masm->RecordWrite(r2, r3, r1); > } > - masm->bind(&exit); > + // If we did not jump over the assignment, we do not need to bind the > + // exit label. Doing so can defeat peephole optimization. > + if (skipped_write) masm->bind(&exit); > } > } > > Index: src/codegen-ia32.cc > =================================================================== > --- src/codegen-ia32.cc (revision 282) > +++ src/codegen-ia32.cc (working copy) > @@ -975,6 +975,7 @@ > ASSERT(var()->mode() != Variable::DYNAMIC); > > Label exit; > + bool skipped_write = false; > if (init_state == CONST_INIT) { > ASSERT(var()->mode() == Variable::CONST); > // Only the first const initialization must be executed (the slot > @@ -984,6 +985,7 @@ > masm->mov(eax, Ia32CodeGenerator::SlotOperand(masm, scope, this, > ecx)); > masm->cmp(eax, Factory::the_hole_value()); > masm->j(not_equal, &exit); > + skipped_write = true; > } > > // We must execute the store. > @@ -1003,7 +1005,9 @@ > int offset = FixedArray::kHeaderSize + index() * kPointerSize; > masm->RecordWrite(ecx, offset, eax, ebx); > } > - masm->bind(&exit); > + // If we did not jump over the assignment, we do not need to bind the > + // exit label. Doing so can defeat peephole optimization. > + if (skipped_write) masm->bind(&exit); > } > } > > > >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
