<div dir="ltr">There's the explicit sinking code in the main worklist processing loop around TryToSinkInstruction. I don't believe that can move code into a loop since it can only move into a successor block with no predecessors. So I don't believe that's what happened in the addressspace cast case.<div><br></div><div>Then there's accidental sinking that occurs when InstCombine looks at two or more instructions with some in another basic block and combines them to two or more instructions. In that case all of the combined instructions will be created in the block that contains the instruction that started the combine since that's going to be the insert location for the IRBuilder. I think that's what happened in the addressspace cast case. I don't know how to fix this without qualifying every transform that produces mutliple instructions with a check that all the original instructions started in the same basic block.</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 11:15 AM, Davide Italiano 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"><div dir="auto"><span class=""><br><div class="gmail_extra" dir="auto"><br><div class="gmail_quote">On Sep 12, 2017 1:34 AM, "David Chisnall via llvm-dev" <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<blockquote class="m_8543397351472764653quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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.<br></blockquote></div></div><div dir="auto"><br></div></span><div dir="auto">My somewhat long-winded point is instead that IC shouldn't even bother trying to sink. I understand it may eventually expose other optimizations but it doesn't seem to be worth the complexity added. As you say, the heuristic is pretty simple (fold instructions to blocks with a single predecessor, IIRC, but don't quote me on that). There are cases where it regresses, and I'm not surprised as sinking is not done after a proper analysis (as, e.g. the one GVNSink or LICM are doing). You could probably catch some cases teaching IC about profile metadata when taking sink decisions, but that sounds like adding even more madness.</div><div dir="auto"><br></div><div dir="auto">--</div><div dir="auto">Davide</div><div class="gmail_extra" dir="auto"><div class="gmail_quote"><blockquote class="m_8543397351472764653quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote></div></div></div>
<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>
<br></blockquote></div><br></div>