[PATCH] D64972: [Loop Peeling] Do not close further unroll/peel if not full peeling happens

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

I'm not quite sure this is the right macro approach.

The comment near the code your modifying makes me thing the whole reasoning behind disabling future peeling and unrolling after the first peel may be flawed.  It doesn't make sense to "use up" profiling information.  Assuming we correctly updated our profiling when doing the transform, the resulting profile for the loop should indicate that it's cold and thus not profitable to further peel/unroll.  If that's not happening, maybe there's another issue in play?  (I wonder if your other profile bug fix may help here?)

p.s. I'm opened to being convinced that this is a practical answer, even if not an ideal one.  Just make the argument.  :)



================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:478
     bool UnrollAndJam;
+    /// Allow full peeling. Uses to enable peeling off all iterations which
+    /// closes further unroll/peeling optimizations on the loop.
----------------
Naming wise: Please call this something other than "Full" peeling.  Full peeling sounds like full unrolling which would imply we were able to entirely eliminate the loop structure.

Maybe: PeelProfiledIterations?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64972/new/

https://reviews.llvm.org/D64972