[LoopUnroll] Fix truncation bug in canUnrollCompletely.

[sanjoy]

Summary

canUnrollCompletely takes unsigned values for UnrolledCost and
RolledDynamicCost but is passed in uint64_ts that are silently
truncated. Because of this, when UnrolledSize is a large integer
that has a small remainder with UINT32_MAX, LLVM tries to completely
unroll loops with high trip counts.

[mzolotukhin]

LGTM.

I've seen that bug, and I believed that I've fixed it - but probably it got lost in reviews and did make it to trunk. Thank you for catching it!

Michael

[mzolotukhin]

In my previous comment: s/did make/did not make/

llvm/trunk/lib/Transforms/Scalar/LoopUnrollPass.cpp

void selectThresholds(const Loop *L, bool HasPragma,
         if (PartialThreshold != NoThreshold)
           PartialThreshold =
               std::max<unsigned>(PartialThreshold, PragmaUnrollThreshold);
       }
     }
     bool canUnrollCompletely(Loop *L, unsigned Threshold,
                              unsigned PercentDynamicCostSavedThreshold,
                              unsigned DynamicCostSavingsDiscount,
-                             unsigned UnrolledCost, unsigned RolledDynamicCost);
+                             uint64_t UnrolledCost, uint64_t RolledDynamicCost);
   };
 }
 
 char LoopUnroll::ID = 0;
 INITIALIZE_PASS_BEGIN(LoopUnroll, "loop-unroll", "Unroll loops", false, false)
 INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
 INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
static void SetLoopAlreadyUnrolled(Loop *L) {
   // Set operand 0 to refer to the loop id itself.
   NewLoopID->replaceOperandWith(0, NewLoopID);
   L->setLoopID(NewLoopID);
 }
 
 bool LoopUnroll::canUnrollCompletely(Loop *L, unsigned Threshold,
                                      unsigned PercentDynamicCostSavedThreshold,
                                      unsigned DynamicCostSavingsDiscount,
-                                     unsigned UnrolledCost,
-                                     unsigned RolledDynamicCost) {
+                                     uint64_t UnrolledCost,
+                                     uint64_t RolledDynamicCost) {
 
   if (Threshold == NoThreshold) {
     DEBUG(dbgs() << "  Can fully unroll, because no threshold is set.\n");
     return true;
   }
 
   if (UnrolledCost <= Threshold) {
     DEBUG(dbgs() << "  Can fully unroll, because unrolled cost: "

llvm/trunk/test/Transforms/LoopUnroll/full-unroll-bad-cost.ll

+; RUN: opt -S -loop-unroll < %s | FileCheck %s
+
+; LLVM should not try to fully unroll this loop.
+
+declare void @f()
+declare void @g()
+declare void @h()
+
+define void @trivial_loop() {
+; CHECK-LABEL: @trivial_loop(
+ entry:
+  br label %loop
+
+ loop:
+  %idx = phi i32 [ 0, %entry ], [ %idx.inc, %loop ]
+  %idx.inc = add i32 %idx, 1
+  call void @f()
+  call void @g()
+  call void @h()
+  call void @f()
+  call void @g()
+  call void @h()
+  call void @f()
+  call void @g()
+  call void @h()
+  call void @f()
+  call void @g()
+  call void @h()
+  call void @f()
+  call void @g()
+  call void @h()
+  %be = icmp slt i32 %idx, 268435456
+  br i1 %be, label %loop, label %exit
+
+; CHECK: loop:
+; CHECK-NEXT:  %idx = phi i32 [ 0, %entry ], [ %idx.inc, %loop ]
+; CHECK-NEXT:  %idx.inc = add i32 %idx, 1
+; CHECK-NEXT:  call void @f()
+; CHECK-NEXT:  call void @g()
+; CHECK-NEXT:  call void @h()
+; CHECK-NEXT:  call void @f()
+; CHECK-NEXT:  call void @g()
+; CHECK-NEXT:  call void @h()
+; CHECK-NEXT:  call void @f()
+; CHECK-NEXT:  call void @g()
+; CHECK-NEXT:  call void @h()
+; CHECK-NEXT:  call void @f()
+; CHECK-NEXT:  call void @g()
+; CHECK-NEXT:  call void @h()
+; CHECK-NEXT:  call void @f()
+; CHECK-NEXT:  call void @g()
+; CHECK-NEXT:  call void @h()
+; CHECK-NEXT:  %be = icmp slt i32 %idx, 268435456
+; CHECK-NEXT:  br i1 %be, label %loop, label %exit
+
+ exit:
+  ret void
+}