[PATCH] D31613: [LoopPeeling] Get rid of Phis that become invariant after N steps

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev marked 2 inline comments as done.
mkazantsev added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:165
+      DesiredPeelCount = std::min(DesiredPeelCount, MaxPeelCount);
+      assert(DesiredPeelCount > 0 && "Wrong loop size estimation?");
+      DEBUG(dbgs() << "Peel " << DesiredPeelCount << " iteration(s) to get rid"
----------------
sanjoy wrote:
> mkazantsev wrote:
> > mkazantsev wrote:
> > > sanjoy wrote:
> > > > Can `UP.Threshold` be `0` (say it was directly set by the user by `UnrollThreshold`)?
> > > Only if loop size is also 0, we enter here witn condition 2 * LoopSize <= UP.Threshold :) So I Don't think it's real.
> > Line 131 rejects empty loops:
> >   if (!L->empty())
> >     return;
> > 
> I don't think line 131 has anything to do with `LoopSize`, it is just checking if there are *subloops* or not.
> 
> But I think your point on `LoopSize` being non-zero is correct.  Can you please add an assert?
Ok. Zero loop size is not allowed where it is calculated, like

  // Don't allow an estimate of size zero.  This would allows unrolling of loops
  // with huge iteration counts, which is a compile time problem even if it's
  // not a problem for code quality. Also, the code using this size may assume
  // that each loop has at least three instructions (likely a conditional
  // branch, a comparison feeding that branch, and some kind of loop increment
  // feeding that comparison instruction).
  LoopSize = std::max(LoopSize, BEInsns + 1);

So I'll add this assert in the very beginning of the method.


https://reviews.llvm.org/D31613