Skip to content

Conversation

@ivankelarev
Copy link
Contributor

@ivankelarev ivankelarev commented Oct 24, 2025

Currently, LoopFullUnrollPass incorrectly performs partial unrolling when #pragma unroll is specified and both TripCount and MaxTripCount are unknown. This patch adds a check to prevent partial unrolling when OnlyFullUnroll parameter is true and both trip count values are zero.

@ivankelarev ivankelarev changed the title Prevent LoopUnrollPass from doing partial unroll if TripCount and MaxTripCount are unknown [LoopUnroll] Prevent LoopFullUnrollPass from performing partial unrolling when trip counts are unknown Oct 24, 2025
@ivankelarev ivankelarev marked this pull request as ready for review October 24, 2025 17:04
@ivankelarev ivankelarev requested a review from modiking October 24, 2025 17:05
@llvmbot
Copy link
Member

llvmbot commented Oct 24, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Ivan Kelarev (ivankelarev)

Changes

Currently, LoopFullUnrollPass incorrectly performs partial unrolling when #pragma unroll is specified and both TripCount and MaxTripCount are unknown. This patch adds a check to prevent partial unrolling when OnlyFullUnroll parameter is true and both trip count values are zero.


Full diff: https://github.com/llvm/llvm-project/pull/165013.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp (+2-1)
  • (modified) llvm/test/Transforms/LoopUnroll/full-unroll-avoid-partial.ll (+28)
diff --git a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
index 2bda9d83236e8..802ae4e9c28e3 100644
--- a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
@@ -1327,7 +1327,8 @@ tryToUnrollLoop(Loop *L, DominatorTree &DT, LoopInfo *LI, ScalarEvolution &SE,
   }
 
   // Do not attempt partial/runtime unrolling in FullLoopUnrolling
-  if (OnlyFullUnroll && (UP.Count < TripCount || UP.Count < MaxTripCount)) {
+  if (OnlyFullUnroll && ((!TripCount && !MaxTripCount) ||
+                         UP.Count < TripCount || UP.Count < MaxTripCount)) {
     LLVM_DEBUG(
         dbgs() << "Not attempting partial/runtime unroll in FullLoopUnroll.\n");
     return LoopUnrollResult::Unmodified;
diff --git a/llvm/test/Transforms/LoopUnroll/full-unroll-avoid-partial.ll b/llvm/test/Transforms/LoopUnroll/full-unroll-avoid-partial.ll
index 7f266a754d1bc..55f2c8b8dc19a 100644
--- a/llvm/test/Transforms/LoopUnroll/full-unroll-avoid-partial.ll
+++ b/llvm/test/Transforms/LoopUnroll/full-unroll-avoid-partial.ll
@@ -85,6 +85,34 @@ for.body:                                         ; preds = %for.body.preheader,
   br i1 %exitcond, label %for.body, label %for.cond.cleanup.loopexit, !llvm.loop !3
 }
 
+; LOOP-UNROLL-LABEL: Loop Unroll: F[pragma_unroll_count2] Loop %
+; LOOP-UNROLL-NEXT: Loop Size = 4
+; LOOP-UNROLL-NEXT: Exiting block %: TripCount=0, TripMultiple=1, BreakoutTrip=1
+; LOOP-UNROLL-NEXT: Trying runtime unrolling on Loop:
+; LOOP-UNROLL-NEXT: Loop at depth 1 containing: %2<header><exiting>,%5<latch>
+; LOOP-UNROLL-NEXT: Using epilog remainder.
+; LOOP-UNROLL-NEXT: Loop latch not terminated by a conditional branch.
+; LOOP-UNROLL-NEXT: UNROLLING loop % by 5!
+
+; LOOP-UNROLL-FULL-LABEL: Loop Unroll: F[pragma_unroll_count2] Loop %
+; LOOP-UNROLL-FULL-NEXT: Loop Size = 4
+; LOOP-UNROLL-FULL-NEXT: Not attempting partial/runtime unroll in FullLoopUnroll
+define void @pragma_unroll_count2(i64 %0) {
+  br label %2
+
+2:                                                ; preds = %5, %1
+  %3 = phi i64 [ 0, %1 ], [ %6, %5 ]
+  %4 = icmp ult i64 %3, %0
+  br i1 %4, label %5, label %7
+
+5:                                                ; preds = %2
+  %6 = add i64 %3, 8
+  br label %2, !llvm.loop !3
+
+7:                                                ; preds = %2
+  ret void
+}
+
 ; LOOP-UNROLL: llvm.loop.unroll.disable
 ; LOOP-UNROLL-FULL: llvm.loop.unroll.enable
 !0 = !{!"llvm.loop.unroll.enable"}

Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

That check should ideally be done together where partial/runtime unrolling is attempted (instead of mixing it with the profitabilit heuristic), but it is correct if the trip count is not known, full unrolling cannot take place.

Comment on lines 103 to 112
2: ; preds = %5, %1
%3 = phi i64 [ 0, %1 ], [ %6, %5 ]
%4 = icmp ult i64 %3, %0
br i1 %4, label %5, label %7

5: ; preds = %2
%6 = add i64 %3, 8
br label %2, !llvm.loop !3

7: ; preds = %2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use named values/basic blocks here to make the test easier to read

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@ivankelarev ivankelarev force-pushed the full-unroll-no-max-trip-count branch from c482629 to 695171b Compare November 3, 2025 20:58
@ivankelarev ivankelarev requested a review from fhahn November 3, 2025 20:58
@ivankelarev ivankelarev merged commit 37825ad into llvm:main Nov 4, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants