<div dir="ltr">hoist-early sink-later may also introduces hoisted instructions that is not sinkable later.<div><br></div><div>e.g.</div><div>orig code:</div><div>for() {</div><div> if (cond) {</div><div> cold_load;</div><div> cold_code;</div><div> }</div><div>}</div><div>store;</div><div><br></div><div>after hoisting:</div><div><div>cold_load;<br class="">for() {</div><div> if (cond) {</div><div> cold_code;<br></div><div> }</div><div>}</div><div>store;</div></div><div><br></div><div>after other code motion:</div><div><div>cold_load;</div><div>store;<br class="">for() {</div><div> if (cond) {</div><div> cold_code;<br></div><div> }</div><div>}</div></div><div><br></div><div>then later in cgp, when you want to sink cold_load to its uses, the store may prevent the sinking due to aliasing.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 11, 2016 at 10:58 AM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, May 10, 2016 at 3:14 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>----- Original Message -----<br>
> From: "Dehao Chen" <<a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a>><br>
> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br>
> Cc: "Xinliang David Li" <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>>, <a href="mailto:reviews%2BD19950%2Bpublic%2B38ba22078c2035b8@reviews.llvm.org" target="_blank">reviews+D19950+public+38ba22078c2035b8@reviews.llvm.org</a>, "David<br>
> Majnemer" <<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>>, "Junbum Lim" <<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>>, <a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</a>, "llvm-commits"<br>
> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>, "amara emerson" <<a href="mailto:amara.emerson@arm.com" target="_blank">amara.emerson@arm.com</a>><br>
</span><span>> Sent: Tuesday, May 10, 2016 4:10:49 PM<br>
> Subject: Re: [PATCH] D19950: Use frequency info to guide Loop Invariant Code Motion.<br>
><br>
><br>
</span><span>> Thanks for the comment. I spent quite a while to think, but still<br>
> cannot think of an optimization that could be unblocked by<br>
> speculatively hoisting an loop invariant from an unlikely executed<br>
> path. Can you give some hint (or an example) on what type of<br>
> optimization can benefit from this case?<br>
<br>
</span>I'm specifically thinking about this case (although I suspect there are others):<br>
<br>
for (...) {<br>
if (...) {<br>
hoistable<br>
cold_stuff<br>
}<br>
}<br>
<br>
for (...) {<br>
if (...) {<br>
hoistable<br>
hot_stuff<br>
}<br>
}<br>
<br>
I expect that 'hoistable' will be hoisted by LICM out of both loops, and then CSE'd by GVN. </blockquote><div><br></div><div><br></div></span><div>I think this case can be/should be handled by more general profile driver speculative PRE. The above case may not be profitable even after GVN CSEed two expressions. On the other hand, </div><div><br></div><div>... = a * b;</div><div><br></div><div>for (...) {</div><div> if (cold) {</div><div> .... = a * b;</div><div> }</div><div> }</div><div><br></div><div>It will be good to hoist and CSE. Though in this case, we do not need LICM to enable this CSE. Another case:</div><div><br></div><div>if (....) {</div><div> ... = a*b;</div><div> }</div><div><br></div><div>for (....) {</div><div> if (cold) {</div><div> ... = a * b;</div><div> }</div><div> }</div><div><br></div><div>Depending on the profile, it might be profitable to do:</div><div><br></div><div>t = a * b;</div><div> if (...) {</div><div> .. = t;</div><div> }</div><div>for (...) {</div><div> if (cold) {</div><div> .. = t ;</div><div> }</div><div>}</div><div><br></div><div>Again, LICM won't be necessary to enable this.</div><span class=""><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">One might also imagine cases where the two hoistable sections are SLP vectorized.<br></blockquote><div><br></div><div><br></div></span><div>Will that make it harder to undo the damage later ?</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Failing to host the code might also prevent loop unswitching (by failing to reduce the size of the loop body below the threshold size).<br></blockquote><div><br></div><div><br></div></span><div>There are always existing cleanups that can only happen after loop-unswitching happens. IMO, loop unswiitching, like inliner should also look at the code state if the transformation happens. </div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Another potential issue is that the hoistable code might be cold, and relatively cheap to hoist, but expensive to vectorize. As a result, failing to hoist the code might block otherwise-profitable vectorization. Which reminds me, we need to fix the vectorizer's if-conversion heuristic to use profiling information too ;)<br></blockquote><div><br></div></span><div>SLP vectorize? Any example like this? Can vectorizor be enhanced so that it can be done in absence of the hoisting?</div><div><br></div><div>thanks,</div><div><br></div><div>David</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks again,<br>
Hal<br>
<span><br>
><br>
> Thanks,<br>
> Dehao<br>
><br>
><br>
> On Tue, May 10, 2016 at 1:58 PM, Hal Finkel < <a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> ><br>
> wrote:<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
</span><span>> From: "Xinliang David Li" < <a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a> ><br>
> To: "Dehao Chen" < <a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a> ><br>
</span>> Cc: <a href="mailto:reviews%2BD19950%2Bpublic%2B38ba22078c2035b8@reviews.llvm.org" target="_blank">reviews+D19950+public+38ba22078c2035b8@reviews.llvm.org</a> , "David<br>
<span>> Majnemer" < <a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a> >, "Hal Finkel" <<br>
> <a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> >, "Junbum Lim" < <a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a> >,<br>
</span>> <a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</a> , "llvm-commits" <<br>
<span>> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a> >, "amara emerson" <<br>
> <a href="mailto:amara.emerson@arm.com" target="_blank">amara.emerson@arm.com</a> ><br>
> Sent: Tuesday, May 10, 2016 3:15:24 PM<br>
> Subject: Re: [PATCH] D19950: Use frequency info to guide Loop<br>
> Invariant Code Motion.<br>
><br>
><br>
><br>
><br>
><br>
><br>
> On Tue, May 10, 2016 at 1:03 PM, Dehao Chen < <a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a> ><br>
> wrote:<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> On Tue, May 10, 2016 at 11:48 AM, Xinliang David Li <<br>
> <a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a> > wrote:<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> On Tue, May 10, 2016 at 11:01 AM, Dehao Chen < <a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a> ><br>
> wrote:<br>
><br>
><br>
> danielcdh added a comment.<br>
><br>
> In <a href="http://reviews.llvm.org/D19950#425287" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19950#425287</a> , @hfinkel wrote:<br>
><br>
> > In <a href="http://reviews.llvm.org/D19950#425286" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19950#425286</a> , @hfinkel wrote:<br>
> ><br>
</span><div><div>> > > In <a href="http://reviews.llvm.org/D19950#425285" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19950#425285</a> , @davidxl wrote:<br>
> > ><br>
> > > > Static prediction has been conservative in estimating loop trip<br>
> > > > count -- it produces something like 30ish iterations. If the a<br>
> > > > very hot loop has a big if-then-else (or switch), it is very<br>
> > > > likely to mark many bbs' to be colder than the loop header.<br>
> > > > Turning on this for static prediction really depends on the<br>
> > > > false rate. It seems to be this can get wrong pretty easily<br>
> > > > for very hot loops (which is also the most important thing to<br>
> > > > optimize for).<br>
> > ><br>
> > ><br>
> > > This is a good point. There's no universal conservative choice<br>
> > > (assuming a small trip count is conservative in some cases, and<br>
> > > assuming a large trip count is conservative in other cases).<br>
> ><br>
> ><br>
> > Would it be better (and practical) if there were some way for the<br>
> > BFI client to specify which kind of 'conservative' is desired?<br>
> ><br>
> > Also, why are we doing this instead of sinking later (in CGP or<br>
> > similar)? LICM can expose optimization opportunities, plus<br>
> > represents a code pattern the user might input manually. Sinking<br>
> > later seems more robust.<br>
><br>
><br>
> I looked at CGP pass, looks like it's handling the sinking<br>
> case-by-case (e.g. there is separate routine to handle sinking of<br>
> load, gep, etc. I'm afraid this would miss opportunities.<br>
> Additionally, the file-level comment of CGP pass says "This works<br>
> around limitations in it's basic-block-at-a-time approach. It should<br>
> eventually be removed."<br>
> Yes, but it will be "removed" when the entire subsystem is replaced<br>
> by GlobalISel, and we'll certainly need to make GlobalISel<br>
> profiling-data aware, so I expect this is the right path forward<br>
> regardless. I agree, however, that we want a general sinking here<br>
> based on profiling data, not just the specific existing heuristics<br>
> for loads, GEPs, etc.<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> Perhaps you can do profile driven sinking CGP separately to handle<br>
> manually hoisted code situation mentioned by Hal.<br>
><br>
><br>
> Do you mean we still use frequency to decide whether to hoist code in<br>
> LICM, additionally use frequency info to check if we want to sink<br>
> instructions in CGP?<br>
><br>
><br>
><br>
><br>
> yes -- that is the suggestion. I'd prefer that we try to sink late<br>
> first, and only if there are use cases that we can't handle this<br>
> way, we consider throttling hoisting early. If we come across such<br>
> use cases, I'd like to understand them better. Hoisting can expose<br>
> other optimization opportunities, and you lose those opportunities<br>
> if you don't hoist in the first place.<br>
><br>
> -Hal<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> David<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> Dehao<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> David<br>
><br>
><br>
><br>
> I'm not quite clear why it helps to move code out of loop early and<br>
> later sink it inside. Could you give an example or some more<br>
> context?<br>
><br>
> Thanks,<br>
> Dehao<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D19950" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19950</a><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> --<br>
><br>
> Hal Finkel<br>
> Assistant Computational Scientist<br>
> Leadership Computing Facility<br>
> Argonne National Laboratory<br>
><br>
><br>
<br>
--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>