<div dir="ltr">Hi,<br><br>So am I hearing correctly that this patch seems OK?<div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote">On Tue Feb 10 2015 at 5:21:15 PM Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">----- Original Message -----<br>
> From: "Andrea Di Biagio" <<a href="mailto:andrea.dibiagio@gmail.com" target="_blank">andrea.dibiagio@gmail.com</a>><br>
> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br>
> Cc: "James Molloy" <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>>, <a href="mailto:reviews%2BD7506%2Bpublic%2Bd608f0951474be02@reviews.llvm.org" target="_blank">reviews+D7506+public+<u></u>d608f0951474be02@reviews.llvm.<u></u>org</a>,<br>
> "<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a> for LLVM" <<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>><br>
> Sent: Tuesday, February 10, 2015 11:15:39 AM<br>
> Subject: Re: [PATCH] [SimplifyCFG] Swap to using TargetTransformInfo for cost analysis.<br>
><br>
> On Tue, Feb 10, 2015 at 5:13 PM, Andrea Di Biagio<br>
> <<a href="mailto:andrea.dibiagio@gmail.com" target="_blank">andrea.dibiagio@gmail.com</a>> wrote:<br>
> > On Tue, Feb 10, 2015 at 5:12 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br>
> > wrote:<br>
> >> ----- Original Message -----<br>
> >>> From: "Andrea Di Biagio" <<a href="mailto:andrea.dibiagio@gmail.com" target="_blank">andrea.dibiagio@gmail.com</a>><br>
> >>> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br>
> >>> Cc: "James Molloy" <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>>,<br>
> >>> <a href="mailto:reviews%2BD7506%2Bpublic%2Bd608f0951474be02@reviews.llvm.org" target="_blank">reviews+D7506+public+<u></u>d608f0951474be02@reviews.llvm.<u></u>org</a>,<br>
> >>> "<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a> for LLVM" <<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>><br>
> >>> Sent: Tuesday, February 10, 2015 10:59:15 AM<br>
> >>> Subject: Re: [PATCH] [SimplifyCFG] Swap to using<br>
> >>> TargetTransformInfo for cost analysis.<br>
> >>><br>
> >>> Hi James, Hal<br>
> >>><br>
> >>> thanks for the feedback.<br>
> >>><br>
> >>> In that case, I'll see if I can come up with a fix to improve the<br>
> >>> cost<br>
> >>> of intrinsic calls for x86.<br>
> >>> On x86, the cost of cttz/ctlz would be either TCC_Basic or<br>
> >>> TCC_Expensive based on whether we have certain features.<br>
> >>><br>
> >>> @Hal: Does it mean that this patch would make the the code I<br>
> >>> added in<br>
> >>> CodeGenPrepare to speculate calls to cttz/ctlz redundant? I see<br>
> >>> that<br>
> >>> with this patch, some of the test cases in<br>
> >>> CodeGen/X86/lzcnt-tzcnt.ll<br>
> >>> are now optimized by SimplifyCFG. The only unoptimized cases are<br>
> >>> those<br>
> >>> where the 'then' block contains extra instructions. I guess D7507<br>
> >>> will<br>
> >>> take care of it.<br>
> >>><br>
> >>> Please correct me if I am wrong.<br>
> >><br>
> >> I think you're right; it should get the simple speculation case.<br>
> >> Will it also get the cases where we clear the 'undef on zero'<br>
> >> flag?<br>
> >><br>
> >>  -Hal<br>
> ><br>
> > Yes, it would work because of this revision:<br>
><br>
> Forget my last email...<br>
><br>
> It will get the 'undef on zero' flag because of revision 227197.<br>
> <a href="http://llvm.org/viewvc/llvm-project?view=revision&revision=227197" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?view=revision&<u></u>revision=227197</a><br>
<br>
Ah, okay, Now I understand. Thanks!<br>
<br>
So, yes, it might make some of that logic redundant. However, we can also reuse the TLI interfaces by making TTI's default implementation use those interfaces for providing user costs for those intrinsics.<br>
<br>
 -Hal<br>
<br>
><br>
> ><br>
> >><br>
> >>><br>
> >>> Thanks!<br>
> >>> Andrea<br>
> >>><br>
> >>> On Tue, Feb 10, 2015 at 4:27 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br>
> >>> wrote:<br>
> >>> > ----- Original Message -----<br>
> >>> >> From: "James Molloy" <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>><br>
> >>> >> To: <a href="mailto:reviews%2BD7506%2Bpublic%2Bd608f0951474be02@reviews.llvm.org" target="_blank">reviews+D7506+public+<u></u>d608f0951474be02@reviews.llvm.<u></u>org</a>,<br>
> >>> >> "james<br>
> >>> >> molloy" <<a href="mailto:james.molloy@arm.com" target="_blank">james.molloy@arm.com</a>>, <a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>,<br>
> >>> >> "t p northover" <<a href="mailto:t.p.northover@gmail.com" target="_blank">t.p.northover@gmail.com</a>>, "Andrea DiBiagio"<br>
> >>> >> <<a href="mailto:Andrea_DiBiagio@sn.scee.net" target="_blank">Andrea_DiBiagio@sn.scee.net</a>><br>
> >>> >> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> >>> >> Sent: Tuesday, February 10, 2015 10:22:07 AM<br>
> >>> >> Subject: Re: [PATCH] [SimplifyCFG] Swap to using<br>
> >>> >> TargetTransformInfo for cost analysis.<br>
> >>> >><br>
> >>> >><br>
> >>> >> Hi Andrea,<br>
> >>> >><br>
> >>> >> Performance on AArch64 showed no major swings. However, it<br>
> >>> >> sounds<br>
> >>> >> to<br>
> >>> >> me like your TargetTransformInfo isn't behaving correctly.<br>
> >>> >> That's<br>
> >>> >> TTI's entire point, and the x86 one should surely return<br>
> >>> >> "TCC_Expensive" for such intrinsic calls.<br>
> >>> >><br>
> >>> >> I see that the default TTI does return TCC_Basic for almost<br>
> >>> >> all<br>
> >>> >> intrinsics, and that is arguable behaviour. But there's no<br>
> >>> >> reason<br>
> >>> >> you shouldn't override this. In fact, it seems the right thing<br>
> >>> >> to<br>
> >>> >> do.<br>
> >>> >><br>
> >>> ><br>
> >>> > I agree, x86 should override these. I'll add that we're going<br>
> >>> > to<br>
> >>> > run into this more because we're starting to use the user-cost<br>
> >>> > interface for more things. The target's user-cost<br>
> >>> > implementations<br>
> >>> > are not as well tuned as the cost interfaces used by the<br>
> >>> > vectorizer.<br>
> >>> ><br>
> >>> >  -Hal<br>
> >>> ><br>
> >>> >><br>
> >>> >> Cheers,<br>
> >>> >><br>
> >>> >><br>
> >>> >> James<br>
> >>> >><br>
> >>> >> On Tue Feb 10 2015 at 2:53:52 PM Andrea Di Biagio <<br>
> >>> >> <a href="mailto:Andrea_DiBiagio@sn.scee.net" target="_blank">Andrea_DiBiagio@sn.scee.net</a> > wrote:<br>
> >>> >><br>
> >>> >><br>
> >>> >> REPOSITORY<br>
> >>> >> rL LLVM<br>
> >>> >><br>
> >>> >> <a href="http://reviews.llvm.org/D7506" target="_blank">http://reviews.llvm.org/D7506</a><br>
> >>> >><br>
> >>> >> EMAIL PREFERENCES<br>
> >>> >> <a href="http://reviews.llvm.org/" target="_blank">http://reviews.llvm.org/</a> settings/panel/ emailpreferences/<br>
> >>> >><br>
> >>> >><br>
> >>> >><br>
> >>> >> ______________________________ _________________<br>
> >>> >> llvm-commits mailing list<br>
> >>> >> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> >>> >> <a href="http://lists.cs.uiuc.edu/" target="_blank">http://lists.cs.uiuc.edu/</a> mailman/listinfo/llvm-commits<br>
> >>> >><br>
> >>> ><br>
> >>> > --<br>
> >>> > Hal Finkel<br>
> >>> > Assistant Computational Scientist<br>
> >>> > Leadership Computing Facility<br>
> >>> > Argonne National Laboratory<br>
> >>> > ______________________________<u></u>_________________<br>
> >>> > llvm-commits mailing list<br>
> >>> > <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> >>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
> >>><br>
> >><br>
> >> --<br>
> >> Hal Finkel<br>
> >> Assistant Computational Scientist<br>
> >> Leadership Computing Facility<br>
> >> Argonne National Laboratory<br>
><br>
<br>
--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</blockquote></div>