[PATCH] D36157: [PM] Split LoopUnrollPass and make partial unroller a function pass

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson marked 5 inline comments as done.
tejohnson added inline comments.


================
Comment at: include/llvm/Transforms/Scalar/LoopUnrollPass.h:24
 public:
-  /// Create an instance of the loop unroll pass that will support both full
-  /// and partial unrolling.
-  ///
-  /// This uses the target information (or flags) to control the thresholds for
-  /// different unrolling stategies but supports all of them.
-  static LoopUnrollPass create(int OptLevel = 2) {
-    return LoopUnrollPass(/*AllowPartialUnrolling*/ true, OptLevel);
-  }
-
-  /// Create an instance of the loop unroll pass that only does full loop
-  /// unrolling.
-  ///
   /// This will disable any runtime or partial unrolling.
+  explicit LoopFullUnrollPass(int OptLevel = 2) : OptLevel(OptLevel) {}
----------------
chandlerc wrote:
> This comment seems a bit stale now.
Removed. I initially left it in since it is still invoking the same underlying unrolling facility, but disabling runtime and partial unrolling.

As an aside, I just discovered that peeling is *not* disabled, and so the complete unroller is performing profile based peeling. I will send a follow on patch to disable that as well, since I want this one to be as NFC as possible. 


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:1247
+  auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
+
+  SmallVector<Loop *, 8> Worklist;
----------------
chandlerc wrote:
> I would directly simplify the loops and form LCSSA here so that you have the requirements for unrolling. That's similar to what the vectorizer does, although it doesn't seem to care about LCSSA and the unroller will.
Done, and removed the places where I explicitly added loop-simplify,lcssa to the pass manger invocations in the runtime-loop*.ll tests.


https://reviews.llvm.org/D36157