[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.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:1124
 Pass *llvm::createSimpleLoopUnrollPass() {
   return llvm::createLoopUnrollPass(-1, -1, 0, 0, 0);
 }
----------------
mzolotukhin wrote:
> 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.
Yeah, but they need the legacy PM bits so I just didn't want to touch them right away. We could refactor this some in the future if you want.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:1158
+                                 /*PreserveLCSSA*/ true, /*Count*/ None,
+                                 /*Threshold*/ None, AllowPartialParam,
+                                 RuntimeParam, UpperBoundParam);
----------------
mzolotukhin wrote:
> How do we use `ProvidedThreshold` and `ProvidedThreshold` now?
We don't at all. It's just vestigal for the legacy PM. We now have target hooks to control threshold and flags to do debugging. That seemed sufficient to me.


https://reviews.llvm.org/D28897