[PATCH] D21112: [LoopSimplify] Preserve LCSSA when merging exit blocks.

[Michael Zolotukhin via llvm-commits]

mzolotukhin added inline comments.

================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:753
@@ -750,2 +752,3 @@
       AU.addPreserved<SCEVAAWrapperPass>();
+      AU.addPreservedID(LCSSAID);
       AU.addPreserved<DependenceAnalysisWrapperPass>();
----------------
sanjoy wrote:
> We should definitely add asserts verifying this.  I'd suggest something like
> 
> ```
> LoopSimplify::runOnFunction() {
> #ifndef NDEBUG
>   DenseSet<Loop *> InLCSSA;
>   // put loops that are in LCSSA in InLCSSA
> #endif
> 
>   .. do whatever it is that LoopSimplify does ..
> 
> #ifndef NDEBUG
>   // assert every Loop in InLCSSA is in LCSSA
> #endif
> }
> ```
> 
> (i.e. we should also catch cases where simplifying a loop breaks LCSSA in another loop).
Good point. I'll add some verification.

================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:786
@@ -785,3 +788,3 @@
   AC = &getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
   bool PreserveLCSSA = mustPreserveAnalysisID(LCSSAID);
 
----------------
sanjoy wrote:
> Can this be folded to `true`?  (I'm not asking you to do that in this change itself.)
Theoretically, there might be users that don't want LCSSA to be preserved. They don't have LCSSA prior loop-simplify, so PreserveLCSSA is `false` for them. But I'm not sure if such users exist in real world, and I'd be glad to replace it with plain `true` and always preserve LCSSA here (and eventually in all loop passes).


http://reviews.llvm.org/D21112