<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 12, 2016 at 6:56 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"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:#000000"><br><br><hr><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><span class=""><b>From: </b>"Xinliang David Li" <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>><br><b>To: </b>"Dehao Chen" <<a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a>><br></span><b>Cc: </b>"Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, <a href="mailto:reviews%2BD19950%2Bpublic%2B38ba22078c2035b8@reviews.llvm.org" target="_blank">reviews+D19950+public+38ba22078c2035b8@reviews.llvm.org</a>, "David 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" <<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><b>Sent: </b>Wednesday, May 11, 2016 6:01:32 PM<span class=""><br><b>Subject: </b>Re: [PATCH] D19950: Use frequency info to guide Loop Invariant Code Motion.<br><br></span><div dir="ltr"><br><span class=""><div>This is probably just a concern in theory -- current store motion only does downward code motion (sink and merge). </div></span></div></blockquote>I'm not sure I understand the example. To host the cold_load, it must not alias with any stores in the loop. To hoist the store past the loop, it also must not alias with anything in the loop. <br></div></div></blockquote><div><br></div><div>The store is only aliased with the load. As the load is hoisted, the store can also be hoisted.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:#000000"><span class=""><br><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><div dir="ltr"><div></div><div><br></div><div>While upward code motion for stores is also possible (e.g. to shrink live ranges of the stored value and address val), it is not likely done as an IR optimization pass.</div></div></blockquote><br></span>This reminds me of a conversation I was having with Philip some weeks ago about how InstCombine has a somewhat-unfortunate heuristic of sinking instructions with a single use in only one predecessor into that predecessor. It does this even for instructions with potential side effects, and so we lose the ability to hoist them back again. Hosting them back might be important later for scheduling, etc. For what it wants to do, however, the heurisitic is also not strong enough because it a) does not handle multiple uses in the predecessor and b) only looks in direct predecessors, not any dominated block. Sort of the worst of both worlds ;)<br><br> -Hal<br><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><div dir="ltr"><div></div><div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Wed, May 11, 2016 at 3:44 PM, Dehao Chen <span dir="ltr"><<a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5"><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>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>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></div><div><div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">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></div></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span><div><div class="h5">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></div></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span><hr><div><div class="h5"><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>
</div></div></span><div><div class="h5"><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. </div></div></blockquote><div><br></div><div><br></div></span><div><div class="h5"><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><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);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><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);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><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);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><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);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></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</blockquote><div><div class="h5"><br><br><br>-- <br><div><span name="x"></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></div></div></div></div></blockquote></div><br></div></div>