[PATCH] D64972: [Loop Peeling] Do not close further unroll/peel if profile based peeling was not used

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 1 17:03:32 PDT 2019


reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM.

Serguei and I spent a fair amount of time talking about this one offline.  Neither of us are completely happy with the structure of this patch, but since it's using the same pattern as what was already there (just a bit more fine grained), I decide to approve this so as to unblock Serguei's other work.

He and I are planning to continuing thinking about the code structure here, and may come back with an NFC for the whole area if we can find a design that seems cleaner.  Ideas welcome.



================
Comment at: llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp:1140
   // mark loop as unrolled to prevent unrolling beyond that requested.
   // If the loop was peeled, we already "used up" the profile information
   // we had, so we don't want to unroll or peel again.
----------------
The more I look at this comment, the more it just feels wrong.  Absolutely not a blocking item for this patch though!


================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp:330
       assert(DesiredPeelCount > 0 && "Wrong loop size estimation?");
       LLVM_DEBUG(dbgs() << "Peel " << DesiredPeelCount
                         << " iteration(s) to turn"
----------------
Sink this debug output under your if please.


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

https://reviews.llvm.org/D64972





More information about the llvm-commits mailing list