<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 23, 2015 at 11:05 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">Will try to reply to the larger thread again soon, but a quick reply on a bit of a tangent...</div><div class="gmail_extra"><span class=""><br><div class="gmail_quote">On Mon, Feb 23, 2015 at 8:45 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="overflow:hidden"> 2. Should unreachable code be allowed to contain nonsense (like instructions that depend on themselves)? To this, I believe the answer is no. We currently permit this, and I think that a lot of the bugs regarding unreachable code some from this. I've yet to hear a good argument why, for example, JumpThreading must produce self-referential instructions in trivially-dead regions.<br></div></blockquote></div><br></span>I could easily see tightening the rules on what passes the verifier within unreachable code to try to make it substantially cheaper, easier, and less error prone to handle unreachable code. It seems likely that there are good pragmatic compromises here that would actually cost nothing. </div></div></blockquote><div><br></div><div><br></div><div>One such compromise (which you didn't seem fond of) was that "unreachable blocks have to contain nothing but a terminator".</div><div><br></div><div>Earlier, you said "<span style="font-size:12.8000001907349px">The code which is most likely to create unreachable code is similarly likely to destroy the analysis information we would use to remove it cheaply.</span><span style="font-size:12.8000001907349px"> " and "</span><span style="font-size:12.8000001907349px">When transforming the IR we must never forget about regions we have made unreachable.</span><span style="font-size:12.8000001907349px"> "  </span></div><div><br></div><div>I would like to challenge this assertion with actual evidence:<br>Right now the state of the world is actually "the vast majority of llvm passes try to clean up after themselves, try not to create weird unreachable code".</div><div><br></div><div>GVN removes dead blocks that replacement of values/equality propagation creates</div><div>SCCP deletes all instructions in dead blocks except terminators (because it preserves CFG)</div><div>ADCE does the same (it never removes terminators)</div><div>DCE the same</div><div><div>IPSCCP actually removes dead blocks, because it does not preserve CFG</div></div><div>JumpThreading removes unreachable blocks on input but not output (and in fact, as you'll see, it's the only pass that seems to really be a problem)</div><div>SROA cleans up unreachable code it generates in it's utility class</div><div>SimplifyCFG removes unreachable blocks</div><div>CorrelatedValuePropagation does the same through a utility (it calls constantfoldterminator, which in turn takes great pains to clean up the CFG properly)<br></div><div>etc</div><div>I didn't look at the loop passes, but outside of jump threading, it appears there is little work to be done to be in a state where things clean up after themselves.</div><div><br></div><div>That is, *right* now, we know for at least a ton of the passes and their analysis, it is not "destroying the analysis information we would use to remove it" because they are in fact removing it.</div><div>They already "do not forget about regions they have made unreachable", because they already clean up after themselves.</div><div><br></div><div>For giggle, i stopped two of them that i happened to know pretty well (GVN, CorrelatedValuePropagation) from removing unreachable blocks, just leaving them unreachable with instructions still in them.</div><div>While it does nothing to cause verifier crashes, it causes a *bunch* of brand new crashes *in* passes in the standard pass ordering, which it wouldn't if we were really living in a world where passes were really handling unreachable code properly already.</div><div><br></div><div>Regardless of what the outcome is (and i'm pretty clearly on one side), we shouldn't go half-way. Right now we are already in a world, where, because we have some passes cleaning up and some not,we have passes that can't handle unreachable code as input, and some that can.<br></div><div>So if you change the pass order, and those passes do something, you get wonderful crashes.  </div><div>Either we should be testing passes with unreachable code as input, or expecting their not to be any.</div><div>(and the side-issue of "in a world where anything can be in an unreachable block, all of this cleanup and carefulness is a waste of time")</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">Obvious example is, as you highlight, self-referential non-phi instructions could easily just be rejected, and asserted during construction by the IR builder.</div></div>
<br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a>         <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
<br></blockquote></div><br></div></div>