<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 4, 2018 at 5:30 PM, Michael Zolotukhin 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mzolotukhin added a comment.<br>
<br>
I looked deeper into this, and I think you exposed several issues here:<br>
<br>
- The entire thing is triggered by the fact that the value we're rewriting is used in an unreachable block. We might just ignore such cases (see the loop on lines 102-110).<br>
- The loops in this test are not in simplified form. While they are not guaranteed to be in it, it might indicate that we have a bug in some other pass that fails to preserve the loop in a simplified form. Whatever fix for the current issue we choose, it would be interesting to understand what led to this situation.<br>
<br>
Returning back to the issue in question, here is my understanding of what happens in this case. LCSSA adds a phi-node for the `%value`. The phi-nodes it creates in the process happen to land into another loop, and thus we need to build LCSSA for those loop too (that's why we iterate several times in the worklist loop). When we add LCSSA phi-nodes for the second loop, we again insert phi-nodes into yet another loop. This continues until we convert all touched loops to LCSSA form. What happens in this process is that a PHI-node might have 0 uses at the time it was inserted first time, but as we keep inserting new phis, it might happen to be used in some of the latter phis - which is why the assertion fails. I think the fix should be to just replace the assert with the check (+add a comment about it). After this some redundant phi-nodes might remain in the program, but that could be fixed by adding operands of the phi to `PHIsToRemove` if they are not used anywhere except the phi, which we're removing.<br></blockquote><div><br></div><div><br></div><div>Your understanding is roughly correct.</div><div>FWIW: This is because how we do LCSSA is iterative.</div><div>It's possible to do it in a single pass (gcc does, see tree-ssa-loop-manip.c)</div><div><br></div><div>That would also cause you to have no useless phis.</div><div><br></div><div>The useless phis can also be removed by the scc based ssa minimization algorithm very quickly as well.</div><div><br></div><div>(Also note: gcc is 10-100x faster on larger cases at loop closed ssa formation :()</div><div><br></div><div><br></div><div><br></div></div></div></div>