[PATCH] D14526: Don't recompute LCSSA after loop-unrolling when possible.

[Michael Zolotukhin via llvm-commits]

mzolotukhin added a comment.

Hi Mehdi,

Thanks for the remarks, replies below:

Michael


================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:233
@@ -224,1 +232,3 @@
+    }
+  }
 
----------------
joker.eph wrote:
>  `LI->getLoopFor(BB) != ParentL; ` could be written `ParentL->contains(BB)`;
> 
> Also, as an alternative to the raw loop: 
> 
> ```
>  bool AllExitsAreInsideParentLoop = std::all_of(ExitBlocks.begin(), ExitBlocks.end(), [&] (BasicBlock *BB) { return ParentL->contains(BB); })
> ```
> 
> 
Good idea, thanks!

================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:567
@@ -556,2 +566,3 @@
 
-      formLCSSARecursively(*OuterL, *DT, LI, SE);
+      if (CompletelyUnroll && !AllExitsAreInsideParentLoop)
+        formLCSSARecursively(*OuterL, *DT, LI, SE);
----------------
joker.eph wrote:
> Are we sure that there is no case where partial unrolling can break the LCSSA form for the enclosing loop? I don't think so but not sure why or if it is an implementation guarantee that can be lost?
> Also, are we allowed to break LCSSA for the current loop in case of partial unrolling? I.e. if I have a pass manager that contains unrolling + another loop pass, and the second expect LCSSA form for every loop, it seems your patch could change that (unless partial unrolling guarantee LCSSA on the partially unrolled loop as well).
AFAIU, partial unrolling shouldn't break LCSSA. The reason is that we don't change loops structure and don't add/remove exits from loops. Provided LCSSA was valid before partial unrolling, it should remain valid as all LCSSA phis would stay in correct places, and we won't create new in-loop definitions with uses outside the loop and those phis.
Complete loop unrolling, in contrast, changes loop structure and might change exits even for an outer loop - that's why we usually need to rebuild LCSSA after it. However, if all loop exits are inside the parent loop, then full unrolling of the inner loop shouldn't change exits for outer loops, and we won't create definitions inside the loop with uses outside (since outside uses would be in phis, which are located in exits, which are inside the enclosing loop). Thus, in this case there is also no need to rebuild LCSSA.
If my logic is flawed anywhere, I'd appreciate to be corrected!


http://reviews.llvm.org/D14526