[LoopUnroll] Double thresholds instead of increasing to pragma-unroll-threshold for llvm.loop.unroll.enable

[MaskRay]

Summary

Fix PR51084: #pragma unroll aggressively increases UP.Threshold from (150
for -O2 and 300 for -O3) to 16384. This is usually undesired. Just double the
thresholds.

[lebedev.ri]

I'm not really sold on this. I suspect the fix should either be in LSR, or perhaps it's working as intended.
I guess the main problem is - is there proper documentation for #pragma clang loop unroll(enable)?

[reames]

I agree with Roman, this does not seem obvious. The bug seems to be either a) an issue with LSR, or b) invalid (e.g. we obeyed a user directive which was a poor choice - but not our choice).

[MaskRay]

I agree. I wasn't aware of the semantics of unroll pragmas.

LangRef: "This metadata suggests that the loop should be fully unrolled if the trip count is known at compile time and partially unrolled if the trip count is not known at compile time."

https://clang.llvm.org/docs/LanguageExtensions.html#loop-unrolling : "If unroll(enable) is specified the unroller will attempt to fully unroll the loop if the trip count is known at compile time."

"If unroll(full) is specified the unroller will attempt to fully unroll the loop if the trip count is known at compile time identically to unroll(enable)."

Full unrolling does what the user asks, even though such a large trip count may look silly.

llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp

bool llvm::computeUnrollCount(
   }
 
   bool PragmaEnableUnroll = hasUnrollEnablePragma(L);
   bool ExplicitUnroll = PragmaCount > 0 || PragmaFullUnroll ||
                         PragmaEnableUnroll || UserUnrollCount;
 
   if (ExplicitUnroll && TripCount != 0) {
     // If the loop has an unrolling pragma, we want to be more aggressive with
-    // unrolling limits. Set thresholds to at least the PragmaUnrollThreshold
-    // value which is larger than the default limits.
-    UP.Threshold = std::max<unsigned>(UP.Threshold, PragmaUnrollThreshold);
+    // unrolling limits. Double thresholds.
+    UP.Threshold = std::min<unsigned>(UP.Threshold * 2, PragmaUnrollThreshold);
     UP.PartialThreshold =
-        std::max<unsigned>(UP.PartialThreshold, PragmaUnrollThreshold);
+        std::min<unsigned>(UP.PartialThreshold * 2, PragmaUnrollThreshold);
   }
 
   // 3rd priority is full unroll count.
   // Full unroll makes sense only when TripCount or its upper bound could be
   // statically calculated.
   // Also we need to check if we exceed FullUnrollMaxCount.
 
   // We can unroll by the upper bound amount if it's generally allowed or if

llvm/test/Transforms/LoopUnroll/unroll-pragmas.ll

 for.end:                                          ; preds = %for.body
   ret void
 }
 !12 = !{!12, !4}
 
 ; #pragma clang loop unroll(enable)
 ; Loop should be fully unrolled.
 ;
-; CHECK-LABEL: @loop64_with_enable(
+; CHECK-LABEL: @loop32_with_enable(
 ; CHECK-NOT: br i1
-define void @loop64_with_enable(i32* nocapture %a) {
+define void @loop32_with_enable(i32* nocapture %a) {
 entry:
   br label %for.body
 
 for.body:                                         ; preds = %for.body, %entry
   %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
   %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv
   %0 = load i32, i32* %arrayidx, align 4
   %inc = add nsw i32 %0, 1
   store i32 %inc, i32* %arrayidx, align 4
   %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
-  %exitcond = icmp eq i64 %indvars.iv.next, 64
+  %exitcond = icmp eq i64 %indvars.iv.next, 32
   br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !13
 
 for.end:                                          ; preds = %for.body
   ret void
 }
 !13 = !{!13, !14}
 !14 = !{!"llvm.loop.unroll.enable"}
 
 ; #pragma clang loop unroll(enable)
+; Partially unrolled with a count of 32.
+;
+; CHECK-LABEL: @loop64_with_enable(
+; CHECK:     %inc.31 = add nsw i32 %31, 1
+; CHECK-NOT: %inc.32
+define void @loop64_with_enable(i32* nocapture %a) {
+entry:
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %entry
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv
+  %0 = load i32, i32* %arrayidx, align 4
+  %inc = add nsw i32 %0, 1
+  store i32 %inc, i32* %arrayidx, align 4
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond = icmp eq i64 %indvars.iv.next, 64
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !15
+
+for.end:                                          ; preds = %for.body
+  ret void
+}
+!15 = !{!15, !16}
+!16 = !{!"llvm.loop.unroll.enable"}
+
+; #pragma clang loop unroll(enable)
 ; Loop has a runtime trip count and should be runtime unrolled and duplicated
 ; (original and 8x) if remainder is allowed, otherwise it should not be
 ; unrolled.
 ;
 ; CHECK-LABEL: @runtime_loop_with_enable(
 ; CHECK: for.body:
 ; CHECK: store i32
 ; REM: store i32
for.body:                                         ; preds = %entry, %for.body
   %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %entry ]
   %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv
   %0 = load i32, i32* %arrayidx, align 4
   %inc = add nsw i32 %0, 1
   store i32 %inc, i32* %arrayidx, align 4
   %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
   %lftr.wideiv = trunc i64 %indvars.iv.next to i32
   %exitcond = icmp eq i32 %lftr.wideiv, %b
-  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !15
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !17
 
 for.end:                                          ; preds = %for.body, %entry
   ret void
 }
-!15 = !{!15, !14}
+!17 = !{!17, !16}
 
 ; #pragma clang loop unroll_count(3)
 ; Loop has a runtime trip count.  Runtime unrolling should occur and loop
 ; should be duplicated (original and 3x unrolled) if remainder is allowed,
 ; otherwise it should not be unrolled.
 ;
 ; CHECK-LABEL: @runtime_loop_with_count3(
 ; CHECK: for.body
 ; CHECK: store
 ; REM: store
 ; REM: store
 ; CHECK-NOT: store
 ; CHECK: br i1
 ; REM: for.body.epil:
 ; REM: store
 ; NOREM-NOT: for.body.epil:
 ; NOREM-NOT: store
 ; CHECK-NOT: store
 ; REM: br i1
 define void @runtime_loop_with_count3(i32* nocapture %a, i32 %b) {
 entry:
   %cmp3 = icmp sgt i32 %b, 0
-  br i1 %cmp3, label %for.body, label %for.end, !llvm.loop !16
+  br i1 %cmp3, label %for.body, label %for.end, !llvm.loop !18
 
 for.body:                                         ; preds = %entry, %for.body
   %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %entry ]
   %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv
   %0 = load i32, i32* %arrayidx, align 4
   %inc = add nsw i32 %0, 1
   store i32 %inc, i32* %arrayidx, align 4
   %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
   %lftr.wideiv = trunc i64 %indvars.iv.next to i32
   %exitcond = icmp eq i32 %lftr.wideiv, %b
-  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !16
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !18
 
 for.end:                                          ; preds = %for.body, %entry
   ret void
 }
-!16 = !{!16, !17}
-!17 = !{!"llvm.loop.unroll.count", i32 3}
+!18 = !{!18, !19}
+!19 = !{!"llvm.loop.unroll.count", i32 3}