<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 5, 2016 at 1:51 PM, David Callahan <span dir="ltr"><<a href="mailto:dcallahan@fb.com" target="_blank">dcallahan@fb.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word;color:rgb(0,0,0);font-size:14px;font-family:Calibri,sans-serif">
<div>
<div>Thanks for the GNU pointer.</div>
<div><br></div></div></div></blockquote><div>Of course.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;color:rgb(0,0,0);font-size:14px;font-family:Calibri,sans-serif"><div><div>
</div>
<div>Rather than replacing a “dead” conditional branch with an unconditional branch the live post-dominator, the make it unconditional to a preferred successor and then remove the other outgoing edges. </div></div></div></blockquote><div><br></div><div>Right, it implements the morgan textbook algorithm, which is a little less advanced, but may not matter in practice for most normal CFG's.</div><div><br></div><div>(the nearest live post-dominator is going to be somewhere in the same DFS tree branch, but maybe not this one)</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;color:rgb(0,0,0);font-size:14px;font-family:Calibri,sans-serif"><div><div>The preferred successor is the smallest in an inverted
post order numbering of the CFG. This means that PHI nodes get updated only as a side effect of removing edges which does seem simpler.</div>
<div><br>
</div>
<div>Let me code that and see how it compares.</div>
</div>
<div><br>
</div>
<div><br>
</div>
<span id="m_-223590906335516294OLK_SRC_BODY_SECTION">
<div style="font-family:Calibri;font-size:11pt;text-align:left;color:black;BORDER-BOTTOM:medium none;BORDER-LEFT:medium none;PADDING-BOTTOM:0in;PADDING-LEFT:0in;PADDING-RIGHT:0in;BORDER-TOP:#b5c4df 1pt solid;BORDER-RIGHT:medium none;PADDING-TOP:3pt">
<span style="font-weight:bold">From: </span>Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>><br>
<span style="font-weight:bold">Date: </span>Monday, December 5, 2016 at 12:33 PM<span class=""><br>
<span style="font-weight:bold">To: </span>"<a href="mailto:reviews+D24918+public+0f389072c9f76e8c@reviews.llvm.org" target="_blank">reviews+D24918+public+<wbr>0f389072c9f76e8c@reviews.llvm.<wbr>org</a>" <<a href="mailto:reviews+D24918+public+0f389072c9f76e8c@reviews.llvm.org" target="_blank">reviews+D24918+public+<wbr>0f389072c9f76e8c@reviews.llvm.<wbr>org</a>><br>
<span style="font-weight:bold">Cc: </span>David Callahan <<a href="mailto:dcallahan@fb.com" target="_blank">dcallahan@fb.com</a>>, David Majnemer <<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>>, "<a href="mailto:nadav.rotem@me.com" target="_blank">nadav.rotem@me.com</a>"
<<a href="mailto:nadav.rotem@me.com" target="_blank">nadav.rotem@me.com</a>>, Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>>, llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>, Kevin Frei <<a href="mailto:freik@fb.com" target="_blank">freik@fb.com</a>>,
Taewook Oh <<a href="mailto:twoh@fb.com" target="_blank">twoh@fb.com</a>><br>
<span style="font-weight:bold">Subject: </span>Re: [PATCH] D24918: [ADCE] Add code to remove dead branches<br>
</span></div>
<div><br>
</div>
<div>
<div>
<div dir="ltr"><br><div><div class="h5">
<div class="gmail_extra"><br>
<div class="gmail_quote">On Mon, Dec 5, 2016 at 10:41 AM, David Callahan via Phabricator
<span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</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">
david2050 added a comment.<br>
<br>
Regarding the question of simpler update to the control flow, I reviewed two references from @dberlin:<br>
<br>
(1) <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__www.clear.rice.edu_comp512_Lectures_10Dead-2DClean-2DSCCP.pdf&d=DgMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=lFyiPUrFdOHdaobP7i4hoA&m=a2z_JzEUPXIR8SW8ncviG0QnzTSu5qgwQaVsknCXa5k&s=_LlVFr4CX1Vt9bxHJQIOgbALfcgzVvktMe7lJEiIDdQ&e=" rel="noreferrer" target="_blank">
https://www.clear.rice.edu/com<wbr>p512/Lectures/10Dead-Clean-SCC<wbr>P.pdf</a><br>
<br>
In this work, the PHI nodes contain only references to named definitions and do not include a reference to the predecessor blocks associated with reaching paths from those definitions.<br>
</blockquote>
<div> </div>
<div>Yup, i'm not sure what the issue is that makes this harder :)</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>
(2) <a href="https://github.com/apple/swift/blob/master/lib/SILOptimizer/Transforms/DeadCodeElimination.cpp" rel="noreferrer" target="_blank">
https://github.com/apple/swift<wbr>/blob/master/lib/SILOptimizer/<wbr>Transforms/DeadCodeElimination<wbr>.cpp</a><br>
<br>
This swift variant makes no explicit reference to Phi nodes and does neither updates them nor manages explicit predecessors as far as I can tell.<br>
</blockquote>
<div>You keep saying explicit predecessors, but again, i'm not sure what the issue there is.</div>
<div>LLVM does not actually have explicit predecessors.</div>
<div>The predecessor iterators walk over the users of the basic block (which is a Value, and so has Uses/Users) to see which terminator instructions use it.</div>
<div>It then hands those blocks as predecessors.</div>
<div><br>
</div>
<div>So, for example, changing a *use* in a conditional branch updates the predecessors for the used block.</div>
<div><br>
</div>
<div>It's certainly true that this does not update phi nodes, which store explicit references, but that does not seem difficult to handle (since you can update them when you change the conditional branch, since at that point you know which phi nodes have edges
that are about to change).</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">
Does anyone have another pointer to compiler which have SSA-based dead code elimination including control flow which might guide to a similar approach the code here?<br>
</blockquote>
<div><br>
</div>
<div>GCC has explicit edges, and phi nodes have explicit blocks.</div>
<div><a href="https://gcc.gnu.org/svn/gcc/trunk/gcc/tree-ssa-dce.c" target="_blank">https://gcc.gnu.org/svn/gcc/<wbr>trunk/gcc/tree-ssa-dce.c</a><br>
</div>
<div>(ignore the virtual phi handling, which is memoryssa updating, not regular CFG updating).</div>
<div><br>
</div>
<div>Note it does not do anything amazingly difficult :)</div>
<div><br>
</div>
<div>In any case, this seems like one of those things where if we had a whiteboard in person, we could probably just solve it in 10 minutes.</div>
<div><br>
</div>
<div>I don't think this should block the patch right now, since this is not heavily used or if you think this is too hard to do, let's just get this in and iterate on it in tree.</div>
<div><br>
</div>
<div><br>
</div>
<div><br>
</div>
</div>
</div>
</div></div></div>
</div>
</div>
</span>
</div>
</blockquote></div><br></div></div>