Do not add unroll disable metadata after unrolling pass for loops with #pragma clang loop unroll(full)

[meheff]

Summary

Currently all loop unrolling hint metadata is removed after a loop is unrolled and this metadata is replaced with metadata which disables further unrolling (llvm.loop.unroll.disable). This makes sense for the hint which specify loop unroll factor, that is "llvm.loop.unroll.count N" which would be added for "#pragma clang loop unroll_count(N)"). This prevents unrolling beyond what the pragma specified. However, disabling further unrolling doesn't make sense for full unrolling metadata in which we want to keep trying to unroll the loop in subsequent unrolling passes. This patch fixes this oversight by leaving full unrolling metadata in place after unrolling.

Mark

[jingyue]

LGTM with some minor comments. Thanks!

lib/Transforms/Scalar/LoopUnrollPass.cpp

460

I'd say PragmaCount != 0 because it's an integer.

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

41

To be clear, can we explain why this loop cannot be unrolled?

56

Would it be better if we make the trip count a variable, so that the loop can't be unrolled regardless of the threshold? I think that would make the test more resilience.

[meheff]

Thanks! Committed with r213789.

lib/Transforms/Scalar/LoopUnrollPass.cpp

460

Done.

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

41

Done.

56

Done. Trip count is now dynamic.

lib/Transforms/Scalar/LoopUnrollPass.cpp

while (Count != 0 && UnrolledSize > PartialThreshold) {
       UnrolledSize = LoopSize * Count;
     }
     if (Count > UP.MaxCount)
       Count = UP.MaxCount;
     DEBUG(dbgs() << "  partially unrolling with count: " << Count << "\n");
   }
 
   if (HasPragma) {
-    // Mark loop as unrolled to prevent unrolling beyond that
-    // requested by the pragma.
+    if (PragmaCount)
+      // If loop has an unroll count pragma mark loop as unrolled to prevent
+      // unrolling beyond that requested by the pragma.
       SetLoopAlreadyUnrolled(L);
 
     // Emit optimization remarks if we are unable to unroll the loop
     // as directed by a pragma.
     DebugLoc LoopLoc = L->getStartLoc();
     Function *F = Header->getParent();
     LLVMContext &Ctx = F->getContext();
     if (PragmaFullUnroll && PragmaCount == 0) {
       if (TripCount && Count != TripCount) {

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

 ; RUN: opt < %s -loop-unroll -S | FileCheck %s
 ;
-; Verify that the unrolling pass removes existing loop unrolling metadata
+; Verify that the unrolling pass removes existing unroll count metadata
 ; and adds a disable unrolling node after unrolling is complete.
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-; #pragma clang loop  vectorize(enable) unroll(full) unroll_count(4) vectorize_width(8)
+; #pragma clang loop  vectorize(enable) unroll_count(4) vectorize_width(8)
 ;
-; Unroll metadata should be replaced with unroll(disable).  Vectorize
+; Unroll count metadata should be replaced with unroll(disable).  Vectorize
 ; metadata should be untouched.
 ;
 ; CHECK-LABEL: @loop1(
 ; CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
 define void @loop1(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* %a, i64 %indvars.iv
   %0 = load 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 !1
 
 for.end:                                          ; preds = %for.body
   ret void
 }
-!1 = metadata !{metadata !1, metadata !2, metadata !3, metadata !4, metadata !5}
+!1 = metadata !{metadata !1, metadata !2, metadata !3, metadata !4}
 !2 = metadata !{metadata !"llvm.loop.vectorize.enable", i1 true}
-!3 = metadata !{metadata !"llvm.loop.unroll.full"}
-!4 = metadata !{metadata !"llvm.loop.unroll.count", i32 4}
-!5 = metadata !{metadata !"llvm.loop.vectorize.width", i32 8}
+!3 = metadata !{metadata !"llvm.loop.unroll.count", i32 4}
+!4 = metadata !{metadata !"llvm.loop.vectorize.width", i32 8}
 
-; #pragma clang loop unroll(disable)
+; #pragma clang loop unroll(full)
 ;
-; Unroll metadata should not change.
+; An unroll disable metadata node is only added for the unroll count case.
+; In this case, metadata should not change.
 ;
 ; CHECK-LABEL: @loop2(
 ; CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_2:.*]]
 define void @loop2(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* %a, i64 %indvars.iv
   %0 = load 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, 1000000
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !5
+
+for.end:                                          ; preds = %for.body
+  ret void
+}
+!5 = metadata !{metadata !5, metadata !6}
+!6 = metadata !{metadata !"llvm.loop.unroll.full"}
+
+; #pragma clang loop unroll(disable)
+;
+; Unroll metadata should not change.
+;
+; CHECK-LABEL: @loop3(
+; CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_3:.*]]
+define void @loop3(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* %a, i64 %indvars.iv
+  %0 = load 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 !6
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !7
 
 for.end:                                          ; preds = %for.body
   ret void
 }
-!6 = metadata !{metadata !6, metadata !7}
-!7 = metadata !{metadata !"llvm.loop.unroll.disable"}
+!7 = metadata !{metadata !7, metadata !8}
+!8 = metadata !{metadata !"llvm.loop.unroll.disable"}
 
 ; CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[VEC_ENABLE:.*]], metadata ![[WIDTH_8:.*]], metadata ![[UNROLL_DISABLE:.*]]}
 ; CHECK: ![[VEC_ENABLE]] = metadata !{metadata !"llvm.loop.vectorize.enable", i1 true}
 ; CHECK: ![[WIDTH_8]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 8}
 ; CHECK: ![[UNROLL_DISABLE]] = metadata !{metadata !"llvm.loop.unroll.disable"}
-; CHECK: ![[LOOP_2]] = metadata !{metadata ![[LOOP_2]], metadata ![[UNROLL_DISABLE:.*]]}
+; CHECK: ![[LOOP_2]] = metadata !{metadata ![[LOOP_2]], metadata ![[UNROLL_FULL:.*]]}
+; CHECK: ![[UNROLL_FULL]] = metadata !{metadata !"llvm.loop.unroll.full"}
+; CHECK: ![[LOOP_3]] = metadata !{metadata ![[LOOP_3]], metadata ![[UNROLL_DISABLE:.*]]}