[LoopUnrolling] Fix loop size check for peeling

[mkazantsev]

Summary

We should check if loop size allows us to peel at least one iteration before we do so.

[mkuper]

LGTM with a style nit.

lib/Transforms/Utils/LoopUnrollPeel.cpp

78

I would combine this into a single if with a &&, rather than two nested ones.

[mkazantsev]

lib/Transforms/Utils/LoopUnrollPeel.cpp

78

We can't combine it with variable's declaration, and declaring the latch externally looks quite ugly.

[mkuper]

lib/Transforms/Utils/LoopUnrollPeel.cpp

78

I think having another level of nesting here is worse than declaring the latch externally.
I'd suggest either (1) declaring it externally, or (2) pull this entire thing into a helper function, in which case you can have an early exit.

[mkazantsev]

lib/Transforms/Utils/LoopUnrollPeel.cpp

78

I just realized that getLoopPatch never returns null, and this is ensured within isLoopSimplifyForm in canPeel method. So I will replace the condition here and make an assert for latch.

[mkuper]

LGTM.
Also, forgot it before, sorry - please add a test. (No need for a huge test, you can force a small threshold.)

[mkuper]

Do you need someone to check this in for you?

lib/Transforms/Utils/LoopUnrollPeel.cpp

void llvm::computePeelCount(Loop *L, unsigned LoopSize,
   // Only try to peel innermost loops.
   if (!L->empty())
     return;
 
   // Try to find a Phi node that has the same loop invariant as an input from
   // its only back edge. If there is such Phi, peeling 1 iteration from the
   // loop is profitable, because starting from 2nd iteration we will have an
   // invariant instead of this Phi.
+  if (LoopSize <= UP.Threshold) {
     if (auto *BackEdge = L->getLoopLatch()) {
       BasicBlock *Header = L->getHeader();
       // Iterate over Phis to find one with invariant input on back edge.
       bool FoundCandidate = false;
       PHINode *Phi;
       for (auto BI = Header->begin(); isa<PHINode>(&*BI); ++BI) {
         Phi = cast<PHINode>(&*BI);
         Value *Input = Phi->getIncomingValueForBlock(BackEdge);
         if (L->isLoopInvariant(Input)) {
           FoundCandidate = true;
           break;
         }
       }
       if (FoundCandidate) {
         DEBUG(dbgs() << "Peel one iteration to get rid of " << *Phi
                      << " because starting from 2nd iteration it is always"
                      << " an invariant\n");
         UP.PeelCount = 1;
         return;
       }
     }
+  }
 
   // Bail if we know the statically calculated trip count.
   // In this case we rather prefer partial unrolling.
   if (TripCount)
     return;
 
   // If the user provided a peel count, use that.
   bool UserPeelCount = UnrollForcePeelCount.getNumOccurrences() > 0;