[LoopPeel] Allow to bypass profitability checks. NFC

[anna]

Summary

canPeel API checks legality as well as profitability of peeling the
loop. Separate this out since sometimes (for example downstream), we
just need to know if we can peel the loop.

Currently, since we have exactly one profitability check, it seems
unnecessary to have a separate API for this. So, we just decide based on
the passed in argument.
This is an NFC, since by default we always check profitability.

[nikic]

As the comment indicates, this is not entirely a profitability check: The current peeling code does not correctly update branch weights if this condition is not satisfied. D134803 implements the necessary support and removes this check entirely.

I also don't get how you plan to use this flag -- after all peelLoop() also contains a canPeel() assertion, so you wouldn't be able to call to call it anyway.

[anna]

In D135387#3840832, @nikic wrote:

As the comment indicates, this is not entirely a profitability check: The current peeling code does not correctly update branch weights if this condition is not satisfied. D134803 implements the necessary support and removes this check entirely.

Ah, I only saw the need for the profitability check. Note that we still needed the profitability check downstream (the comment just piggybacked on the fact that there was missing functionality) so removing it entirely in D134803 would break performance for us. We are using canPeel in more cases (java optimizations) than here in upstream code.

I also don't get how you plan to use this flag -- after all peelLoop() also contains a canPeel() assertion, so you wouldn't be able to call to call it anyway.

Yes, that's right. I didn't see the canPeel assertion. I just had a separate API downstream for profitability. I think it makes sense to introduce the profitability API as such upstream and separate out the legality from profitability.

[nikic]

I don't think there is a single "profitability" API for loop peeling. Loop peeling currently performs three different types of peeling: To render conditions invariant, to make loads dereferenceable and to peel based on PGO loop count. These have different profitability criterions. The current (in-tree) implementation already separately checks profitability for these cases (with the first case being essentially always profitable, and the other two using the deopt/unreachable exit check, plus additional profitability checks). I believe the check in canPeel() itself is only relevant for the branch weight issues at this point, profitability is handled separately on a case-by-case basis. If you have out-of-tree uses of loop peeling, you likely want to do the same there.

[anna]

agreed. abandoning the change in favour of D134803. We will add the profitability check downstream for the case we need. Thanks.

llvm/include/llvm/Transforms/Utils/LoopPeel.h

 
 #ifndef LLVM_TRANSFORMS_UTILS_LOOPPEEL_H
 #define LLVM_TRANSFORMS_UTILS_LOOPPEEL_H
 
 #include "llvm/Analysis/TargetTransformInfo.h"
 
 namespace llvm {
 
-bool canPeel(Loop *L);
+// We do the profitability checks for loop peeling under CheckPeelProfitability.
+bool canPeel(Loop *L, const bool CheckPeelProfitability = true);
 
 bool peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI, ScalarEvolution *SE,
               DominatorTree &DT, AssumptionCache *AC, bool PreserveLCSSA);
 
 TargetTransformInfo::PeelingPreferences
 gatherPeelingPreferences(Loop *L, ScalarEvolution &SE,
                          const TargetTransformInfo &TTI,
                          Optional<bool> UserAllowPeeling,

llvm/lib/Transforms/Utils/LoopPeel.cpp

 
 static cl::opt<unsigned> UnrollForcePeelCount(
     "unroll-force-peel-count", cl::init(0), cl::Hidden,
     cl::desc("Force a peel count regardless of profiling information."));
 
 static const char *PeeledCountMetaData = "llvm.loop.peeled.count";
 
 // Check whether we are capable of peeling this loop.
-bool llvm::canPeel(Loop *L) {
+bool llvm::canPeel(Loop *L, const bool CheckPeelProfitability) {
   // Make sure the loop is in simplified form
   if (!L->isLoopSimplifyForm())
     return false;
 
   // Don't try to peel loops where the latch is not the exiting block.
   // This can be an indication of two different things:
   // 1) The loop is not rotated.
   // 2) The loop contains irreducible control flow that involves the latch.
   const BasicBlock *Latch = L->getLoopLatch();
   if (!L->isLoopExiting(Latch))
     return false;
 
   // Peeling is only supported if the latch is a branch.
   if (!isa<BranchInst>(Latch->getTerminator()))
     return false;
 
   SmallVector<BasicBlock *, 4> Exits;
   L->getUniqueNonLatchExitBlocks(Exits);
+  // All legality checks are done.
+  if (!CheckPeelProfitability)
+    return true;
+
   // The latch must either be the only exiting block or all non-latch exit
   // blocks have either a deopt or unreachable terminator or compose a chain of
   // blocks where the last one is either deopt or unreachable terminated. Both
   // deopt and unreachable terminators are a strong indication they are not
   // taken. Note that this is a profitability check, not a legality check. Also
   // note that LoopPeeling currently can only update the branch weights of latch
   // blocks and branch weights to blocks with deopt or unreachable do not need
   // updating.