[LoopPeel] Use loop guards when checking if last iter can be peeled. #142605
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesApply loop guards to BTC before checking if the last iteration should be peeled off. This also adds an assert to make sure applying the guards does not pessimize the results. I checked on a large test set and it did not trigger there, but it adds an additional guard to catch potential cases where loop-guards pessimize results. Peels ~15% more loops. Full diff: https://github.com/llvm/llvm-project/pull/142605.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/LoopPeel.cpp b/llvm/lib/Transforms/Utils/LoopPeel.cpp
index bd025fddd0cf7..564c2298f6f2f 100644
--- a/llvm/lib/Transforms/Utils/LoopPeel.cpp
+++ b/llvm/lib/Transforms/Utils/LoopPeel.cpp
@@ -357,17 +357,14 @@ bool llvm::canPeelLastIteration(const Loop &L, ScalarEvolution &SE) {
m_scev_AffineAddRec(m_SCEV(), m_scev_One(), m_SpecificLoop(&L)));
}
-/// Returns true if the last iteration can be peeled off and the condition (Pred
-/// LeftAR, RightSCEV) is known at the last iteration and the inverse condition
-/// is known at the second-to-last.
+/// Returns true if the last iteration should be peeled off, i.e. the condition
+/// (Pred LeftAR, RightSCEV) is known at the last iteration and the inverse
+/// condition is known at the second-to-last.
static bool shouldPeelLastIteration(Loop &L, CmpPredicate Pred,
const SCEVAddRecExpr *LeftAR,
- const SCEV *RightSCEV, ScalarEvolution &SE,
+ const SCEV *RightSCEV, const SCEV *BTC,
+ ScalarEvolution &SE,
const TargetTransformInfo &TTI) {
- if (!canPeelLastIteration(L, SE))
- return false;
-
- const SCEV *BTC = SE.getBackedgeTakenCount(&L);
SCEVExpander Expander(SE, L.getHeader()->getDataLayout(), "loop-peel");
if (!SE.isKnownNonZero(BTC) &&
Expander.isHighCostExpansion(BTC, &L, SCEVCheapExpansionBudget, &TTI,
@@ -377,7 +374,6 @@ static bool shouldPeelLastIteration(Loop &L, CmpPredicate Pred,
const SCEV *ValAtLastIter = LeftAR->evaluateAtIteration(BTC, SE);
const SCEV *ValAtSecondToLastIter = LeftAR->evaluateAtIteration(
SE.getMinusSCEV(BTC, SE.getOne(BTC->getType())), SE);
-
return SE.isKnownPredicate(ICmpInst::getInversePredicate(Pred), ValAtLastIter,
RightSCEV) &&
SE.isKnownPredicate(Pred, ValAtSecondToLastIter, RightSCEV);
@@ -484,8 +480,19 @@ countToEliminateCompares(Loop &L, unsigned MaxPeelCount, ScalarEvolution &SE,
const SCEV *Step = LeftAR->getStepRecurrence(SE);
if (!PeelWhilePredicateIsKnown(NewPeelCount, IterVal, RightSCEV, Step,
Pred)) {
- if (shouldPeelLastIteration(L, Pred, LeftAR, RightSCEV, SE, TTI))
+ if (!canPeelLastIteration(L, SE))
+ return;
+
+ const SCEV *BTC = SE.getBackedgeTakenCount(&L);
+ auto Guards = ScalarEvolution::LoopGuards::collect(&L, SE);
+ if (shouldPeelLastIteration(L, Pred, LeftAR,
+ SE.applyLoopGuards(RightSCEV, Guards),
+ SE.applyLoopGuards(BTC, Guards), SE, TTI))
DesiredPeelCountLast = 1;
+ else
+ assert(!shouldPeelLastIteration(L, Pred, LeftAR, RightSCEV, BTC, SE,
+ TTI) &&
+ "loop guards pessimized result");
return;
}
diff --git a/llvm/test/Transforms/LoopUnroll/peel-last-iteration-with-guards.ll b/llvm/test/Transforms/LoopUnroll/peel-last-iteration-with-guards.ll
index af07a97131322..824e23fcf3e6e 100644
--- a/llvm/test/Transforms/LoopUnroll/peel-last-iteration-with-guards.ll
+++ b/llvm/test/Transforms/LoopUnroll/peel-last-iteration-with-guards.ll
@@ -13,14 +13,33 @@ define void @peel_with_guard_known_nonnegative_1(i32 %n) {
; CHECK-NEXT: [[N_EXT:%.*]] = zext i32 [[N]] to i64
; CHECK-NEXT: [[N_1:%.*]] = add i32 [[N]], 1
; CHECK-NEXT: [[WIDE_TRIP_COUNT:%.*]] = zext i32 [[N_1]] to i64
+; CHECK-NEXT: [[TMP0:%.*]] = add nsw i64 [[WIDE_TRIP_COUNT]], -1
+; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i64 [[TMP0]], 0
+; CHECK-NEXT: br i1 [[TMP1]], label %[[PH_SPLIT:.*]], label %[[EXIT_LOOPEXIT_PEEL_BEGIN:.*]]
+; CHECK: [[PH_SPLIT]]:
; CHECK-NEXT: br label %[[LOOP:.*]]
; CHECK: [[LOOP]]:
-; CHECK-NEXT: [[IV:%.*]] = phi i64 [ 0, %[[PH]] ], [ [[IV_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[IV1:%.*]] = phi i64 [ 0, %[[PH_SPLIT]] ], [ [[IV_NEXT1:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[IV_NEXT1]] = add nuw nsw i64 [[IV1]], 1
+; CHECK-NEXT: [[TMP2:%.*]] = sub i64 [[WIDE_TRIP_COUNT]], 1
+; CHECK-NEXT: [[EC1:%.*]] = icmp eq i64 [[IV_NEXT1]], [[TMP2]]
+; CHECK-NEXT: br i1 [[EC1]], label %[[EXIT_LOOPEXIT_PEEL_BEGIN_LOOPEXIT:.*]], label %[[LOOP]], !llvm.loop [[LOOP0:![0-9]+]]
+; CHECK: [[EXIT_LOOPEXIT_PEEL_BEGIN_LOOPEXIT]]:
+; CHECK-NEXT: [[DOTPH:%.*]] = phi i64 [ [[IV_NEXT1]], %[[LOOP]] ]
+; CHECK-NEXT: br label %[[EXIT_LOOPEXIT_PEEL_BEGIN]]
+; CHECK: [[EXIT_LOOPEXIT_PEEL_BEGIN]]:
+; CHECK-NEXT: [[IV:%.*]] = phi i64 [ 0, %[[PH]] ], [ [[DOTPH]], %[[EXIT_LOOPEXIT_PEEL_BEGIN_LOOPEXIT]] ]
+; CHECK-NEXT: br label %[[LOOP_PEEL:.*]]
+; CHECK: [[LOOP_PEEL]]:
; CHECK-NEXT: [[C:%.*]] = icmp eq i64 [[IV]], [[N_EXT]]
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[C]], i32 10, i32 20
-; CHECK-NEXT: [[IV_NEXT]] = add i64 [[IV]], 1
+; CHECK-NEXT: [[IV_NEXT:%.*]] = add i64 [[IV]], 1
; CHECK-NEXT: [[EC:%.*]] = icmp eq i64 [[IV_NEXT]], [[WIDE_TRIP_COUNT]]
-; CHECK-NEXT: br i1 [[EC]], label %[[EXIT_LOOPEXIT:.*]], label %[[LOOP]]
+; CHECK-NEXT: br i1 [[EC]], label %[[EXIT_LOOPEXIT_PEEL_NEXT:.*]], label %[[EXIT_LOOPEXIT_PEEL_NEXT]]
+; CHECK: [[EXIT_LOOPEXIT_PEEL_NEXT]]:
+; CHECK-NEXT: br label %[[LOOP_PEEL_NEXT:.*]]
+; CHECK: [[LOOP_PEEL_NEXT]]:
+; CHECK-NEXT: br label %[[EXIT_LOOPEXIT:.*]]
; CHECK: [[EXIT_LOOPEXIT]]:
; CHECK-NEXT: br label %[[EXIT]]
; CHECK: [[EXIT]]:
@@ -137,7 +156,7 @@ define void @peel_with_guard2(i32 %n) {
; CHECK-NEXT: [[IV_NEXT]] = add nuw i32 [[IV]], 1
; CHECK-NEXT: [[TMP2:%.*]] = sub i32 [[N]], 1
; CHECK-NEXT: [[EC:%.*]] = icmp eq i32 [[IV_NEXT]], [[TMP2]]
-; CHECK-NEXT: br i1 [[EC]], label %[[EXIT_LOOPEXIT_PEEL_BEGIN_LOOPEXIT:.*]], label %[[LOOP_HEADER]], !llvm.loop [[LOOP0:![0-9]+]]
+; CHECK-NEXT: br i1 [[EC]], label %[[EXIT_LOOPEXIT_PEEL_BEGIN_LOOPEXIT:.*]], label %[[LOOP_HEADER]], !llvm.loop [[LOOP2:![0-9]+]]
; CHECK: [[EXIT_LOOPEXIT_PEEL_BEGIN_LOOPEXIT]]:
; CHECK-NEXT: [[DOTPH:%.*]] = phi i32 [ [[IV_NEXT]], %[[LOOP_LATCH]] ]
; CHECK-NEXT: br label %[[EXIT_LOOPEXIT_PEEL_BEGIN]]
@@ -188,4 +207,5 @@ exit:
;.
; CHECK: [[LOOP0]] = distinct !{[[LOOP0]], [[META1:![0-9]+]]}
; CHECK: [[META1]] = !{!"llvm.loop.peeled.count", i32 1}
+; CHECK: [[LOOP2]] = distinct !{[[LOOP2]], [[META1]]}
;.
|
nikic
reviewed
Jun 3, 2025
| else | ||
| assert(!shouldPeelLastIteration(L, Pred, LeftAR, RightSCEV, BTC, SE, | ||
| TTI) && | ||
| "loop guards pessimized result"); |
There was a problem hiding this comment.
I don't think this assertion should exist. Based on past experience, you can always construct edge cases where this happens.
There was a problem hiding this comment.
Yeah fair enough, I removed it. Still wondering if there's a nice way to have some kind of verification as opt-in?
| if (!canPeelLastIteration(L, SE)) | ||
| return false; | ||
|
|
||
| const SCEV *BTC = SE.getBackedgeTakenCount(&L); | ||
| SCEVExpander Expander(SE, L.getHeader()->getDataLayout(), "loop-peel"); | ||
| if (!SE.isKnownNonZero(BTC) && | ||
| Expander.isHighCostExpansion(BTC, &L, SCEVCheapExpansionBudget, &TTI, |
There was a problem hiding this comment.
The high cost expansion check should probably not use the BTC with loop guards? We're not going to actually expand it with the loop guards, right?
There was a problem hiding this comment.
Yep loop-guards aren't used during expansion, removed for now. Some of the info may be used by later passes to simplifiy the expansion.
Apply loop guards to BTC before checking if the last iteration should be peeled off. This also adds an assert to make sure applying the guards does not pessimize the results. I checked on a large test set and it did not trigger there, but it adds an additional guard to catch potential cases where loop-guards pessimize results. Peels ~15% more loops.
fhahn
force-pushed
the
peel-last-iter-with-guards
branch
from
8e0bea9 to
4b4a394
Compare
fhahn
force-pushed
the
peel-last-iter-with-guards
branch
from
4b4a394 to
c7deb11
Compare
|
ping :) |
nikic
approved these changes
Jun 9, 2025
| @@ -374,6 +374,9 @@ static bool shouldPeelLastIteration(Loop &L, CmpPredicate Pred, | |||
| L.getLoopPredecessor()->getTerminator())) | |||
| return false; | |||
|
|
|||
| auto Guards = ScalarEvolution::LoopGuards::collect(&L, SE); | |||
| BTC = SE.applyLoopGuards(BTC, Guards); | |||
There was a problem hiding this comment.
FWIW, I do think using loop guards for the isKnownNonZero(BTC) check makes sense, I was just saying it shouldn't be used for the high cost expansion check.
There was a problem hiding this comment.
Ah yes, makes sense. I'll adjust it separately, with a separate test case.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 10, 2025
…be peeled. (#142605) Apply loop guards to BTC before checking if the last iteration should be peeled off. This also adds an assert to make sure applying the guards does not pessimize the results. I checked on a large test set and it did not trigger there, but it adds an additional guard to catch potential cases where loop-guards pessimize results. Peels ~15% more loops. PR: llvm/llvm-project#142605
rorth
pushed a commit
to rorth/llvm-project
that referenced
this pull request
Jun 11, 2025
…lvm#142605) Apply loop guards to BTC before checking if the last iteration should be peeled off. This also adds an assert to make sure applying the guards does not pessimize the results. I checked on a large test set and it did not trigger there, but it adds an additional guard to catch potential cases where loop-guards pessimize results. Peels ~15% more loops. PR: llvm#142605
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
…lvm#142605) Apply loop guards to BTC before checking if the last iteration should be peeled off. This also adds an assert to make sure applying the guards does not pessimize the results. I checked on a large test set and it did not trigger there, but it adds an additional guard to catch potential cases where loop-guards pessimize results. Peels ~15% more loops. PR: llvm#142605
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Apply loop guards to BTC before checking if the last iteration should be peeled off. This also adds an assert to make sure applying the guards does not pessimize the results. I checked on a large test set and it did not trigger there, but it adds an additional guard to catch potential cases where loop-guards pessimize results.
Peels ~15% more loops.