LGTM On Mon, Jan 26, 2009 at 10:08 AM, <[email protected]> wrote: > Reviewers: Mads Ager, > > Message: > This optimization in change 18551 turns out to be a performance > regression. The picture is a little mixed, but many bitops tests suffer > significantly. > > Description: > Revert change to Smi check that was a performance regression. > > Please review this at http://codereview.chromium.org/18582 > > SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ > > Affected files: > M src/codegen-ia32.cc > > > Index: src/codegen-ia32.cc > =================================================================== > --- src/codegen-ia32.cc (revision 1142) > +++ src/codegen-ia32.cc (working copy) > @@ -1127,14 +1127,14 @@ > __ j(not_zero, deferred->enter(), not_taken); > __ sar(ebx, kSmiTagSize); > __ shl(ebx, shift_value); > - // Convert the int to a Smi, and check that it is in > - // the range of valid Smis. > - ASSERT(kSmiTagSize == times_2); // Adjust code if not true. > - ASSERT(kSmiTag == 0); // Adjust code if not true. > - __ add(ebx, Operand(ebx)); > - __ j(overflow, deferred->enter(), not_taken); > - __ mov(eax, Operand(ebx)); > - > + // This is the Smi check for the shifted result. > + // After signed subtraction of 0xc0000000, the valid > + // Smis are positive. > + __ cmp(ebx, 0xc0000000); > + __ j(sign, deferred->enter(), not_taken); > + // tag result and store it in TOS (eax) > + ASSERT(kSmiTagSize == times_2); // adjust code if not the case > + __ lea(eax, Operand(ebx, ebx, times_1, kSmiTag)); > __ bind(deferred->exit()); > frame_->Push(eax); > } > > >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
