[PATCH] D28897: [PM] Simplify the new PM interface to the loop unroller and expose two factory functions for the two modes the loop unroller is actually used in in-tree: simplified full-unrolling and the entire thing including partial unrolling.

[Michael Zolotukhin via Phabricator via llvm-commits]

mzolotukhin added inline comments.


================
Comment at: lib/Passes/PassRegistry.def:228
+LOOP_PASS("unroll", LoopUnrollPass::create())
+LOOP_PASS("unroll-simple", LoopUnrollPass::createSimple())
 LOOP_PASS("print-access-info", LoopAccessInfoPrinterPass(dbgs()))
----------------
I agree with @davide regarding the name of the pass. IMO `unroll-full` better reflects what the pass is doing.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:1124
 Pass *llvm::createSimpleLoopUnrollPass() {
   return llvm::createLoopUnrollPass(-1, -1, 0, 0, 0);
 }
----------------
Do we want to use `LoopUnrollPass::createSimple` here? This part (and the function above) seems to be doing almost the same as we do with the new functions.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:1158
+                                 /*PreserveLCSSA*/ true, /*Count*/ None,
+                                 /*Threshold*/ None, AllowPartialParam,
+                                 RuntimeParam, UpperBoundParam);
----------------
How do we use `ProvidedThreshold` and `ProvidedThreshold` now?


https://reviews.llvm.org/D28897