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

[Michael Kuperstein via llvm-commits]

mkuper added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:111
+// PreHeader and Exit are the preheader and exit block of the original loop.
+// RemainingHeaderWeight is the number of *dynamic* loop entries still
+// unaccounted for - that is, it is the number of times we expect to enter
----------------
davidxl wrote:
> I think this parameter should better be called 'PeeledHeaderWeight' -- it is really the weight of the 'header' block of the current peeled iteration.  The PeeledHeaderWeight will be updated to the next iteration's header weight after this call is done.
> 
> The name 'Remaining' sounds like the original value of this weight parameter is the sum of all peeled iteration's header weight, but it is actually not.
Sure, I'll change it.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:295
+    // the current peeled-off static iteration uses up.
+    BackEdgeWeight -= ExitWeight;
+
----------------
davidxl wrote:
> If user request a large peel count, this may end up < 0 which should be avoided. Make it as least >=0.
Nice catch, thanks! I actually noticed it, but forgot to change the code.
This was one of the reasons I think the distribution is problematic - this shouldn't happen in a distribution when the peel count really is the average.

(Also, I think it probably should be >=1, not just >=0).


https://reviews.llvm.org/D25963