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

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 23 19:37:11 PST 2017


davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

I have a fundamental question about the constructors. Also, I would prefer the spelling `FullUnroll` instead of `SimpleUnroll`.



================
Comment at: include/llvm/Transforms/Scalar/LoopUnrollPass.h:25-31
+public:
+  static LoopUnrollPass createSimple() {
+    return LoopUnrollPass(/*AllowPartialUnrolling*/ false);
+  }
+  static LoopUnrollPass create() {
+    return LoopUnrollPass(/*AllowPartialUnrolling*/ false);
+  }
----------------
Is it intended that you're passing `false` to both `create()` and `createSimple()` ?
Shouldn't `create()` allow the entire thing, including partial unrolling? Here you don't enable it at all.


https://reviews.llvm.org/D28897





More information about the llvm-commits mailing list