[PATCH] D25963: [LoopUnroll] Implement profile-based loop peeling

[Michael Kuperstein via llvm-commits]

mkuper added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:47
+    cl::desc("Force a peel count regardless of profiling information."));
+
+// Check whether we are capable of peeling this loop.
----------------
Gerolf wrote:
> Please add an option to turn it off by default.
That option already exists - it's in LoopUnrollPass (because it's needed as part of the UnrollingPreferences initialization), and is currently off by default. These two options also started out living in LoopUnrollPass but moved since I tried to consolidate choosing the peeling factor here.
That's part of the "ugliness" I referred to above.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:125
+                            ValueToValueMapTy &LVMap, LoopInfo *LI) {
+
+  BasicBlock *Header = L->getHeader();
----------------
Gerolf wrote:
> Could this be implemented as
> - distribute k: N-k (assuming trip count N)
> - complete unroll first k iterations?
Anyway, yes, I've originally thought about implementing it like that (it seemed like it'd be simpler, although I'm not entirely sure there's a big difference). But I think a more direct implementation fits the flow of the unroller better.

I wanted to make the "unroll / runtime unroll / peel" decision at the same point (otherwise this could be a separate pass that runs before the unroller, performs the split and marks the two loops as nounroll and force-unroll). And if this lives in the unroller - splitting and then unrolling the new loop while in the middle of "unrolling" the original one seemed awkward.


https://reviews.llvm.org/D25963