[PATCH] D21665: [LoopSimplify] Update LCSSA after separating nested loops.
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 13 15:17:34 PDT 2016
sanjoy requested changes to this revision.
This revision now requires changes to proceed.
================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:350
@@ +349,3 @@
+ for (BasicBlock *ExitBlock : ExitBlockSet) {
+ for (pred_iterator PI = pred_begin(ExitBlock), PE = pred_end(ExitBlock);
+ PI != PE; ++PI)
----------------
Why not use `predecessors` ?
Actually, why not:
```
if (any_of(predecessors(ExitBlock), [](BasicBlock *BB) {
return !L->contains(BB);
}) {
rewriteLoopExitBlock(L, ExitBlock, DT, LI, PreserveLCSSA);
}
================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:368
@@ +367,3 @@
+ L->getExitBlocks(ExitBlocks);
+ SmallSetVector<BasicBlock *, 8> ExitBlockSet(ExitBlocks.begin(),
+ ExitBlocks.end());
----------------
Why do you need to separately create a `SmallSetVector` here?
================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:374
@@ +373,3 @@
+ // preserve LCSSA.
+ for (Instruction &NewI : *NewBB) {
+ PHINode *PN = dyn_cast<PHINode>(&NewI);
----------------
`NewBB` is the header of the new loop, right? (if so, a naming change
would be nice). Then I'm not sure if scanning `NewBB` for LCSSA
violations is sufficient -- what if we have:
```
loop:
%iv = phi [ 0, %entry ] [ %iv, (%loop, %left, %right)], [ %z, %outer ]
switch(condition), %left, %right, %loop
left:
%x = def()
br condition, %outer_l, %loop
outer_l:
br outer
right:
%y = def()
br condition, %outer_r, %loop
outer_r:
br outer
outer:
%z = phi(%x, %y)
br %loop
```
then the separated structure would be
```
outer_l:
%iv = phi [ 0, %entry ], [ %z, outer ]
br loop
loop:
switch(condition), %left, %right, %loop
left:
%x = def()
br condition, %outer_l, %loop
outer_l:
br outer
right:
%y = def()
br condition, %outer, %loop
outer_r:
br outer
outer:
%z = phi(%x, %y) ;; this PHI needs to be edited as well
br %loop
```
================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:388
@@ +387,3 @@
+ for (BasicBlock *ExitBB : ExitBlockSet) {
+ if (SSAUpdate.HasValueForBlock(ExitBB))
+ continue;
----------------
When is this true?
================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:393
@@ +392,3 @@
+
+ // We've found the exit block where we need to insert the phi-node.
+ // Create it and register in SSAUpdater.
----------------
s/the exit block/an exit block/
s/the phi-node/an LCSSA phi node/
http://reviews.llvm.org/D21665
More information about the llvm-commits
mailing list