[PATCH] D32353: [LoopSimplify] Simplify constant conditional branches to unconditional branches
Anna Thomas via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 21 13:45:29 PDT 2017
anna planned changes to this revision.
anna added a comment.
Based on Eli's comments, this definitely needs more thought.
================
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.
----------------
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).
================
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:
> 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.
https://reviews.llvm.org/D32353
More information about the llvm-commits
mailing list