<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 6, 2016 at 4:06 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">eli.friedman added a comment.<br>
<br>
It probably doesn't make sense to turn every `call` instruction into an `invoke` or equivalent; that would cause the size of the IR to explode,</blockquote><div><br></div><div>I'm going to disagree. GCC has EH/abnormal "fake" edges to handle these cases, and works much better than LLVM in this regard, and has no such explosion problem on normal C++ code. In fact, it's going to be faster.</div><div> </div><div>If we have a modeling problem, we should fix the modeling problem.</div><div>You can abstract this to the block level. The block has an EH successor, That just means in some fashion, the block may go to this EH edge.</div><div>It does not require you terminate the block at every mayThrow instruction.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> and make the IR more difficult to use for passes which don't actually care about "trivial" edges (including most of GVN itself).<br></blockquote><div><br></div><div>So far this has been added to three passes, each in an N^2 way. It is not the local hoisting that is concerning, it is that every path must have every instruction checked just to get the single bit that there is a possible EH successor for a given block.</div><div><br></div><div>Anything that wants to hoist or sink has to care, and that's a lot of passes, despite your claim.<br></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
It might be possible to add some sort of sub-block abstraction over basic blocks... for example:<br>
<br>
block:<br>
call void @a() ; sub-block 0<br>
%y = add i32 %x, 1 ; sub-block 1<br>
call void @b() ; sub-block 1<br>
ret i32 %y ; sub-block 2<br>
<br>
Then we provide some sort of sub-block tree that includes edges to trivial unwind blocks.<br></blockquote><div>This is not needed, what is needed is to correctly represent that there is possible abnormal control flow. If something wants to go trying to figure out what instructions generated it, they could, but the local computation is already cheap, it's the fact that you have to check every instruction on every path.</div><div><br></div><div><br></div><div>The pass you changed is trying to figure out whether it can hoist into predecessors.</div><div><br></div><div>With EH edges, </div><div><div><br></div><div> // If any of these blocks has more than one successor (i.e. if the edge we</div><div> // just traversed was critical), then there are other paths through this</div><div> // block along which the load may not be anticipated. Hoisting the load</div><div> // above this block would be adding the load to execution paths along</div><div> // which it was not previously executed.</div></div><div><div> if (TmpBB->getTerminator()->getNumSuccessors() != 1)</div><div><span class="" style="white-space:pre"> </span> return false;</div><div><br></div><div><span class="">Will simply fail and we will go about our way.</span></div><div><span class="" style="white-space:pre"><br></span></div><div><span class="" style="white-space:pre">Standard dominance checking will also take care of making sure smarter algorithms don't try to select these things as hoist points.</span></div></div><div><span class="" style="white-space:pre"><br></span></div><div><span class="" style="white-space:pre">Trying to figure out if we can move it "just one more instruction" is pretty much a pointless microoptimization :)</span></div><div><span class="" style="white-space:pre"><br></span></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Assuming we have such a thing, the PRE algorithm can iterate over it to behave correctly. (I'm assuming MemoryDependenceAnalysis itself wouldn't use it because of the overhead involved.)<br>
<br>
The question, of course, is how exactly you would implement this. If you try to compute it on the fly, it boils down to exactly the same loop I've already written. If you try to maintain it as a side-table, you now have a gigantic hashtable containing every instruction in the function, and modifying the IR requires special sub-block-aware methods to insert, remove, or move instructions. Attaching it to the IR itself adds an overhead of at least one pointer per Instruction, plus extra overhead for passes which don't care.</blockquote><div><br></div><div>I don't think any of these are necessary :)</div><div><br></div><div> </div><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
<a href="http://reviews.llvm.org/D21041" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21041</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>