Skip to content

Commit facca6e

Browse files
committed
Fix a bug in SCEV's backedge taken count computation from my prior fix in Jan.
This has to do with the trip count computation for loops with multiple exits, which is quite subtle. Most passes just ask for a single trip count number, so we must be conservative assuming any exit could be taken. Normally, we rely on the "exact" trip count, which was correctly given as "unknown". However, SCEV also gives a "max" back-edge taken count. The loops max BE taken count is conservatively a maximum over the max of each exit's non-exiting iterations count. Note that some exit tests can be skipped so the max loop back-edge taken count can actually exceed the max non-exiting iterations for some exits. However, when we know the loop *latch* cannot be skipped, we can directly use its max taken count disregarding other exits. I previously took the minimum here without checking whether the other exit could be skipped. The correct, and simpler thing to do here is just to directly use the loop latch's max non-exiting iterations as the loops max back-edge count. In the problematic test case, the first loop exit had a max of zero non-exiting iterations, but could be skipped. The loop latch was known not to be skipped but had max of one non-exiting iteration. We incorrectly claimed the loop back-edge could be taken zero times, when it is actually taken one time. Fixes Loop %for.body.i: <multiple exits> Unpredictable backedge-taken count. Loop %for.body.i: max backedge-taken count is 1. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@209358 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 107db21 commit facca6e

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

lib/Analysis/ScalarEvolution.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4413,7 +4413,7 @@ ScalarEvolution::ComputeBackedgeTakenCount(const Loop *L) {
44134413
const SCEV *MaxBECount = getCouldNotCompute();
44144414
bool CouldComputeBECount = true;
44154415
BasicBlock *Latch = L->getLoopLatch(); // may be NULL.
4416-
const SCEV *LatchMaxCount = nullptr;
4416+
bool LatchMustExit = false;
44174417
SmallVector<std::pair<BasicBlock *, const SCEV *>, 4> ExitCounts;
44184418
for (unsigned i = 0, e = ExitingBlocks.size(); i != e; ++i) {
44194419
ExitLimit EL = ComputeExitLimit(L, ExitingBlocks[i]);
@@ -4431,16 +4431,14 @@ ScalarEvolution::ComputeBackedgeTakenCount(const Loop *L) {
44314431
// skip some loop tests. Taking the max over the exits is sufficiently
44324432
// conservative. TODO: We could do better taking into consideration
44334433
// non-latch exits that dominate the latch.
4434-
if (EL.MustExit && ExitingBlocks[i] == Latch)
4435-
LatchMaxCount = EL.Max;
4436-
else
4434+
if (EL.MustExit && ExitingBlocks[i] == Latch) {
4435+
MaxBECount = EL.Max;
4436+
LatchMustExit = true;
4437+
}
4438+
else if (!LatchMustExit)
44374439
MaxBECount = getUMaxFromMismatchedTypes(MaxBECount, EL.Max);
44384440
}
44394441
}
4440-
// Be more precise in the easy case of a loop latch that must exit.
4441-
if (LatchMaxCount) {
4442-
MaxBECount = getUMinFromMismatchedTypes(MaxBECount, LatchMaxCount);
4443-
}
44444442
return BackedgeTakenInfo(ExitCounts, CouldComputeBECount, MaxBECount);
44454443
}
44464444

test/Analysis/ScalarEvolution/max-trip-count.ll

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,29 @@ for.end: ; preds = %for.cond.for.end_cr
9898
; CHECK: Determining loop execution counts for: @test
9999
; CHECK-NEXT: backedge-taken count is
100100
; CHECK-NEXT: max backedge-taken count is -1
101+
102+
; PR19799: Indvars miscompile due to an incorrect max backedge taken count from SCEV.
103+
; CHECK-LABEL: @pr19799
104+
; CHECK: Loop %for.body.i: <multiple exits> Unpredictable backedge-taken count.
105+
; CHECK: Loop %for.body.i: max backedge-taken count is 1
106+
@a = common global i32 0, align 4
107+
108+
define i32 @pr19799() {
109+
entry:
110+
store i32 -1, i32* @a, align 4
111+
br label %for.body.i
112+
113+
for.body.i: ; preds = %for.cond.i, %entry
114+
%storemerge1.i = phi i32 [ -1, %entry ], [ %add.i.i, %for.cond.i ]
115+
%tobool.i = icmp eq i32 %storemerge1.i, 0
116+
%add.i.i = add nsw i32 %storemerge1.i, 2
117+
br i1 %tobool.i, label %bar.exit, label %for.cond.i
118+
119+
for.cond.i: ; preds = %for.body.i
120+
store i32 %add.i.i, i32* @a, align 4
121+
%cmp.i = icmp slt i32 %storemerge1.i, 0
122+
br i1 %cmp.i, label %for.body.i, label %bar.exit
123+
124+
bar.exit: ; preds = %for.cond.i, %for.body.i
125+
ret i32 0
126+
}

0 commit comments

Comments
 (0)