[PATCH] Teaching loop simplify to preserve LCSSA

Chandler Carruth chandlerc at gmail.com
Mon May 5 15:56:12 PDT 2014


================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:700-701
@@ -699,4 +699,4 @@
 
-      BI->getSuccessor(0)->removePredecessor(ExitingBlock);
-      BI->getSuccessor(1)->removePredecessor(ExitingBlock);
+      BI->getSuccessor(0)->removePredecessor(ExitingBlock, true);
+      BI->getSuccessor(1)->removePredecessor(ExitingBlock, true);
       ExitingBlock->eraseFromParent();
----------------
This seems like a ... very heavy hammer. And yet, it is used pervasively. I think this code should be written differently, but your patch doesn't make it any worse.

If you do this, you should make the actual LoopSimplify pass preserve LCSSA.

It would also be good to write some direct test cases for LoopSimplify preserving LCSSA.

================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:472-473
@@ -471,5 +471,4 @@
     if (OuterL) {
-      ScalarEvolution *SE = PP->getAnalysisIfAvailable<ScalarEvolution>();
-      simplifyLoop(OuterL, DT, LI, PP, /*AliasAnalysis*/ 0, SE);
-      formLCSSARecursively(*OuterL, *DT, SE);
+      simplifyLoop(OuterL, DT, LI, PP, /*AliasAnalysis*/ 0,
+                   PP->getAnalysisIfAvailable<ScalarEvolution>());
     }
----------------
Hmm, I'm a touch hesitant to just remove this. When I first changed the loop pass stuff it was... surprisingly noisy due to how much broke.

If we're going to switch back, I'd like to do it in a separate commit so its easier to revert if things break. Have you tested this change on any large code bases out of curiosity?

http://reviews.llvm.org/D2986






More information about the llvm-commits mailing list