[PATCH] D13087: A fix for loop vectorizer with handling loops with volatile induction variables

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

Just a few drive-by comments. With the current (new, and different from the description) direction, I think Hal has the rest of the review covered.


================
Comment at: include/llvm/Transforms/Utils/Local.h:140-142
@@ -138,4 +139,5 @@
 ///
 bool SimplifyCFG(BasicBlock *BB, const TargetTransformInfo &TTI,
-                 unsigned BonusInstThreshold, AssumptionCache *AC = nullptr);
+            unsigned BonusInstThreshold, AssumptionCache *AC = nullptr, 
+            SmallPtrSet<BasicBlock*, 16> * LoopHeaders = nullptr);
 
----------------
Try using clang-format to re-format the code.

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:211
@@ -208,1 +210,3 @@
+          BB->getFirstNonPHIOrDbg()->isTerminator()  &&
+          !LoopHeaders.count(BB)) {	      
         // Since TryToSimplifyUncondBranchFromEmptyBlock may delete the
----------------
Again, clang-format would be a useful tool to ensure consistent formatting.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:4624
@@ +4623,3 @@
+  // (This is for early invocations before loop simplify and vectorization
+  // to keep canonical loop forms for loops with volatile induction variables.
+  // These blocks can be eliminated when the pass is invoked later 
----------------
hfinkel wrote:
> loops with volatile induction variables -> nested loops
> 
> (the problem is much more general than loops with volatile induction variables)
Note that the entire patch description really needs to be updated to correctly indicate that this is about preventing passes from destroying important loop structure, especially of nested loops.


Repository:
  rL LLVM

http://reviews.llvm.org/D13087