Skip to content

Commit

Permalink
Fix a bug in SCEV's poison value propagation
Browse files Browse the repository at this point in the history
The worklist algorithm introduced in rL271151 didn't check to see if the
direct users of the post-inc add recurrence propagates poison.  This
change fixes the problem and makes the code structure more obvious.

Note for release managers: correctness wise, this bug wasn't a
regression introduced by rL271151 -- the behavior of SCEV around
post-inc add recurrences was strictly improved (in terms of correctness)
in rL271151.

llvm-svn: 272179
  • Loading branch information
sanjoy committed Jun 8, 2016
1 parent 86be374 commit a19edc4
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 12 deletions.
25 changes: 13 additions & 12 deletions llvm/lib/Analysis/ScalarEvolution.cpp
Expand Up @@ -4880,22 +4880,23 @@ bool ScalarEvolution::isAddRecNeverPoison(const Instruction *I, const Loop *L) {
return false;

SmallPtrSet<const Instruction *, 16> Pushed;
SmallVector<const Instruction *, 8> Stack;
SmallVector<const Instruction *, 8> PoisonStack;

// We start by assuming \c I, the post-inc add recurrence, is poison. Only
// things that are known to be fully poison under that assumption go on the
// PoisonStack.
Pushed.insert(I);
for (auto *U : I->users())
if (Pushed.insert(cast<Instruction>(U)).second)
Stack.push_back(cast<Instruction>(U));
PoisonStack.push_back(I);

bool LatchControlDependentOnPoison = false;
while (!Stack.empty()) {
const Instruction *I = Stack.pop_back_val();

for (auto *U : I->users()) {
if (propagatesFullPoison(cast<Instruction>(U))) {
if (Pushed.insert(cast<Instruction>(U)).second)
Stack.push_back(cast<Instruction>(U));
} else if (auto *BI = dyn_cast<BranchInst>(U)) {
while (!PoisonStack.empty()) {
const Instruction *Poison = PoisonStack.pop_back_val();

for (auto *PoisonUser : Poison->users()) {
if (propagatesFullPoison(cast<Instruction>(PoisonUser))) {
if (Pushed.insert(cast<Instruction>(PoisonUser)).second)
PoisonStack.push_back(cast<Instruction>(PoisonUser));
} else if (auto *BI = dyn_cast<BranchInst>(PoisonUser)) {
assert(BI->isConditional() && "Only possibility!");
if (BI->getParent() == LatchBB) {
LatchControlDependentOnPoison = true;
Expand Down
36 changes: 36 additions & 0 deletions llvm/test/Analysis/ScalarEvolution/nsw.ll
Expand Up @@ -204,3 +204,39 @@ for.body:
for.end:
ret void
}


define void @bad_postinc_nsw_a(i32 %n) {
; CHECK-LABEL: Classifying expressions for: @bad_postinc_nsw_a
entry:
br label %loop

loop:
%iv = phi i32 [ 0, %entry ], [ %iv.inc, %loop ]
%iv.inc = add nsw i32 %iv, 7
; CHECK: %iv.inc = add nsw i32 %iv, 7
; CHECK-NEXT: --> {7,+,7}<nuw><%loop>
%becond = icmp ult i32 %iv, %n
br i1 %becond, label %loop, label %leave

leave:
ret void
}

define void @bad_postinc_nsw_b(i32 %n) {
; CHECK-LABEL: Classifying expressions for: @bad_postinc_nsw_b
entry:
br label %loop

loop:
%iv = phi i32 [ 0, %entry ], [ %iv.inc, %loop ]
%iv.inc = add nsw i32 %iv, 7
%iv.inc.and = and i32 %iv.inc, 0
; CHECK: %iv.inc = add nsw i32 %iv, 7
; CHECK-NEXT: --> {7,+,7}<nuw><%loop>
%becond = icmp ult i32 %iv.inc.and, %n
br i1 %becond, label %loop, label %leave

leave:
ret void
}

0 comments on commit a19edc4

Please sign in to comment.