[UnJ] Add debug messages at to why loops are not unrolled. NFC

[dmgreen]

Summary

Adds some cleaned up debug messages from back when I was writing this.

[Meinersbur]

LGTM.

lib/Transforms/Utils/LoopUnrollAndJam.cpp

757–758 ↗

(On Diff #158232)

Could we name this in respect to what it returns? E.g. HasConsistentIterationCounts?

[xbolva00]

lib/Transforms/Utils/LoopUnrollAndJam.cpp

780 ↗

(On Diff #158232)

may throw

[dmgreen]

Thanks

lib/Transforms/Utils/LoopUnrollAndJam.cpp

757–758 ↗

(On Diff #158232)

There's another patch (D50063) which renames this again and pulls it out into a separate function. I'll rename it there.

780 ↗

(On Diff #158232)

Will do

llvm/trunk/lib/Transforms/Utils/LoopUnrollAndJam.cpp

llvm::UnrollAndJamLoop(Loop *L, unsigned Count, unsigned TripCount,
 
   // When we enter here we should have already checked that it is safe
   BasicBlock *Header = L->getHeader();
   assert(L->getSubLoops().size() == 1);
   Loop *SubLoop = *L->begin();
 
   // Don't enter the unroll code if there is nothing to do.
   if (TripCount == 0 && Count < 2) {
-    LLVM_DEBUG(dbgs() << "Won't unroll; almost nothing to do\n");
+    LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; almost nothing to do\n");
     return LoopUnrollResult::Unmodified;
   }
 
   assert(Count > 0);
   assert(TripMultiple > 0);
   assert(TripCount == 0 || TripCount % TripMultiple == 0);
 
   // Are we eliminating the loop control altogether?
for (Value *J : Later) {
       // Track dependencies, and if we find them take a conservative approach
       // by allowing only = or < (not >), altough some > would be safe
       // (depending upon unroll width).
       // For the inner loop, we need to disallow any (> <) dependencies
       // FIXME: Allow > so long as distance is less than unroll width
       if (auto D = DI.depends(Src, Dst, true)) {
         assert(D->isOrdered() && "Expected an output, flow or anti dep.");
 
-        if (D->isConfused())
+        if (D->isConfused()) {
+          LLVM_DEBUG(dbgs() << "  Confused dependency between:\n"
+                            << "  " << *Src << "\n"
+                            << "  " << *Dst << "\n");
           return false;
+        }
         if (!InnerLoop) {
-          if (D->getDirection(LoopDepth) & Dependence::DVEntry::GT)
+          if (D->getDirection(LoopDepth) & Dependence::DVEntry::GT) {
+            LLVM_DEBUG(dbgs() << "  > dependency between:\n"
+                              << "  " << *Src << "\n"
+                              << "  " << *Dst << "\n");
             return false;
+          }
         } else {
           assert(LoopDepth + 1 <= D->getLevels());
           if (D->getDirection(LoopDepth) & Dependence::DVEntry::GT &&
-              D->getDirection(LoopDepth + 1) & Dependence::DVEntry::LT)
+              D->getDirection(LoopDepth + 1) & Dependence::DVEntry::LT) {
+            LLVM_DEBUG(dbgs() << "  < > dependency between:\n"
+                              << "  " << *Src << "\n"
+                              << "  " << *Dst << "\n");
             return false;
           }
         }
       }
     }
+  }
   return true;
 }
 
 static bool checkDependencies(Loop *L, BasicBlockSet &ForeBlocks,
                               BasicBlockSet &SubLoopBlocks,
                               BasicBlockSet &AftBlocks, DependenceInfo &DI) {
   // Get all loads/store pairs for each blocks
   SmallVector<Value *, 4> ForeMemInstr;
bool llvm::isSafeToUnrollAndJam(Loop *L, ScalarEvolution &SE, DominatorTree &DT,
   BasicBlock *SubLoopLatch = SubLoop->getLoopLatch();
   BasicBlock *SubLoopExit = SubLoop->getExitingBlock();
 
   if (Latch != Exit)
     return false;
   if (SubLoopLatch != SubLoopExit)
     return false;
 
-  if (Header->hasAddressTaken() || SubLoopHeader->hasAddressTaken())
+  if (Header->hasAddressTaken() || SubLoopHeader->hasAddressTaken()) {
+    LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; Address taken\n");
     return false;
+  }
 
   // Split blocks into Fore/SubLoop/Aft based on dominators
   BasicBlockSet SubLoopBlocks;
   BasicBlockSet ForeBlocks;
   BasicBlockSet AftBlocks;
   if (!partitionOuterLoopBlocks(L, SubLoop, ForeBlocks, SubLoopBlocks,
-                                AftBlocks, &DT))
+                                AftBlocks, &DT)) {
+    LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; Incompatible loop layout\n");
     return false;
+  }
 
   // Aft blocks may need to move instructions to fore blocks, which becomes more
   // difficult if there are multiple (potentially conditionally executed)
   // blocks. For now we just exclude loops with multiple aft blocks.
-  if (AftBlocks.size() != 1)
+  if (AftBlocks.size() != 1) {
+    LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; Can't currently handle "
+                         "multiple blocks after the loop\n");
     return false;
+  }
 
-  // Check inner loop IV is consistent between all iterations
+  // Check inner loop backedge count is consistent on all iterations of the
+  // outer loop
+  auto CheckInnerLoopIterationCountInvariant = [](Loop *SubLoop, Loop *OuterL,
+                                                  ScalarEvolution &SE) {
+    BasicBlock *SubLoopLatch = SubLoop->getLoopLatch();
     const SCEV *SubLoopBECountSC = SE.getExitCount(SubLoop, SubLoopLatch);
     if (isa<SCEVCouldNotCompute>(SubLoopBECountSC) ||
         !SubLoopBECountSC->getType()->isIntegerTy())
       return false;
     ScalarEvolution::LoopDisposition LD =
-      SE.getLoopDisposition(SubLoopBECountSC, L);
+        SE.getLoopDisposition(SubLoopBECountSC, OuterL);
     if (LD != ScalarEvolution::LoopInvariant)
       return false;
+    return true;
+  };
+  if (!CheckInnerLoopIterationCountInvariant(SubLoop, L, SE)) {
+    LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; Inner loop iteration count is "
+                         "not consistent on each iteration\n");
+    return false;
+  }
 
   // Check the loop safety info for exceptions.
   LoopSafetyInfo LSI;
   computeLoopSafetyInfo(&LSI, L);
-  if (LSI.MayThrow)
+  if (LSI.MayThrow) {
+    LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; Something may throw\n");
     return false;
+  }
 
   // We've ruled out the easy stuff and now need to check that there are no
   // interdependencies which may prevent us from moving the:
   //  ForeBlocks before Subloop and AftBlocks.
   //  Subloop before AftBlocks.
   //  ForeBlock phi operands before the subloop
 
   // Make sure we can move all instructions we need to before the subloop
   if (!processHeaderPhiOperands(
           Header, Latch, AftBlocks, [&AftBlocks, &SubLoop](Instruction *I) {
             if (SubLoop->contains(I->getParent()))
               return false;
             if (AftBlocks.count(I->getParent())) {
               // If we hit a phi node in afts we know we are done (probably
               // LCSSA)
               if (isa<PHINode>(I))
                 return false;
               // Can't move instructions with side effects or memory
               // reads/writes
               if (I->mayHaveSideEffects() || I->mayReadOrWriteMemory())
                 return false;
             }
             // Keep going
             return true;
-          }))
+          })) {
+    LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; can't move required "
+                         "instructions after subloop to before it\n");
     return false;
+  }
 
   // Check for memory dependencies which prohibit the unrolling we are doing.
   // Because of the way we are unrolling Fore/Sub/Aft blocks, we need to check
   // there are no dependencies between Fore-Sub, Fore-Aft, Sub-Aft and Sub-Sub.
-  if (!checkDependencies(L, ForeBlocks, SubLoopBlocks, AftBlocks, DI))
+  if (!checkDependencies(L, ForeBlocks, SubLoopBlocks, AftBlocks, DI)) {
+    LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; failed dependency check\n");
     return false;
+  }
 
   return true;
 }