From ecb35ece5c42d89057da3c2c7bc2e95f08b1dbef Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Tue, 29 Nov 2011 02:16:38 +0000 Subject: [PATCH] SCEV fix. In general, Add/Mul expressions should not inherit NSW/NUW. This reverts r139450, fixes r139453, and adds much needed comments and a unit test. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@145367 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/ScalarEvolution.cpp | 16 ++++++++-------- lib/Transforms/Scalar/IndVarSimplify.cpp | 8 ++++++-- test/Analysis/ScalarEvolution/nsw.ll | 20 +++++++++++++++++++- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/lib/Analysis/ScalarEvolution.cpp b/lib/Analysis/ScalarEvolution.cpp index a054801fb88..ff85906a5e7 100644 --- a/lib/Analysis/ScalarEvolution.cpp +++ b/lib/Analysis/ScalarEvolution.cpp @@ -3592,6 +3592,12 @@ const SCEV *ScalarEvolution::createSCEV(Value *V) { // because it leads to N-1 getAddExpr calls for N ultimate operands. // Instead, gather up all the operands and make a single getAddExpr call. // LLVM IR canonical form means we need only traverse the left operands. + // + // Don't apply this instruction's NSW or NUW flags to the new + // expression. The instruction may be guarded by control flow that the + // no-wrap behavior depends on. Non-control-equivalent instructions can be + // mapped to the same SCEV expression, and it would be incorrect to transfer + // NSW/NUW semantics to those operations. SmallVector AddOps; AddOps.push_back(getSCEV(U->getOperand(1))); for (Value *Op = U->getOperand(0); ; Op = U->getOperand(0)) { @@ -3606,16 +3612,10 @@ const SCEV *ScalarEvolution::createSCEV(Value *V) { AddOps.push_back(Op1); } AddOps.push_back(getSCEV(U->getOperand(0))); - SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap; - OverflowingBinaryOperator *OBO = cast(V); - if (OBO->hasNoSignedWrap()) - Flags = setFlags(Flags, SCEV::FlagNSW); - if (OBO->hasNoUnsignedWrap()) - Flags = setFlags(Flags, SCEV::FlagNUW); - return getAddExpr(AddOps, Flags); + return getAddExpr(AddOps); } case Instruction::Mul: { - // See the Add code above. + // Don't transfer NSW/NUW for the same reason as AddExpr. SmallVector MulOps; MulOps.push_back(getSCEV(U->getOperand(1))); for (Value *Op = U->getOperand(0); diff --git a/lib/Transforms/Scalar/IndVarSimplify.cpp b/lib/Transforms/Scalar/IndVarSimplify.cpp index 1f5ba51824a..ea083e36ca4 100644 --- a/lib/Transforms/Scalar/IndVarSimplify.cpp +++ b/lib/Transforms/Scalar/IndVarSimplify.cpp @@ -944,9 +944,13 @@ const SCEVAddRecExpr* WidenIV::GetExtendedOperandRecurrence(NarrowIVDefUse DU) { else return 0; + // When creating this AddExpr, don't apply the current operations NSW or NUW + // flags. This instruction may be guarded by control flow that the no-wrap + // behavior depends on. Non-control-equivalent instructions can be mapped to + // the same SCEV expression, and it would be incorrect to transfer NSW/NUW + // semantics to those operations. const SCEVAddRecExpr *AddRec = dyn_cast( - SE->getAddExpr(SE->getSCEV(DU.WideDef), ExtendOperExpr, - IsSigned ? SCEV::FlagNSW : SCEV::FlagNUW)); + SE->getAddExpr(SE->getSCEV(DU.WideDef), ExtendOperExpr)); if (!AddRec || AddRec->getLoop() != L) return 0; diff --git a/test/Analysis/ScalarEvolution/nsw.ll b/test/Analysis/ScalarEvolution/nsw.ll index da35a6cf7ae..659cf4f8da9 100644 --- a/test/Analysis/ScalarEvolution/nsw.ll +++ b/test/Analysis/ScalarEvolution/nsw.ll @@ -103,4 +103,22 @@ for.body.i.i: ; preds = %entry, %for.body.i. ; CHECK: Loop %for.body.i.i: max backedge-taken count is ((-4 + (-1 * %begin) + %end) /u 4) _ZSt4fillIPiiEvT_S1_RKT0_.exit: ; preds = %for.body.i.i, %entry ret void -} \ No newline at end of file +} + +; A single AddExpr exists for (%a + %b), which is not always . +; CHECK: @addnsw +; CHECK-NOT: --> (%a + %b) +define i32 @addnsw(i32 %a, i32 %b) nounwind ssp { +entry: + %tmp = add i32 %a, %b + %cmp = icmp sgt i32 %tmp, 0 + br i1 %cmp, label %greater, label %exit + +greater: + %tmp2 = add nsw i32 %a, %b + br label %exit + +exit: + %result = phi i32 [ %a, %entry ], [ %tmp2, %greater ] + ret i32 %result +}