From 31713943aac87baba0792c250a606b33eda36fbe Mon Sep 17 00:00:00 2001 From: Zinovy Nis Date: Thu, 2 Oct 2014 13:01:15 +0000 Subject: [PATCH] [BUG][INDVAR] Fix for PR21014: wrong SCEV operands commuting for non-commutative instructions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My commit rL216160 introduced a bug PR21014: IndVars widens code 'for (i = ; i < ...; i++) arr[ CONST - i]' into 'for (i = ; i < ...; i++) arr[ i - CONST]' thus inverting index expression. This patch fixes it. Thanks to Jörg Sonnenberger for pointing. Differential Revision: http://reviews.llvm.org/D5576 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@218867 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/IndVarSimplify.cpp | 15 ++++++++++++--- .../IndVarSimplify/2011-09-10-widen-nsw.ll | 8 ++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/Transforms/Scalar/IndVarSimplify.cpp b/lib/Transforms/Scalar/IndVarSimplify.cpp index 2959c7bd3c7..03dffb66f35 100644 --- a/lib/Transforms/Scalar/IndVarSimplify.cpp +++ b/lib/Transforms/Scalar/IndVarSimplify.cpp @@ -865,7 +865,8 @@ const SCEVAddRecExpr* WidenIV::GetExtendedOperandRecurrence(NarrowIVDefUse DU) { // One operand (NarrowDef) has already been extended to WideDef. Now determine // if extending the other will lead to a recurrence. - unsigned ExtendOperIdx = DU.NarrowUse->getOperand(0) == DU.NarrowDef ? 1 : 0; + const unsigned ExtendOperIdx = + DU.NarrowUse->getOperand(0) == DU.NarrowDef ? 1 : 0; assert(DU.NarrowUse->getOperand(1-ExtendOperIdx) == DU.NarrowDef && "bad DU"); const SCEV *ExtendOperExpr = nullptr; @@ -885,8 +886,16 @@ const SCEVAddRecExpr* WidenIV::GetExtendedOperandRecurrence(NarrowIVDefUse DU) { // 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( - GetSCEVByOpCode(SE->getSCEV(DU.WideDef), ExtendOperExpr, OpCode)); + const SCEV *lhs = SE->getSCEV(DU.WideDef); + const SCEV *rhs = ExtendOperExpr; + + // Let's swap operands to the initial order for the case of non-commutative + // operations, like SUB. See PR21014. + if (ExtendOperIdx == 0) + std::swap(lhs, rhs); + const SCEVAddRecExpr *AddRec = + dyn_cast(GetSCEVByOpCode(lhs, rhs, OpCode)); + if (!AddRec || AddRec->getLoop() != L) return nullptr; return AddRec; diff --git a/test/Transforms/IndVarSimplify/2011-09-10-widen-nsw.ll b/test/Transforms/IndVarSimplify/2011-09-10-widen-nsw.ll index 477d15251c6..64fef10463f 100644 --- a/test/Transforms/IndVarSimplify/2011-09-10-widen-nsw.ll +++ b/test/Transforms/IndVarSimplify/2011-09-10-widen-nsw.ll @@ -19,7 +19,8 @@ for.body153: ; preds = %for.body153, %for.b ; CHECK: add nsw i64 %indvars.iv, 1 ; CHECK: sub nsw i64 %indvars.iv, 2 -; CHECK: mul nsw i64 %indvars.iv, 4 +; CHECK: sub nsw i64 4, %indvars.iv +; CHECK: mul nsw i64 %indvars.iv, 8 for.body170: ; preds = %for.body170, %for.body153 %i2.19 = phi i32 [ %add249, %for.body170 ], [ 0, %for.body153 ] @@ -29,7 +30,10 @@ for.body170: ; preds = %for.body170, %for.b %sub = sub nsw i32 %i2.19, 2 %sub.idxprom = sext i32 %sub to i64 - %mul = mul nsw i32 %i2.19, 4 + %sub.neg = sub nsw i32 4, %i2.19 + %sub.neg.idxprom = sext i32 %sub.neg to i64 + + %mul = mul nsw i32 %i2.19, 8 %mul.idxprom = sext i32 %mul to i64 %add249 = add nsw i32 %i2.19, %shl132