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