<div dir="ltr">This sounds less like the explicit sinking code where InstCombine tries to analyzer users and just moves already existing instructions. That code shouldn't be able to move into loops because it checks that the successor its moving into only has a single predecessor.<div><br></div><div>I think this case is a consequence of InstCombine creating new instructions in the basic block that the parent instruction of the transform lives in.</div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div>
<br><div class="gmail_quote">On Tue, Sep 12, 2017 at 1:33 AM, David Chisnall via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 11 Sep 2017, at 21:32, Sean Silva via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
> As a data point for this, I did a quick walkthrough of the top-level of instcombine, and one thing that stuck out to me is that it tries to sink instructions in certain "easy" scenarios. That doesn't fit a graph rewriting system.<br>
<br>
</span>The sinking in instcombine is not always a win.  We hit a case recently where instcombine made a hot loop in a benchmark significantly slower by moving an address-space cast instruction into a loop as part of its gep of addrspacecast to addrspacecast of gep transformation.  On our architecture, this translates to an extra instruction in a hot loop for the address-space cast *and* means that we then can’t use the complex addressing modes for the loads and stores because we have an address space cast in between the GEP and the memory op.  We end up increasing the number of instruction in the loop by around 20%, with a corresponding decrease in performance.<br>
<br>
My somewhat long-winded point is that I’m not convinced that InstCombine is doing sufficient reasoning when it moves instructions between basic blocks to determine that the moves actually make sense.  In this benchmark, this one misoptimisation offset all of the other gains from InstCombine and simply disabling it gave us better code.<br>
<br>
I don’t know how many other examples of this there are, but it would probably be good to have an InstCombine that ran on single basic blocks (or sequences with trivial flow control) and did folding that was always a win and something separate that’s more path-aware (and probably doesn’t run at O1).<br>
<br>
David<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
</div></div></blockquote></div><br></div>