[LoopUnroll] Clean up remarks for unroll remainder

[dmgreen]

Summary

The optimisation remarks for loop unrolling with an unrolled remainder looks something like:

test.c:7:18: remark: completely unrolled loop with 3 iterations [-Rpass=loop-unroll]
            C[i] += A[i*N+j];
                 ^
test.c:6:9: remark: unrolled loop by a factor of 4 with run-time trip count [-Rpass=loop-unroll]
        for(int j = 0; j < N; j++)
        ^

This removes the first of the two messages.

[fhahn]

lib/Transforms/Utils/LoopUnroll.cpp

504 ↗

(On Diff #118371)

Do I understand correctly that we want to only display this remark in case the following condition is true, so if we successfully unrolled the loop with a run-time trip count (condition at line 428)?

RuntimeTripCount && TripMultiple % Count != 0 && 
     UnrollRuntimeLoopRemainder(....)

I am probably missing something, but it seems like this code path does not consider the TripMultiple % Count != 0 from the condition.

[dmgreen]

Hello again. Thanks for taking a look.

lib/Transforms/Utils/LoopUnroll.cpp

504 ↗

(On Diff #118371)

Currently with -unroll-runtime=true -unroll-remainder (like we have for cortex-m cores), the remainder loop is unrolled. It does this by calling UnrollLoop() from inside UnrollRuntimeLoopRemainder(). When that happens we get two remarks, one from the "outer" UnrollLoop for the original loop, one from the inner UnrollLoop() for the remainder loop. I'm trying to remove that second one for the remainder loop.

Clang-format moved things around a bit here, but I've just made it so that the ORE is optional, and not passed through to the inner UnrollRuntimeLoopRemainder/UnrollLoop.

[fhahn]

LGTM, please consider moving the test.

lib/Transforms/Utils/LoopUnroll.cpp

504 ↗

(On Diff #118371)

Ah yes. Not sure what I was looking at when I left the comment.

test/Transforms/LoopUnroll/runtime-unroll-remainder.ll

2 ↗

(On Diff #118371)

I think it would make sense to move the test to test/Transforms/LoopUnroll/loop-remarks.ll?

[dmgreen]

Rebase and moved test over to loop-remarks.ll

[fhahn]

Thanks Dave, I think looks good to commit now.

llvm/trunk/include/llvm/Transforms/Utils/UnrollLoop.h

LoopUnrollResult UnrollLoop(Loop *L, unsigned Count, unsigned TripCount,
                             OptimizationRemarkEmitter *ORE, bool PreserveLCSSA);
 
 bool UnrollRuntimeLoopRemainder(Loop *L, unsigned Count,
                                 bool AllowExpensiveTripCount,
                                 bool UseEpilogRemainder, bool UnrollRemainder,
                                 LoopInfo *LI,
                                 ScalarEvolution *SE, DominatorTree *DT,
                                 AssumptionCache *AC,
-                                OptimizationRemarkEmitter *ORE,
                                 bool PreserveLCSSA);
 
 void computePeelCount(Loop *L, unsigned LoopSize,
                       TargetTransformInfo::UnrollingPreferences &UP,
                       unsigned &TripCount);
 
 bool peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI, ScalarEvolution *SE,
               DominatorTree *DT, AssumptionCache *AC, bool PreserveLCSSA);
 
 MDNode *GetUnrollMetadata(MDNode *LoopID, StringRef Name);
 
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_UTILS_UNROLLLOOP_H

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

DEBUG(
       });
 
   bool EpilogProfitability =
       UnrollRuntimeEpilog.getNumOccurrences() ? UnrollRuntimeEpilog
                                               : isEpilogProfitable(L);
 
   if (RuntimeTripCount && TripMultiple % Count != 0 &&
       !UnrollRuntimeLoopRemainder(L, Count, AllowExpensiveTripCount,
-                                  EpilogProfitability, UnrollRemainder,
-                                  LI, SE, DT, AC, ORE,
-                                  PreserveLCSSA)) {
+                                  EpilogProfitability, UnrollRemainder, LI, SE,
+                                  DT, AC, PreserveLCSSA)) {
     if (Force)
       RuntimeTripCount = false;
     else {
       DEBUG(
           dbgs() << "Wont unroll; remainder loop could not be generated"
                     "when assuming runtime trip count\n");
       return LoopUnrollResult::Unmodified;
     }
BreakoutTrip = TripMultiple =
       (unsigned)GreatestCommonDivisor64(Count, TripMultiple);
   }
 
   using namespace ore;
   // Report the unrolling decision.
   if (CompletelyUnroll) {
     DEBUG(dbgs() << "COMPLETELY UNROLLING loop %" << Header->getName()
                  << " with trip count " << TripCount << "!\n");
+    if (ORE)
       ORE->emit([&]() {
         return OptimizationRemark(DEBUG_TYPE, "FullyUnrolled", L->getStartLoc(),
                                   L->getHeader())
-             << "completely unrolled loop with " << NV("UnrollCount", TripCount)
-             << " iterations";
+               << "completely unrolled loop with "
+               << NV("UnrollCount", TripCount) << " iterations";
       });
   } else if (PeelCount) {
     DEBUG(dbgs() << "PEELING loop %" << Header->getName()
                  << " with iteration count " << PeelCount << "!\n");
+    if (ORE)
       ORE->emit([&]() {
         return OptimizationRemark(DEBUG_TYPE, "Peeled", L->getStartLoc(),
                                   L->getHeader())
                << " peeled loop by " << NV("PeelCount", PeelCount)
                << " iterations";
       });
   } else {
     auto DiagBuilder = [&]() {
       OptimizationRemark Diag(DEBUG_TYPE, "PartialUnrolled", L->getStartLoc(),
                               L->getHeader());
       return Diag << "unrolled loop by a factor of "
                   << NV("UnrollCount", Count);
     };
 
     DEBUG(dbgs() << "UNROLLING loop %" << Header->getName()
           << " by " << Count);
     if (TripMultiple == 0 || BreakoutTrip != TripMultiple) {
       DEBUG(dbgs() << " with a breakout at trip " << BreakoutTrip);
+      if (ORE)
         ORE->emit([&]() {
           return DiagBuilder() << " with a breakout at trip "
                                << NV("BreakoutTrip", BreakoutTrip);
         });
     } else if (TripMultiple != 1) {
       DEBUG(dbgs() << " with " << TripMultiple << " trips per branch");
+      if (ORE)
         ORE->emit([&]() {
           return DiagBuilder() << " with " << NV("TripMultiple", TripMultiple)
                                << " trips per branch";
         });
     } else if (RuntimeTripCount) {
       DEBUG(dbgs() << " with run-time trip count");
-      ORE->emit([&]() { return DiagBuilder() << " with run-time trip count"; });
+      if (ORE)
+        ORE->emit(
+            [&]() { return DiagBuilder() << " with run-time trip count"; });
     }
     DEBUG(dbgs() << "!\n");
   }
 
   bool ContinueOnTrue = L->contains(BI->getSuccessor(0));
   BasicBlock *LoopExit = BI->getSuccessor(ContinueOnTrue);
 
   // For the first iteration of the loop, we should use the precloned values for

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

 /// EpilExit:
 
 bool llvm::UnrollRuntimeLoopRemainder(Loop *L, unsigned Count,
                                       bool AllowExpensiveTripCount,
                                       bool UseEpilogRemainder,
                                       bool UnrollRemainder,
                                       LoopInfo *LI, ScalarEvolution *SE,
                                       DominatorTree *DT, AssumptionCache *AC,
-                                      OptimizationRemarkEmitter *ORE,
                                       bool PreserveLCSSA) {
   DEBUG(dbgs() << "Trying runtime unrolling on Loop: \n");
   DEBUG(L->dump());
   DEBUG(UseEpilogRemainder ? dbgs() << "Using epilog remainder.\n" :
         dbgs() << "Using prolog remainder.\n");
 
   // Make sure the loop is in canonical form.
   if (!L->isLoopSimplifyForm()) {
if (OtherExits.size() > 0) {
     // Generate dedicated exit blocks for the remainder loop if one exists, to
     // preserve LoopSimplifyForm.
     if (remainderLoop)
       formDedicatedExitBlocks(remainderLoop, DT, LI, PreserveLCSSA);
   }
 
   if (remainderLoop && UnrollRemainder) {
     DEBUG(dbgs() << "Unrolling remainder loop\n");
-    UnrollLoop(remainderLoop, /*Count*/Count - 1, /*TripCount*/Count - 1,
-               /*Force*/false, /*AllowRuntime*/false,
-               /*AllowExpensiveTripCount*/false, /*PreserveCondBr*/true,
-               /*PreserveOnlyFirst*/false, /*TripMultiple*/1,
-               /*PeelCount*/0, /*UnrollRemainder*/false, LI, SE, DT, AC, ORE,
-               PreserveLCSSA);
+    UnrollLoop(remainderLoop, /*Count*/ Count - 1, /*TripCount*/ Count - 1,
+               /*Force*/ false, /*AllowRuntime*/ false,
+               /*AllowExpensiveTripCount*/ false, /*PreserveCondBr*/ true,
+               /*PreserveOnlyFirst*/ false, /*TripMultiple*/ 1,
+               /*PeelCount*/ 0, /*UnrollRemainder*/ false, LI, SE, DT, AC,
+               /*ORE*/ nullptr, PreserveLCSSA);
   }
 
   NumRuntimeUnrolled++;
   return true;
 }

llvm/trunk/test/Transforms/LoopUnroll/loop-remarks.ll

 ; RUN: opt < %s -S -loop-unroll -pass-remarks=loop-unroll -unroll-count=16 2>&1 | FileCheck -check-prefix=COMPLETE-UNROLL %s
 ; RUN: opt < %s -S -loop-unroll -pass-remarks=loop-unroll -unroll-count=4 2>&1 | FileCheck -check-prefix=PARTIAL-UNROLL %s
+; RUN: opt < %s -S -loop-unroll -pass-remarks=loop-unroll -unroll-count=4 -unroll-runtime=true -unroll-remainder 2>&1 | FileCheck %s --check-prefix=RUNTIME-UNROLL
 
 ; COMPLETE-UNROLL: remark: {{.*}}: completely unrolled loop with 16 iterations
 ; PARTIAL-UNROLL: remark: {{.*}}: unrolled loop by a factor of 4
+; RUNTIME-UNROLL: remark: {{.*}}: unrolled loop by a factor of 4
 
 define i32 @sum() {
 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 ]
   %add = add nsw i32 %i.05, 4
   %call = tail call i32 @baz(i32 %add) #2
   %add1 = add nsw i32 %call, %s.06
   %inc = add nsw i32 %i.05, 1
   %exitcond = icmp eq i32 %inc, 16
   br i1 %exitcond, label %for.end, label %for.body
 
 for.end:                                          ; preds = %for.body
   ret i32 %add1
 }
 
-declare i32 @baz(i32)
+; RUNTIME-UNROLL-NOT: remark: {{.*}}: completely unrolled loop with 3 iterations
+; RUNTIME-UNROLL: remark: {{.*}}: unrolled loop by a factor of 4
+
+define i32 @runtime(i32 %n) {
+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 ]
+  %add = add nsw i32 %i.05, 4
+  %call = tail call i32 @baz(i32 %add) #2
+  %add1 = add nsw i32 %call, %s.06
+  %inc = add nsw i32 %i.05, 1
+  %exitcond = icmp eq i32 %inc, %n
+  br i1 %exitcond, label %for.end, label %for.body
+
+for.end:                                          ; preds = %for.body
+  ret i32 %add1
+}
+
+declare i32 @baz(i32)
 No newline at end of file