[LoopUnroll] Simplify optimization remarks

[nikic]

Summary

This reduces the optimization remarks for unrolling to only include the unroll count, and whether it is a complete unroll, partial unroll or runtime unroll. The information about ULO.TripMultiple and BreakoutTrip is removed, as it is not compatible with multiple exits. The alternative would be to instead add information about all exits to the optimization remarks, but this doesn't seem particularly useful to me.

[reames]

I think I would prefer to see the information for each exit as opposed to removing it entirely. When understanding why unrolling happened in a particular way, having the detailed information is often very helpful. Not a strong preference, I'm willing to LGTM this if you really object.

[nikic]

In D104482#2827398, @reames wrote:

I think I would prefer to see the information for each exit as opposed to removing it entirely. When understanding why unrolling happened in a particular way, having the detailed information is often very helpful. Not a strong preference, I'm willing to LGTM this if you really object.

I'm not particularly familiar with how optimization remarks work -- if we wanted to go the other way around, we need to be able to identify specific exits in some way. For debug output, we can just print the block label, but I assume that wouldn't be suitable for optimization remarks, as the block label wouldn't really be meaningful in that context, it's an IR detail. Do you know how one would go about that?

[reames]

In D104482#2827677, @nikic wrote:

In D104482#2827398, @reames wrote:

I think I would prefer to see the information for each exit as opposed to removing it entirely. When understanding why unrolling happened in a particular way, having the detailed information is often very helpful. Not a strong preference, I'm willing to LGTM this if you really object.

I'm not particularly familiar with how optimization remarks work -- if we wanted to go the other way around, we need to be able to identify specific exits in some way. For debug output, we can just print the block label, but I assume that wouldn't be suitable for optimization remarks, as the block label wouldn't really be meaningful in that context, it's an IR detail. Do you know how one would go about that?

Hm, good point I was thinking in terms of debug output, and hadn't recognized the ORE complication.

Let's give it a day or so for someone more ORE familiar to chime in. If no one responds, you can consider this an LGTM of the current patch.

[nikic]

What do you think about this variant? It prints information for all exits as debug output, but keeps optimization remarks basic.

[reames]

LGTM

llvm/lib/Transforms/Utils/LoopUnroll.cpp

LoopUnrollResult llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
 
   if (L->getHeader()->hasAddressTaken()) {
     // The loop-rotate pass can be helpful to avoid this in many cases.
     LLVM_DEBUG(
         dbgs() << "  Won't unroll loop: address of header block is taken.\n");
     return LoopUnrollResult::Unmodified;
   }
 
-  if (ULO.TripCount != 0)
-    LLVM_DEBUG(dbgs() << "  Trip Count = " << ULO.TripCount << "\n");
-  if (ULO.TripMultiple != 1)
-    LLVM_DEBUG(dbgs() << "  Trip Multiple = " << ULO.TripMultiple << "\n");
-
   // Don't enter the unroll code if there is nothing to do.
   if (ULO.TripCount == 0 && ULO.Count < 2) {
     LLVM_DEBUG(dbgs() << "Won't unroll; almost nothing to do\n");
     return LoopUnrollResult::Unmodified;
   }
 
   assert(ULO.Count > 0);
   assert(ULO.TripMultiple > 0);
if (Info.TripCount != 0) {
       Info.BreakoutTrip = Info.TripCount % ULO.Count;
       Info.TripMultiple = 0;
     } else {
       Info.BreakoutTrip = Info.TripMultiple =
           (unsigned)GreatestCommonDivisor64(ULO.Count, Info.TripMultiple);
     }
     Info.ExitOnTrue = !L->contains(BI->getSuccessor(0));
     Info.ExitingBlocks.push_back(ExitingBlock);
+    LLVM_DEBUG(dbgs() << "  Exiting block %" << ExitingBlock->getName()
+                      << ": TripCount=" << Info.TripCount
+                      << ", TripMultiple=" << Info.TripMultiple
+                      << ", BreakoutTrip=" << Info.BreakoutTrip << "\n");
   }
 
   // Are we eliminating the loop control altogether?  Note that we can know
   // we're eliminating the backedge without knowing exactly which iteration
   // of the unrolled body exits.
   const bool CompletelyUnroll = ULO.Count == MaxTripCount;
 
   const bool PreserveOnlyFirst = CompletelyUnroll && MaxOrZero;
 
   // We assume a run-time trip count if the compiler cannot
   // figure out the loop trip count and the unroll-runtime
   // flag is specified.
-  bool RuntimeTripCount =
-      !CompletelyUnroll && ULO.TripCount == 0 && ULO.AllowRuntime;
+  bool RuntimeTripCount = !CompletelyUnroll && ULO.TripCount == 0 &&
+                          ULO.TripMultiple % ULO.Count != 0 && ULO.AllowRuntime;
 
   // Go through all exits of L and see if there are any phi-nodes there. We just
   // conservatively assume that they're inserted to preserve LCSSA form, which
   // means that complete unrolling might break this form. We need to either fix
   // it in-place after the transformation, or entirely rebuild LCSSA. TODO: For
   // now we just recompute LCSSA for the outer loop, but it should be possible
   // to fix it in-place.
   bool NeedToFixLCSSA =
LLVM_DEBUG(
                "Unroll count must divide trip multiple if loop contains a "
                "convergent operation.");
       });
 
   bool EpilogProfitability =
       UnrollRuntimeEpilog.getNumOccurrences() ? UnrollRuntimeEpilog
                                               : isEpilogProfitable(L);
 
-  if (RuntimeTripCount && ULO.TripMultiple % ULO.Count != 0 &&
+  if (RuntimeTripCount &&
       !UnrollRuntimeLoopRemainder(L, ULO.Count, ULO.AllowExpensiveTripCount,
                                   EpilogProfitability, ULO.UnrollRemainder,
                                   ULO.ForgetAllSCEV, LI, SE, DT, AC, TTI,
                                   PreserveLCSSA, RemainderLoop)) {
     if (ULO.Force)
       RuntimeTripCount = false;
     else {
       LLVM_DEBUG(dbgs() << "Won't unroll; remainder loop could not be "
                            "generated when assuming runtime trip count\n");
       return LoopUnrollResult::Unmodified;
     }
   }
 
-  // If we know the trip count, we know the multiple...
-  // TODO: This is only used for the ORE code, remove it.
-  unsigned BreakoutTrip = 0;
-  if (ULO.TripCount != 0) {
-    BreakoutTrip = ULO.TripCount % ULO.Count;
-    ULO.TripMultiple = 0;
-  } else {
-    // Figure out what multiple to use.
-    BreakoutTrip = ULO.TripMultiple =
-        (unsigned)GreatestCommonDivisor64(ULO.Count, ULO.TripMultiple);
-  }
-
   using namespace ore;
   // Report the unrolling decision.
   if (CompletelyUnroll) {
     LLVM_DEBUG(dbgs() << "COMPLETELY UNROLLING loop %" << Header->getName()
-                      << " with trip count " << ULO.TripCount << "!\n");
+                      << " with trip count " << ULO.Count << "!\n");
     if (ORE)
       ORE->emit([&]() {
         return OptimizationRemark(DEBUG_TYPE, "FullyUnrolled", L->getStartLoc(),
                                   L->getHeader())
                << "completely unrolled loop with "
-               << NV("UnrollCount", ULO.TripCount) << " iterations";
+               << NV("UnrollCount", ULO.Count) << " iterations";
       });
   } else {
-    auto DiagBuilder = [&]() {
-      OptimizationRemark Diag(DEBUG_TYPE, "PartialUnrolled", L->getStartLoc(),
-                              L->getHeader());
-      return Diag << "unrolled loop by a factor of "
-                  << NV("UnrollCount", ULO.Count);
-    };
-
     LLVM_DEBUG(dbgs() << "UNROLLING loop %" << Header->getName() << " by "
                       << ULO.Count);
-    if (ULO.TripMultiple == 0 || BreakoutTrip != ULO.TripMultiple) {
-      LLVM_DEBUG(dbgs() << " with a breakout at trip " << BreakoutTrip);
-      if (ORE)
-        ORE->emit([&]() {
-          return DiagBuilder() << " with a breakout at trip "
-                               << NV("BreakoutTrip", BreakoutTrip);
-        });
-    } else if (ULO.TripMultiple != 1) {
-      LLVM_DEBUG(dbgs() << " with " << ULO.TripMultiple << " trips per branch");
-      if (ORE)
-        ORE->emit([&]() {
-          return DiagBuilder()
-                 << " with " << NV("TripMultiple", ULO.TripMultiple)
-                 << " trips per branch";
-        });
-    } else if (RuntimeTripCount) {
-      LLVM_DEBUG(dbgs() << " with run-time trip count");
-      if (ORE)
-        ORE->emit(
-            [&]() { return DiagBuilder() << " with run-time trip count"; });
-    }
-    LLVM_DEBUG(dbgs() << "!\n");
+    if (RuntimeTripCount)
+      LLVM_DEBUG(dbgs() << " with run-time trip count");
+    LLVM_DEBUG(dbgs() << "!\n");
+
+        OptimizationRemark Diag(DEBUG_TYPE, "PartialUnrolled", L->getStartLoc(),
+                                L->getHeader());
+        Diag << "unrolled loop by a factor of " << NV("UnrollCount", ULO.Count);
+        if (RuntimeTripCount)
+          Diag << " with run-time trip count";
+        return Diag;
   }
 
   // We are going to make changes to this loop. SCEV may be keeping cached info
   // about it, in particular about backedge taken count. The changes we make
   // are guaranteed to invalidate this information for our loop. It is tempting
   // to only invalidate the loop being unrolled, but it is incorrect as long as
   // all exiting branches from all inner loops have impact on the outer loops,
   // and if something changes inside them then any of outer loops may also

llvm/test/CodeGen/AArch64/loop-micro-op-buffer-size-t99.ll

 ; REQUIRES: asserts
 ; RUN: opt -mcpu=thunderx2t99 -passes=loop-unroll --debug-only=loop-unroll --debug-only=basicblock-utils -S -unroll-allow-partial < %s 2>&1 | FileCheck %s
 
 target triple = "aarch64-unknown-linux-gnu"
 
 ; CHECK: Loop Unroll: F[foo] Loop %loop.header
 ; CHECK:   Loop Size = 18
-; CHECK:   Trip Count = 512
-; CHECK:   Trip Multiple = 512
-; CHECK: UNROLLING loop %loop.header by 4 with a breakout at trip 0
+; CHECK:   Exiting block %loop.inc: TripCount=512, TripMultiple=0, BreakoutTrip=0
+; CHECK: UNROLLING loop %loop.header by 4
 ; CHECK: Merging:
 ; CHECK: Loop Unroll: F[foo] Loop %loop.2.header
-; CHECK: Loop Size = 19
-; CHECK: Trip Count = 512
-; CHECK: Trip Multiple = 512
-; CHECK: UNROLLING loop %loop.2.header by 4 with a breakout at trip 0
+; CHECK:   Loop Size = 19
+; CHECK:   Exiting block %loop.2.inc: TripCount=512, TripMultiple=0, BreakoutTrip=0
+; CHECK: UNROLLING loop %loop.2.header by 4
 ; CHECK: Merging:
 ; CHECK: %counter = phi i32 [ 0, %entry ], [ %inc.3, %loop.inc.3 ]
 ; CHECK: %val = add nuw nsw i32 %counter, 5
 ; CHECK: %val1 = add nuw nsw i32 %counter, 6
 ; CHECK: %val2 = add nuw nsw i32 %counter, 7
 ; CHECK: %val3 = add nuw nsw i32 %counter, 8
 ; CHECK: %val4 = add nuw nsw i32 %counter, 9
 ; CHECK: %val5 = add nuw nsw i32 %counter, 10

llvm/test/Transforms/LoopUnroll/loop-remarks-with-hotness.ll

 ; RUN: opt < %s -S -loop-unroll -pass-remarks=loop-unroll -pass-remarks-with-hotness -unroll-count=16 2>&1 | FileCheck -check-prefix=COMPLETE-UNROLL %s
 ; RUN: opt < %s -S -loop-unroll -pass-remarks=loop-unroll -pass-remarks-with-hotness -unroll-count=4 2>&1 | FileCheck -check-prefix=PARTIAL-UNROLL %s
 
 ; COMPLETE-UNROLL: remark: {{.*}}: completely unrolled loop with 16 iterations (hotness: 300)
-; PARTIAL-UNROLL: remark: {{.*}}: unrolled loop by a factor of 4 {{.*}} (hotness: 300)
+; PARTIAL-UNROLL: remark: {{.*}}: unrolled loop by a factor of 4 (hotness: 300)
 
 define i32 @sum() !prof !0 {
 entry:
   br label %for.body
 
 for.body:                                         ; preds = %for.body, %entry
   %s.06 = phi i32 [ 0, %entry ], [ %add1, %for.body ]
   %i.05 = phi i32 [ 0, %entry ], [ %inc, %for.body ]