Thanks Yang! Xi: DBC means Drive By Comment :-) -- Vyacheslav Egorov
On Tue, Sep 18, 2012 at 10:59 AM, Yang Guo <[email protected]> wrote: > 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 >>> > > File src/ia32/code-stubs-ia32.cc (right): >>> > > >>> > > >>> > >> >> >> >> 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 >>> >>> > > 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/. Would you please >> review >> it? >> >> Thanks, >> -Xi >> >> https://codereview.chromium.org/10916311/ > > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
