[PATCH] D32353: [LoopSimplify] Simplify constant conditional branches to unconditional branches

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:97
+  // So, we need to update the relevant data structures.
+  // NB: This will never delete any loops completely. It will only delete
+  // blocks within loops.
----------------
anna wrote:
> anna wrote:
> > efriedma wrote:
> > > efriedma wrote:
> > > > This is pretty clearly false... for example, consider:
> > > > 
> > > >   OldDest:
> > > >     br label %OldDest
> > > > 
> > > > OldDest is no longer a Loop in the LLVM sense because loops only exist in reachable code.
> > > > 
> > > > Or consider the case where OldDest == block; L is not a loop anymore because you deleted the backedge.
> > > Err, the OldDest == block example doesn't precisely work because the other branch would be an exit.  Better example:
> > > 
> > >   block1:
> > >     br label %block2
> > > 
> > >   block2:
> > >     bi i1 true, label %block2, label %block1
> > > 
> > > You're destroying the outer loop by deleting its backedge.
> > In the first example:
> > ```
> > OldDest:
> >   br label %OldDest
> > ```
> > I was under the impression we are only worried about making a loop become a non-loop by removing back edge (see line 70). From what I understood, we *are removing loops* if a loop that was originally unreachable, i.e. the selfLoop `OldDest` is now *effectively* unreachable. By effectively unreachable, I mean the edge that targets the `OldDest` is deleted (when it was originally unreachable - the branch existed, but it was never taken).
> The outerloop example is very useful. Thank you! I think even changing the check in line 70, to consider `getLoopFor` for both target blocks would not be enough. Again, it would be making an llvm loop become a non-loop because it is not reachable. Will have to think about this more.
For the first example, folding the branch turns a loop into unreachable code. That unreachable code might look like a loop, but LoopInfo ignores any code which isn't reachable from the entry block, so you have to erase the Loop* for that loop.


https://reviews.llvm.org/D32353