[llvm] r294181 - [SCEV] limit recursion depth and operands number in getAddExpr
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 6 11:22:22 PST 2017
This adds a unittest that *with optimizations* takes over 20 seconds to
run. That seems excessive.
On Mon, Feb 6, 2017 at 4:49 AM Daniil Fukalov via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: dfukalov
> Date: Mon Feb 6 06:38:06 2017
> New Revision: 294181
>
> URL: http://llvm.org/viewvc/llvm-project?rev=294181&view=rev
> Log:
> [SCEV] limit recursion depth and operands number in getAddExpr
>
> for a quite big function with source like
>
> %add = add nsw i32 %mul, %conv
> %mul1 = mul nsw i32 %add, %conv
> %add2 = add nsw i32 %mul1, %add
> %mul3 = mul nsw i32 %add2, %add
> ; repeat couple of thousands times
> that can be produced by loop unroll, getAddExpr() tries to recursively
> construct SCEV and runs almost infinite time.
>
> Added recursion depth restriction (with new parameter to set it)
>
> Reviewers: sanjoy
>
> Subscribers: hfinkel, llvm-commits, mzolotukhin
>
> Differential Revision: https://reviews.llvm.org/D28158
>
> Modified:
> llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
> llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp
>
> Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolution.h?rev=294181&r1=294180&r2=294181&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/ScalarEvolution.h (original)
> +++ llvm/trunk/include/llvm/Analysis/ScalarEvolution.h Mon Feb 6 06:38:06
> 2017
> @@ -1140,7 +1140,8 @@ public:
> const SCEV *getSignExtendExpr(const SCEV *Op, Type *Ty);
> const SCEV *getAnyExtendExpr(const SCEV *Op, Type *Ty);
> const SCEV *getAddExpr(SmallVectorImpl<const SCEV *> &Ops,
> - SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap);
> + SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap,
> + unsigned Depth = 0);
> const SCEV *getAddExpr(const SCEV *LHS, const SCEV *RHS,
> SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap) {
> SmallVector<const SCEV *, 2> Ops = {LHS, RHS};
> @@ -1613,6 +1614,10 @@ private:
> bool doesIVOverflowOnGT(const SCEV *RHS, const SCEV *Stride, bool
> IsSigned,
> bool NoWrap);
>
> + /// Get add expr already created or create a new one
> + const SCEV *getOrCreateAddExpr(SmallVectorImpl<const SCEV *> &Ops,
> + SCEV::NoWrapFlags Flags);
> +
> private:
> FoldingSet<SCEV> UniqueSCEVs;
> FoldingSet<SCEVPredicate> UniquePreds;
>
> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=294181&r1=294180&r2=294181&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Mon Feb 6 06:38:06 2017
> @@ -137,6 +137,11 @@ static cl::opt<unsigned>
> cl::desc("Maximum depth of recursive compare
> complexity"),
> cl::init(32));
>
> +static cl::opt<unsigned>
> + MaxAddExprDepth("scalar-evolution-max-addexpr-depth", cl::Hidden,
> + cl::desc("Maximum depth of recursive AddExpr"),
> + cl::init(32));
> +
> static cl::opt<unsigned> MaxConstantEvolvingDepth(
> "scalar-evolution-max-constant-evolving-depth", cl::Hidden,
> cl::desc("Maximum depth of recursive constant evolving"),
> cl::init(32));
> @@ -2100,7 +2105,8 @@ StrengthenNoWrapFlags(ScalarEvolution *S
>
> /// Get a canonical add expression, or something simpler if possible.
> const SCEV *ScalarEvolution::getAddExpr(SmallVectorImpl<const SCEV *>
> &Ops,
> - SCEV::NoWrapFlags Flags) {
> + SCEV::NoWrapFlags Flags,
> + unsigned Depth) {
> assert(!(Flags & ~(SCEV::FlagNUW | SCEV::FlagNSW)) &&
> "only nuw or nsw allowed");
> assert(!Ops.empty() && "Cannot get empty add!");
> @@ -2139,6 +2145,10 @@ const SCEV *ScalarEvolution::getAddExpr(
> if (Ops.size() == 1) return Ops[0];
> }
>
> + // Limit recursion calls depth
> + if (Depth > MaxAddExprDepth)
> + return getOrCreateAddExpr(Ops, Flags);
> +
> // Okay, check to see if the same value occurs in the operand list more
> than
> // once. If so, merge them together into an multiply expression.
> Since we
> // sorted the list, these values are required to be adjacent.
> @@ -2210,7 +2220,7 @@ const SCEV *ScalarEvolution::getAddExpr(
> }
> if (Ok) {
> // Evaluate the expression in the larger type.
> - const SCEV *Fold = getAddExpr(LargeOps, Flags);
> + const SCEV *Fold = getAddExpr(LargeOps, Flags, Depth + 1);
> // If it folds to something simple, use it. Otherwise, don't.
> if (isa<SCEVConstant>(Fold) || isa<SCEVUnknown>(Fold))
> return getTruncateExpr(Fold, DstType);
> @@ -2239,7 +2249,7 @@ const SCEV *ScalarEvolution::getAddExpr(
> // and they are not necessarily sorted. Recurse to resort and
> resimplify
> // any operands we just acquired.
> if (DeletedAdd)
> - return getAddExpr(Ops);
> + return getAddExpr(Ops, SCEV::FlagAnyWrap, Depth + 1);
> }
>
> // Skip over the add expression until we get to a multiply.
> @@ -2274,13 +2284,14 @@ const SCEV *ScalarEvolution::getAddExpr(
> Ops.push_back(getConstant(AccumulatedConstant));
> for (auto &MulOp : MulOpLists)
> if (MulOp.first != 0)
> - Ops.push_back(getMulExpr(getConstant(MulOp.first),
> - getAddExpr(MulOp.second)));
> + Ops.push_back(getMulExpr(
> + getConstant(MulOp.first),
> + getAddExpr(MulOp.second, SCEV::FlagAnyWrap, Depth + 1)));
> if (Ops.empty())
> return getZero(Ty);
> if (Ops.size() == 1)
> return Ops[0];
> - return getAddExpr(Ops);
> + return getAddExpr(Ops, SCEV::FlagAnyWrap, Depth + 1);
> }
> }
>
> @@ -2305,8 +2316,8 @@ const SCEV *ScalarEvolution::getAddExpr(
> MulOps.append(Mul->op_begin()+MulOp+1, Mul->op_end());
> InnerMul = getMulExpr(MulOps);
> }
> - const SCEV *One = getOne(Ty);
> - const SCEV *AddOne = getAddExpr(One, InnerMul);
> + SmallVector<const SCEV *, 2> TwoOps = {getOne(Ty), InnerMul};
> + const SCEV *AddOne = getAddExpr(TwoOps, SCEV::FlagAnyWrap,
> Depth + 1);
> const SCEV *OuterMul = getMulExpr(AddOne, MulOpSCEV);
> if (Ops.size() == 2) return OuterMul;
> if (AddOp < Idx) {
> @@ -2317,7 +2328,7 @@ const SCEV *ScalarEvolution::getAddExpr(
> Ops.erase(Ops.begin()+AddOp-1);
> }
> Ops.push_back(OuterMul);
> - return getAddExpr(Ops);
> + return getAddExpr(Ops, SCEV::FlagAnyWrap, Depth + 1);
> }
>
> // Check this multiply against other multiplies being added
> together.
> @@ -2345,13 +2356,15 @@ const SCEV *ScalarEvolution::getAddExpr(
> MulOps.append(OtherMul->op_begin()+OMulOp+1,
> OtherMul->op_end());
> InnerMul2 = getMulExpr(MulOps);
> }
> - const SCEV *InnerMulSum = getAddExpr(InnerMul1,InnerMul2);
> + SmallVector<const SCEV *, 2> TwoOps = {InnerMul1, InnerMul2};
> + const SCEV *InnerMulSum =
> + getAddExpr(TwoOps, SCEV::FlagAnyWrap, Depth + 1);
> const SCEV *OuterMul = getMulExpr(MulOpSCEV, InnerMulSum);
> if (Ops.size() == 2) return OuterMul;
> Ops.erase(Ops.begin()+Idx);
> Ops.erase(Ops.begin()+OtherMulIdx-1);
> Ops.push_back(OuterMul);
> - return getAddExpr(Ops);
> + return getAddExpr(Ops, SCEV::FlagAnyWrap, Depth + 1);
> }
> }
> }
> @@ -2387,7 +2400,7 @@ const SCEV *ScalarEvolution::getAddExpr(
> // This follows from the fact that the no-wrap flags on the outer
> add
> // expression are applicable on the 0th iteration, when the add
> recurrence
> // will be equal to its start value.
> - AddRecOps[0] = getAddExpr(LIOps, Flags);
> + AddRecOps[0] = getAddExpr(LIOps, Flags, Depth + 1);
>
> // Build the new addrec. Propagate the NUW and NSW flags if both the
> // outer add and the inner addrec are guaranteed to have no
> overflow.
> @@ -2404,7 +2417,7 @@ const SCEV *ScalarEvolution::getAddExpr(
> Ops[i] = NewRec;
> break;
> }
> - return getAddExpr(Ops);
> + return getAddExpr(Ops, SCEV::FlagAnyWrap, Depth + 1);
> }
>
> // Okay, if there weren't any loop invariants to be folded, check to
> see if
> @@ -2428,14 +2441,15 @@ const SCEV *ScalarEvolution::getAddExpr(
> OtherAddRec->op_end());
> break;
> }
> - AddRecOps[i] = getAddExpr(AddRecOps[i],
> - OtherAddRec->getOperand(i));
> + SmallVector<const SCEV *, 2> TwoOps = {
> + AddRecOps[i], OtherAddRec->getOperand(i)};
> + AddRecOps[i] = getAddExpr(TwoOps, SCEV::FlagAnyWrap,
> Depth + 1);
> }
> Ops.erase(Ops.begin() + OtherIdx); --OtherIdx;
> }
> // Step size has changed, so we cannot guarantee no
> self-wraparound.
> Ops[Idx] = getAddRecExpr(AddRecOps, AddRecLoop,
> SCEV::FlagAnyWrap);
> - return getAddExpr(Ops);
> + return getAddExpr(Ops, SCEV::FlagAnyWrap, Depth + 1);
> }
>
> // Otherwise couldn't fold anything into this recurrence. Move onto
> the
> @@ -2444,18 +2458,24 @@ const SCEV *ScalarEvolution::getAddExpr(
>
> // Okay, it looks like we really DO need an add expr. Check to see if
> we
> // already have one, otherwise create a new one.
> + return getOrCreateAddExpr(Ops, Flags);
> +}
> +
> +const SCEV *
> +ScalarEvolution::getOrCreateAddExpr(SmallVectorImpl<const SCEV *> &Ops,
> + SCEV::NoWrapFlags Flags) {
> FoldingSetNodeID ID;
> ID.AddInteger(scAddExpr);
> for (unsigned i = 0, e = Ops.size(); i != e; ++i)
> ID.AddPointer(Ops[i]);
> void *IP = nullptr;
> SCEVAddExpr *S =
> - static_cast<SCEVAddExpr *>(UniqueSCEVs.FindNodeOrInsertPos(ID, IP));
> + static_cast<SCEVAddExpr *>(UniqueSCEVs.FindNodeOrInsertPos(ID, IP));
> if (!S) {
> const SCEV **O = SCEVAllocator.Allocate<const SCEV *>(Ops.size());
> std::uninitialized_copy(Ops.begin(), Ops.end(), O);
> - S = new (SCEVAllocator) SCEVAddExpr(ID.Intern(SCEVAllocator),
> - O, Ops.size());
> + S = new (SCEVAllocator)
> + SCEVAddExpr(ID.Intern(SCEVAllocator), O, Ops.size());
> UniqueSCEVs.InsertNode(S, IP);
> }
> S->setNoWrapFlags(Flags);
>
> Modified: llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp?rev=294181&r1=294180&r2=294181&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp (original)
> +++ llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp Mon Feb 6
> 06:38:06 2017
> @@ -532,5 +532,33 @@ TEST_F(ScalarEvolutionsTest, SCEVCompare
> EXPECT_NE(nullptr, SE.getSCEV(Acc[0]));
> }
>
> +TEST_F(ScalarEvolutionsTest, SCEVAddExpr) {
> + Type *Ty32 = Type::getInt32Ty(Context);
> + Type *ArgTys[] = {Type::getInt64Ty(Context), Ty32};
> +
> + FunctionType *FTy =
> + FunctionType::get(Type::getVoidTy(Context), ArgTys, false);
> + Function *F = cast<Function>(M.getOrInsertFunction("f", FTy));
> +
> + Argument *A1 = &*F->arg_begin();
> + Argument *A2 = &*(std::next(F->arg_begin()));
> + BasicBlock *EntryBB = BasicBlock::Create(Context, "entry", F);
> +
> + Instruction *Trunc = CastInst::CreateTruncOrBitCast(A1, Ty32, "",
> EntryBB);
> + Instruction *Mul1 = BinaryOperator::CreateMul(Trunc, A2, "", EntryBB);
> + Instruction *Add1 = BinaryOperator::CreateAdd(Mul1, Trunc, "", EntryBB);
> + Mul1 = BinaryOperator::CreateMul(Add1, Trunc, "", EntryBB);
> + Instruction *Add2 = BinaryOperator::CreateAdd(Mul1, Add1, "", EntryBB);
> + for (int i = 0; i < 1000; i++) {
> + Mul1 = BinaryOperator::CreateMul(Add2, Add1, "", EntryBB);
> + Add1 = Add2;
> + Add2 = BinaryOperator::CreateAdd(Mul1, Add1, "", EntryBB);
> + }
> +
> + ReturnInst::Create(Context, nullptr, EntryBB);
> + ScalarEvolution SE = buildSE(*F);
> + EXPECT_NE(nullptr, SE.getSCEV(Mul1));
> +}
> +
> } // end anonymous namespace
> } // end namespace llvm
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170206/36683a85/attachment.html>
More information about the llvm-commits
mailing list