<div dir="ltr">I suspect such a change will only paper over your actual problem.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 12, 2015 at 2:39 PM, Hariharan Sandanagobalane via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@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">The following change fixes this issue in trunk. Does this fix look good?<br>
<br>
Thanks<br>
Hari<br>
<br>
$ svn diff PruneEH.cpp<br>
Index: PruneEH.cpp<br>
===================================================================<br>
--- PruneEH.cpp (revision 243617)<br>
+++ PruneEH.cpp (working copy)<br>
@@ -268,7 +268,7 @@<br>
std::vector<BasicBlock*> Succs(succ_begin(BB), succ_end(BB));<br>
<br>
for (unsigned i = 0, e = Succs.size(); i != e; ++i)<br>
- Succs[i]->removePredecessor(BB);<br>
+ Succs[i]->removePredecessor(BB, true);<br>
<br>
BB->eraseFromParent();<br>
<span class=""> }<br>
<br>
On Mon, Aug 10, 2015 at 1:24 PM, Hariharan Sandanagobalane<br>
</span><div><div class="h5"><<a href="mailto:hariharan.gcc@gmail.com">hariharan.gcc@gmail.com</a>> wrote:<br>
> Hi,<br>
> Simple description of the problem below. I have code coming into<br>
> pruneEH as follows<br>
> fn a {<br>
> entry:<br>
> call fn b<br>
> ...<br>
><br>
> for_cond:<br>
> %i = phi [1, entry] [%x, for_body]<br>
> cmp $i with someval<br>
> cond-br for_body or for_exit<br>
><br>
> for_body:<br>
> ...<br>
> $x = $i + 1<br>
> branch for_cond<br>
><br>
> for_exit<br>
> ...<br>
> }<br>
><br>
> PruneEH determines that the call to fn-b won't return. The code is<br>
> modified thus.<br>
><br>
> fn a {<br>
> entry:<br>
> call fn b<br>
> unreachable insn /* Instructions after call to fn-b replaced with<br>
> unreachable insn */<br>
><br>
> for_cond: /* No path from entry block */<br>
> %i = phi [%x, for_body]<br>
> cmp $i with someval<br>
> cond-br for_body or for_exit<br>
><br>
> for_body:<br>
> ...<br>
> $x = $i + 1<br>
> branch for_cond<br>
><br>
> for_exit<br>
> ...<br>
> }<br>
><br>
> which then becomes<br>
><br>
> fn a {<br>
> entry:<br>
> call fn b<br>
> unreachable insn /* Instructions after call to fn-b replaced with<br>
> unreachable insn */<br>
><br>
> for_cond: /* No path from entry block */<br>
> cmp $x with someval<br>
> cond-br for_body or for_exit<br>
><br>
> for_body:<br>
> ...<br>
> $x = $x + 1<br>
> branch for_cond<br>
><br>
> for_exit<br>
> ...<br>
> }<br>
><br>
> The instruction<br>
> $x = $x + 1<br>
> is obviously illegal in SSA form, which shows up as an infinite loop<br>
> in value numbering.<br>
><br>
> The source of the problem exists in BasicBlock::removePredecessor<br>
> function in BasicBlock.cpp. The comment in that function describes<br>
> this exact scenario<br>
><br>
> // If there are exactly two predecessors, then we want to nuke the PHI nodes<br>
> // altogether. However, we cannot do this, if this in this case:<br>
> //<br>
> // Loop:<br>
> // %x = phi [X, Loop]<br>
> // %x2 = add %x, 1 ;; This would become %x2 = add %x2, 1<br>
> // br Loop ;; %x2 does not dominate all uses<br>
> //<br>
> // This is because the PHI node input is actually taken from the predecessor<br>
> // basic block. The only case this can happen is with a self loop, so we<br>
> // check for this case explicitly now.<br>
><br>
> but goes on to cause the same issue. There are 2 potential problems in<br>
> this function.<br>
><br>
> 1. The comment above describes a self-loop block. The same problem can<br>
> occur in loops with more than 1 block, as our example shows. In<br>
> general, this can happen when the predecessor being removed does not<br>
> belong to the same loop level as the basic block containing the<br>
> PhiNode.<br>
> 2. The version which introduced this comment r2694 did implement the<br>
> self-loop case okay. A subsequent change - revision 22664 - broke<br>
> this.<br>
><br>
> The revision 22664 dates back to 2005, so this issue probably has been<br>
> around for 10 years. I am not sure why nobody else has seen a problem<br>
> here.<br>
><br>
> I saw this issue in a large testcase. I will try to get a small repro<br>
> to illustrate the issue.<br>
><br>
> Regards<br>
> Hari<br>
_______________________________________________<br>
</div></div>llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>