I removed the no_multiply label from ia32 when I landed the x64 patch. Yang
On Tue, Sep 18, 2012 at 8:36 AM, <[email protected]> wrote: > On 2012/09/17 07:40:14, Yang wrote: > >> On 2012/09/17 03:10:48, xqian wrote: >> > Hi， Yang >> > >> > Thanks for your review. I refined the code according to your comments. >> > Please > >> > check the patch set 3. >> > >> > Thanks, >> > -Xi >> > On 2012/09/14 15:38:16, Yang wrote: >> > > Thanks for this patch! I'll land this patch once the following two >> > comments > >> > are >> > > addressed. >> > > >> > > >> https://codereview.chromium.**org/10916311/diff/3001/src/** >> ia32/code-stubs-ia32.cc<https://codereview.chromium.org/10916311/diff/3001/src/ia32/code-stubs-ia32.cc> >> > > File src/ia32/code-stubs-ia32.cc (right): >> > > >> > > >> > >> > > https://codereview.chromium.**org/10916311/diff/3001/src/** > ia32/code-stubs-ia32.cc#**newcode3224<https://codereview.chromium.org/10916311/diff/3001/src/ia32/code-stubs-ia32.cc#newcode3224> > >> > > src/ia32/code-stubs-ia32.cc:**3224: __ j(above, &while_true, >> Label::kNear); >> > > Please add a comment here that the above flag is set if the bit >> shifted >> > out > >> > was >> > > not set. This would make understanding this easier. >> > > >> > > >> > >> > > https://codereview.chromium.**org/10916311/diff/3001/src/** > ia32/code-stubs-ia32.cc#**newcode3225<https://codereview.chromium.org/10916311/diff/3001/src/ia32/code-stubs-ia32.cc#newcode3225> > >> > > src/ia32/code-stubs-ia32.cc:**3225: __ mulsd(double_result, >> double_scratch); >> > > Since double_result is 1 here, wouldn't movq(double_result, >> > double_scratch) > >> do >> > > the same? That would actually be more readable imo. Maybe also faster? >> > > LGTM. Landing. >> > > Does it make sense to port this to x64? >> > > I tried the patch on x64 and it got similar performance improvement. The > patch > is created at > https://codereview.chromium.**org/10939013/<https://codereview.chromium.org/10939013/>. > Would you please review > it? > > Thanks, > -Xi > > https://codereview.chromium.**org/10916311/<https://codereview.chromium.org/10916311/> > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
