Skip to content

Commit 4e46ff4

Browse files
fhahntstellar
authored andcommitted
[SCEV] By more careful when traversing phis in isImpliedViaMerge.
I think currently isImpliedViaMerge can incorrectly return true for phis in a loop/cycle, if the found condition involves the previous value of Consider the case in exit_cond_depends_on_inner_loop. At some point, we call (modulo simplifications) isImpliedViaMerge(<=, %x.lcssa, -1, %call, -1). The existing code tries to prove IncV <= -1 for all incoming values InvV using the found condition (%call <= -1). At the moment this succeeds, but only because it does not compare the same runtime value. The found condition checks the value of the last iteration, but the incoming value is from the *previous* iteration. Hence we incorrectly determine that the *previous* value was <= -1, which may not be true. I think we need to be more careful when looking at the incoming values here. In particular, we need to rule out that a found condition refers to any value that may refer to one of the previous iterations. I'm not sure there's a reliable way to do so (that also works of irreducible control flow). So for now this patch adds an additional requirement that the incoming value must properly dominate the phi block. This should ensure the values do not change in a cycle. I am not entirely sure if will catch all cases and I appreciate a through second look in that regard. Alternatively we could also unconditionally bail out in this case, instead of checking the incoming values Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D101829 (cherry picked from commit 6c99e63)
1 parent 0ef7836 commit 4e46ff4

File tree

3 files changed

+12
-6
lines changed

3 files changed

+12
-6
lines changed

llvm/lib/Analysis/ScalarEvolution.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10622,6 +10622,10 @@ bool ScalarEvolution::isImpliedViaMerge(ICmpInst::Predicate Pred,
1062210622
if (!dominates(RHS, IncBB))
1062310623
return false;
1062410624
const SCEV *L = getSCEV(LPhi->getIncomingValueForBlock(IncBB));
10625+
// Make sure L does not refer to a value from a potentially previous
10626+
// iteration of a loop.
10627+
if (!properlyDominates(L, IncBB))
10628+
return false;
1062510629
if (!ProvedEasily(L, RHS))
1062610630
return false;
1062710631
}

llvm/test/Transforms/IRCE/decrementing-loop.ll

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,16 +212,17 @@ exit:
212212
ret void
213213
}
214214

215+
; TODO: we need to be more careful when trying to look through phi nodes in
216+
; cycles, because the condition to prove may reference the previous value of
217+
; the phi. So we currently fail to optimize this case.
215218
; Check that we can figure out that IV is non-negative via implication through
216219
; two Phi nodes, one being AddRec.
217220
define void @test_05(i32* %a, i32* %a_len_ptr, i1 %cond) {
218221

219222
; CHECK-LABEL: test_05
220-
; CHECK: mainloop:
221-
; CHECK-NEXT: br label %loop
222-
; CHECK: loop:
223-
; CHECK: br i1 true, label %in.bounds, label %out.of.bounds
224-
; CHECK: loop.preloop:
223+
; CHECK: entry:
224+
; CHECK: br label %merge
225+
; CHECK-NOT: mainloop
225226

226227
entry:
227228
%len.a = load i32, i32* %a_len_ptr, !range !0

llvm/test/Transforms/IndVarSimplify/eliminate-exit.ll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,8 @@ define i32 @exit_cond_depends_on_inner_loop() {
453453
; CHECK-NEXT: br i1 [[INNER_COND]], label [[INNER]], label [[OUTER_EXITING_1:%.*]]
454454
; CHECK: outer.exiting.1:
455455
; CHECK-NEXT: [[X_LCSSA:%.*]] = phi i32 [ [[X]], [[INNER]] ]
456-
; CHECK-NEXT: br i1 false, label [[EXIT:%.*]], label [[OUTER_LATCH]]
456+
; CHECK-NEXT: [[OUTER_COND_1:%.*]] = icmp sgt i32 [[X_LCSSA]], -1
457+
; CHECK-NEXT: br i1 [[OUTER_COND_1]], label [[EXIT:%.*]], label [[OUTER_LATCH]]
457458
; CHECK: outer.latch:
458459
; CHECK-NEXT: [[IV_OUTER_NEXT]] = add nuw nsw i32 [[IV_OUTER]], 1
459460
; CHECK-NEXT: [[OUTER_COND_2:%.*]] = icmp ult i32 [[IV_OUTER]], 100

0 commit comments

Comments
 (0)