My only comments are that I'd like to get rid of the break in the loop in ScanForFreeRegister (can't you just return?) and that returning kNumRegisters as an indication that no free registers exist seems plain weird.
Cheers, Kasper On Tue, Mar 17, 2009 at 12:36 PM, <[email protected]> wrote: > Reviewers: Kasper Lund, Kevin Millikin, > > Message: > A tiny code review for you. > The inner loop could be made > > while (ref_counts_[++i]); > > with a sentinel, but I think it is fast enough as is. > > Description: > Speed up the inner loop of free register allocation. > > Please review this at http://codereview.chromium.org/42296 > > SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ > > Affected files: > M src/register-allocator.h > M src/register-allocator.cc > > > Index: src/register-allocator.h > =================================================================== > --- src/register-allocator.h (revision 1525) > +++ src/register-allocator.h (working copy) > @@ -149,6 +149,7 @@ > // Record that a register will no longer be used by decrementing its > // reference count. > void Unuse(Register reg) { > + ASSERT(!reg.is(no_reg)); > ASSERT(is_used(reg.code())); > if (is_used(reg.code())) { > ref_counts_[reg.code()]--; > @@ -161,6 +162,15 @@ > private: > int ref_counts_[kNumRegisters]; > > + // Very fast inlined loop to find a free register. > + inline int ScanForFreeRegister() { > + int i = 0; > + for (; i < kNumRegisters ; ++i ) { > + if (ref_counts_[i] == 0) break; > + } > + return i; > + } > + > friend class RegisterAllocator; > }; > > Index: src/register-allocator.cc > =================================================================== > --- src/register-allocator.cc (revision 1525) > +++ src/register-allocator.cc (working copy) > @@ -83,11 +83,10 @@ > > Result RegisterAllocator::AllocateWithoutSpilling() { > // Return the first free register, if any. > - for (int i = 0; i < kNumRegisters; i++) { > - if (!is_used(i)) { > - Register free_reg = { i }; > - return Result(free_reg, cgen_); > - } > + int free_reg = registers_.ScanForFreeRegister(); > + if (free_reg < kNumRegisters) { > + Register free_result = { free_reg }; > + return Result(free_result, cgen_); > } > return Result(cgen_); > } > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
