[llvm] r271151 - [SCEV] Don't always add no-wrap flags to post-inc add recs
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Sat May 28 17:32:52 PDT 2016
Author: sanjoy
Date: Sat May 28 19:32:17 2016
New Revision: 271151
URL: http://llvm.org/viewvc/llvm-project?rev=271151&view=rev
Log:
[SCEV] Don't always add no-wrap flags to post-inc add recs
Fixes PR27315.
The post-inc version of an add recurrence needs to "follow the same
rules" as a normal add or subtract expression. Otherwise we miscompile
programs like
```
int main() {
int a = 0;
unsigned a_u = 0;
volatile long last_value;
do {
a_u += 3;
last_value = (long) ((int) a_u);
if (will_add_overflow(a, 3)) {
// Leave, and don't actually do the increment, so no UB.
printf("last_value = %ld\n", last_value);
exit(0);
}
a += 3;
} while (a != 46);
return 0;
}
```
This patch changes SCEV to put no-wrap flags on post-inc add recurrences
only when the poison from a potential overflow will go ahead to cause
undefined behavior.
To avoid regressing performance too much, I've assumed infinite loops
without side effects is undefined behavior to prove poison<->UB
equivalence in more cases. This isn't ideal, but is not new to LLVM as
a whole, and far better than the situation I'm trying to fix.
Added:
llvm/trunk/test/Analysis/ScalarEvolution/pr27315.ll
Modified:
llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
llvm/trunk/lib/Analysis/ScalarEvolution.cpp
llvm/trunk/test/Analysis/ScalarEvolution/infer-prestart-no-wrap.ll
llvm/trunk/test/Analysis/ScalarEvolution/nowrap-preinc-limits.ll
llvm/trunk/test/Analysis/ScalarEvolution/nsw.ll
llvm/trunk/test/Transforms/IndVarSimplify/elim-extend.ll
Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolution.h?rev=271151&r1=271150&r2=271151&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/ScalarEvolution.h (original)
+++ llvm/trunk/include/llvm/Analysis/ScalarEvolution.h Sat May 28 19:32:17 2016
@@ -784,6 +784,10 @@ namespace llvm {
SmallVector<PointerIntPair<const Loop *, 2, LoopDisposition>, 2>>
LoopDispositions;
+ /// A cache of the predicate "does the given loop contain an instruction
+ /// that can throw?"
+ DenseMap<const Loop *, bool> LoopMayThrow;
+
/// Compute a LoopDisposition value.
LoopDisposition computeLoopDisposition(const SCEV *S, const Loop *L);
@@ -1124,6 +1128,12 @@ namespace llvm {
/// it is not okay to annotate (+ a b) with <nsw> in the above example.
bool isSCEVExprNeverPoison(const Instruction *I);
+ /// This is like \c isSCEVExprNeverPoison but it specifically works for
+ /// instructions that will get mapped to SCEV add recurrences. Return true
+ /// if \p I will never generate poison under the assumption that \p I is an
+ /// add recurrence on the loop \p L.
+ bool isAddRecNeverPoison(const Instruction *I, const Loop *L);
+
public:
ScalarEvolution(Function &F, TargetLibraryInfo &TLI, AssumptionCache &AC,
DominatorTree &DT, LoopInfo &LI);
Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=271151&r1=271150&r2=271151&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
+++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Sat May 28 19:32:17 2016
@@ -3980,8 +3980,6 @@ const SCEV *ScalarEvolution::createAddRe
cast<SCEVAddRecExpr>(Accum)->getLoop() == L)) {
SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap;
- // If the increment doesn't overflow, then neither the addrec nor
- // the post-increment will overflow.
if (auto BO = MatchBinaryOp(BEValueV)) {
if (BO->Opcode == Instruction::Add && BO->LHS == PN) {
if (BO->IsNUW)
@@ -4012,16 +4010,19 @@ const SCEV *ScalarEvolution::createAddRe
const SCEV *StartVal = getSCEV(StartValueV);
const SCEV *PHISCEV = getAddRecExpr(StartVal, Accum, L, Flags);
- // Since the no-wrap flags are on the increment, they apply to the
- // post-incremented value as well.
- if (isLoopInvariant(Accum, L))
- (void)getAddRecExpr(getAddExpr(StartVal, Accum), Accum, L, Flags);
-
// Okay, for the entire analysis of this edge we assumed the PHI
// to be symbolic. We now need to go back and purge all of the
// entries for the scalars that use the symbolic expression.
forgetSymbolicName(PN, SymbolicName);
ValueExprMap[SCEVCallbackVH(PN, this)] = PHISCEV;
+
+ // We can add Flags to the post-inc expression only if we
+ // know that it us *undefined behavior* for BEValueV to
+ // overflow.
+ if (auto *BEInst = dyn_cast<Instruction>(BEValueV))
+ if (isLoopInvariant(Accum, L) && isAddRecNeverPoison(BEInst, L))
+ (void)getAddRecExpr(getAddExpr(StartVal, Accum), Accum, L, Flags);
+
return PHISCEV;
}
}
@@ -4850,6 +4851,87 @@ bool ScalarEvolution::isSCEVExprNeverPoi
return false;
}
+bool ScalarEvolution::isAddRecNeverPoison(const Instruction *I, const Loop *L) {
+ // If we know that \c I can never be poison period, then that's enough.
+ if (isSCEVExprNeverPoison(I))
+ return true;
+
+ // For an add recurrence specifically, we assume that infinite loops without
+ // side effects are undefined behavior, and then reason as follows:
+ //
+ // If the add recurrence is poison in any iteration, it is poison on all
+ // future iterations (since incrementing poison yields poison). If the result
+ // of the add recurrence is fed into the loop latch condition and the loop
+ // does not contain any throws or exiting blocks other than the latch, we now
+ // have the ability to "choose" whether the backedge is taken or not (by
+ // choosing a sufficiently evil value for the poison feeding into the branch)
+ // for every iteration including and after the one in which \p I first became
+ // poison. There are two possibilities (let's call the iteration in which \p
+ // I first became poison as K):
+ //
+ // 1. In the set of iterations including and after K, the loop body executes
+ // no side effects. In this case executing the backege an infinte number
+ // of times will yield undefined behavior.
+ //
+ // 2. In the set of iterations including and after K, the loop body executes
+ // at least one side effect. In this case, that specific instance of side
+ // effect is control dependent on poison, which also yields undefined
+ // behavior.
+
+ auto *ExitingBB = L->getExitingBlock();
+ auto *LatchBB = L->getLoopLatch();
+ if (!ExitingBB || !LatchBB || ExitingBB != LatchBB)
+ return false;
+
+ SmallPtrSet<const Instruction *, 16> Pushed;
+ SmallVector<const Instruction *, 8> Stack;
+
+ Pushed.insert(I);
+ for (auto *U : I->users())
+ if (Pushed.insert(cast<Instruction>(U)).second)
+ Stack.push_back(cast<Instruction>(U));
+
+ 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)) {
+ assert(BI->isConditional() && "Only possibility!");
+ if (BI->getParent() == LatchBB) {
+ LatchControlDependentOnPoison = true;
+ break;
+ }
+ }
+ }
+ }
+
+ if (!LatchControlDependentOnPoison)
+ return false;
+
+ // Now check if loop is no-throw, and cache the information. In the future,
+ // we can consider commoning this logic with LICMSafetyInfo into a separate
+ // analysis pass.
+
+ auto Itr = LoopMayThrow.find(L);
+ if (Itr == LoopMayThrow.end()) {
+ bool MayThrow = false;
+ for (auto *BB : L->getBlocks()) {
+ MayThrow = any_of(*BB, [](Instruction &I) { return I.mayThrow(); });
+ if (MayThrow)
+ break;
+ }
+ auto InsertPair = LoopMayThrow.insert({L, MayThrow});
+ assert(InsertPair.second && "We just checked!");
+ Itr = InsertPair.first;
+ }
+
+ return !Itr->second;
+}
+
/// createSCEV - We know that there is no SCEV for the specified value. Analyze
/// the expression.
///
@@ -5439,6 +5521,8 @@ void ScalarEvolution::forgetLoop(const L
// ValuesAtScopes map.
for (Loop::iterator I = L->begin(), E = L->end(); I != E; ++I)
forgetLoop(*I);
+
+ LoopMayThrow.erase(L);
}
/// forgetValue - This method should be called by the client when it has
Modified: llvm/trunk/test/Analysis/ScalarEvolution/infer-prestart-no-wrap.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/infer-prestart-no-wrap.ll?rev=271151&r1=271150&r2=271151&view=diff
==============================================================================
--- llvm/trunk/test/Analysis/ScalarEvolution/infer-prestart-no-wrap.ll (original)
+++ llvm/trunk/test/Analysis/ScalarEvolution/infer-prestart-no-wrap.ll Sat May 28 19:32:17 2016
@@ -1,6 +1,6 @@
; ; RUN: opt -analyze -scalar-evolution < %s | FileCheck %s
-define void @infer.sext.0(i1* %c, i32 %start) {
+define void @infer.sext.0(i1* %c, i32 %start, i32* %buf) {
; CHECK-LABEL: Classifying expressions for: @infer.sext.0
entry:
br label %loop
@@ -12,6 +12,10 @@ define void @infer.sext.0(i1* %c, i32 %s
%idx.inc.sext = sext i32 %idx.inc to i64
; CHECK: %idx.inc.sext = sext i32 %idx.inc to i64
; CHECK-NEXT: --> {(1 + (sext i32 %start to i64))<nsw>,+,1}<nsw><%loop>
+
+ %buf.gep = getelementptr inbounds i32, i32* %buf, i32 %idx.inc
+ %val = load i32, i32* %buf.gep
+
%condition = icmp eq i32 %counter, 1
%counter.inc = add i32 %counter, 1
br i1 %condition, label %exit, label %loop
@@ -20,7 +24,7 @@ define void @infer.sext.0(i1* %c, i32 %s
ret void
}
-define void @infer.zext.0(i1* %c, i32 %start) {
+define void @infer.zext.0(i1* %c, i32 %start, i32* %buf) {
; CHECK-LABEL: Classifying expressions for: @infer.zext.0
entry:
br label %loop
@@ -32,6 +36,10 @@ define void @infer.zext.0(i1* %c, i32 %s
%idx.inc.sext = zext i32 %idx.inc to i64
; CHECK: %idx.inc.sext = zext i32 %idx.inc to i64
; CHECK-NEXT: --> {(1 + (zext i32 %start to i64))<nuw><nsw>,+,1}<nuw><%loop>
+
+ %buf.gep = getelementptr inbounds i32, i32* %buf, i32 %idx.inc
+ %val = load i32, i32* %buf.gep
+
%condition = icmp eq i32 %counter, 1
%counter.inc = add i32 %counter, 1
br i1 %condition, label %exit, label %loop
Modified: llvm/trunk/test/Analysis/ScalarEvolution/nowrap-preinc-limits.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/nowrap-preinc-limits.ll?rev=271151&r1=271150&r2=271151&view=diff
==============================================================================
--- llvm/trunk/test/Analysis/ScalarEvolution/nowrap-preinc-limits.ll (original)
+++ llvm/trunk/test/Analysis/ScalarEvolution/nowrap-preinc-limits.ll Sat May 28 19:32:17 2016
@@ -36,7 +36,8 @@ define void @g(i1* %condition) {
; CHECK: %idx.inc2.sext = sext i32 %idx.inc2 to i64
; CHECK-NEXT: --> {2,+,3}<nuw><nsw><%loop>
- %c = load volatile i1, i1* %condition
+ %cond.gep = getelementptr inbounds i1, i1* %condition, i32 %idx.inc
+ %c = load volatile i1, i1* %cond.gep
br i1 %c, label %loop, label %exit
exit:
Modified: llvm/trunk/test/Analysis/ScalarEvolution/nsw.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/nsw.ll?rev=271151&r1=271150&r2=271151&view=diff
==============================================================================
--- llvm/trunk/test/Analysis/ScalarEvolution/nsw.ll (original)
+++ llvm/trunk/test/Analysis/ScalarEvolution/nsw.ll Sat May 28 19:32:17 2016
@@ -30,15 +30,17 @@ bb: ; preds = %bb1, %bb.nph
%tmp8 = add nsw i32 %i.01, 1 ; <i32> [#uses=2]
; CHECK: %tmp8
; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%bb>
+ %p.gep = getelementptr double, double* %p, i32 %tmp8
+ %p.val = load double, double* %p.gep
br label %bb1
bb1: ; preds = %bb
%phitmp = sext i32 %tmp8 to i64 ; <i64> [#uses=1]
; CHECK: %phitmp
; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%bb>
- %tmp9 = getelementptr double, double* %p, i64 %phitmp ; <double*> [#uses=1]
+ %tmp9 = getelementptr inbounds double, double* %p, i64 %phitmp ; <double*> [#uses=1]
; CHECK: %tmp9
-; CHECK-NEXT: --> {(8 + %p),+,8}<%bb>
+; CHECK-NEXT: --> {(8 + %p)<nsw>,+,8}<nsw><%bb>
%tmp10 = load double, double* %tmp9, align 8 ; <double> [#uses=1]
%tmp11 = fcmp ogt double %tmp10, 2.000000e+00 ; <i1> [#uses=1]
br i1 %tmp11, label %bb, label %bb1.return_crit_edge
@@ -143,15 +145,15 @@ bb7:
}
; CHECK-LABEL: PR12376
-; CHECK: --> {(4 + %arg)<nsw>,+,4}<nuw><%bb2>{{ U: [^ ]+ S: [^ ]+}}{{ *}}Exits: (4 + (4 * ((3 + (-1 * %arg) + (%arg umax %arg1)) /u 4)) + %arg)
+; CHECK: --> {(4 + %arg)<nsw>,+,4}<nuw><%bb2>{{ U: [^ ]+ S: [^ ]+}}{{ *}}Exits: (4 + (4 * ((-1 + (-1 * %arg) + ((4 + %arg)<nsw> umax %arg1)) /u 4)) + %arg)
define void @PR12376(i32* nocapture %arg, i32* nocapture %arg1) {
bb:
br label %bb2
bb2: ; preds = %bb2, %bb
%tmp = phi i32* [ %arg, %bb ], [ %tmp4, %bb2 ]
- %tmp3 = icmp ult i32* %tmp, %arg1
%tmp4 = getelementptr inbounds i32, i32* %tmp, i64 1
+ %tmp3 = icmp ult i32* %tmp4, %arg1
br i1 %tmp3, label %bb2, label %bb5
bb5: ; preds = %bb2
@@ -161,8 +163,8 @@ bb5:
declare void @f(i32)
; CHECK-LABEL: nswnowrap
-; CHECK: --> {(1 + %v),+,1}<nsw><%for.body>{{ U: [^ ]+ S: [^ ]+}}{{ *}}Exits: (2 + %v)
-define void @nswnowrap(i32 %v) {
+; CHECK: --> {(1 + %v)<nsw>,+,1}<nsw><%for.body>{{ U: [^ ]+ S: [^ ]+}}{{ *}}Exits: (2 + %v)
+define void @nswnowrap(i32 %v, i32* %buf) {
entry:
%add = add nsw i32 %v, 1
br label %for.body
@@ -170,8 +172,10 @@ entry:
for.body:
%i.04 = phi i32 [ %v, %entry ], [ %inc, %for.body ]
%inc = add nsw i32 %i.04, 1
- tail call void @f(i32 %i.04)
+ %buf.gep = getelementptr inbounds i32, i32* %buf, i32 %inc
+ %buf.val = load i32, i32* %buf.gep
%cmp = icmp slt i32 %i.04, %add
+ tail call void @f(i32 %i.04)
br i1 %cmp, label %for.body, label %for.end
for.end:
Added: llvm/trunk/test/Analysis/ScalarEvolution/pr27315.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/pr27315.ll?rev=271151&view=auto
==============================================================================
--- llvm/trunk/test/Analysis/ScalarEvolution/pr27315.ll (added)
+++ llvm/trunk/test/Analysis/ScalarEvolution/pr27315.ll Sat May 28 19:32:17 2016
@@ -0,0 +1,31 @@
+; RUN: opt -analyze -scalar-evolution < %s | FileCheck %s
+
+declare i1 @use(i64)
+
+define void @f_0() {
+; CHECK-LABEL: Classifying expressions for: @f_0
+
+; CHECK: %iv = phi i32 [ 0, %entry ], [ %iv.inc.nowrap, %be ]
+; CHECK-NEXT: --> {0,+,1}<nuw><nsw><%loop>
+; CHECK: %iv.inc.maywrap = add i32 %iv, 1
+; CHECK-NEXT: --> {1,+,1}<nuw><%loop>
+; CHECK: %iv.inc.maywrap.sext = sext i32 %iv.inc.maywrap to i64
+; CHECK-NEXT: --> (sext i32 {1,+,1}<nuw><%loop> to i64)
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.inc.nowrap, %be ]
+ %iv.inc.maywrap = add i32 %iv, 1
+ %iv.inc.maywrap.sext = sext i32 %iv.inc.maywrap to i64
+ %cond0 = call i1 @use(i64 %iv.inc.maywrap.sext)
+ br i1 %cond0, label %be, label %leave
+
+be:
+ %iv.inc.nowrap = add nsw i32 %iv, 1
+ %be.cond = call i1 @use(i64 0) ;; Get an unanalyzable value
+ br i1 %be.cond, label %loop, label %leave
+
+leave:
+ ret void
+}
Modified: llvm/trunk/test/Transforms/IndVarSimplify/elim-extend.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/elim-extend.ll?rev=271151&r1=271150&r2=271151&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/IndVarSimplify/elim-extend.ll (original)
+++ llvm/trunk/test/Transforms/IndVarSimplify/elim-extend.ll Sat May 28 19:32:17 2016
@@ -22,7 +22,7 @@ loop:
store i8 0, i8* %postadr
%postivnsw = add nsw i32 %ivnsw, 1
%postofsnsw = sext i32 %postivnsw to i64
- %postadrnsw = getelementptr i8, i8* %base, i64 %postofsnsw
+ %postadrnsw = getelementptr inbounds i8, i8* %base, i64 %postofsnsw
store i8 0, i8* %postadrnsw
%cond = icmp sgt i32 %limit, %iv
br i1 %cond, label %loop, label %exit
More information about the llvm-commits
mailing list