[llvm] r233447 - [SCEV] Look at backedge dominating conditions.
Sanjoy Das
sanjoy at playingwithpointers.com
Mon Mar 30 14:38:40 PDT 2015
Hi Daniel,
It tried compiling all *.cpp files in llvm/lib/Analysis/ (just as a
corpus of interesting C++ code) with -fsanitize=memory, and did not
see any visible difference between the change reverted vs. the change
unreverted. I'll find it very helpful if you can please send me a
sample cpp/c file (preprocessed preferably) that exhibits the
compile-time issue you mention -- there are many things that could be
going wrong and I don't want to waste too much time "fixing" the wrong
issue. :)
Having said that, if you have a way to easily reproduce the problem,
can you please try applying the attached patch and check if you still
see excessively slow compiles? The fix is an almost blind guess, but
it might just do the trick.
> The root cause seems to be the repeated recursion:
>
> ...
> llvm::ScalarEvolution::getZeroExtendExpr(llvm::SCEV const*, llvm::Type*) ()
> llvm::ScalarEvolution::isImpliedCond(llvm::CmpInst::Predicate, llvm::SCEV const*, llvm::SCEV const*, llvm::Value*, bool) ()
> llvm::ScalarEvolution::isLoopBackedgeGuardedByCond(llvm::Loop const*, llvm::CmpInst::Predicate, llvm::SCEV const*, llvm::SCEV > const*) ()
> llvm::ScalarEvolution::getZeroExtendExpr(llvm::SCEV const*, llvm::Type*) ()
> llvm::ScalarEvolution::isImpliedCond(llvm::CmpInst::Predicate, llvm::SCEV const*, llvm::SCEV const*, llvm::Value*, bool) ()
> llvm::ScalarEvolution::isLoopBackedgeGuardedByCond(llvm::Loop const*, llvm::CmpInst::Predicate, llvm::SCEV const*, llvm::SCEV > const*) ()
> ...
By repeated do you mean the recursion itself is too deeply nested? Or
that you repeatedly see this pattern on the stack?
Thanks,
-- Sanjoy
-------------- next part --------------
commit 6c968806c7c8057e6293872c8ba6612958cb80bd
Author: Sanjoy Das <sanjoy at playingwithpointers.com>
Date: Fri Mar 27 23:18:08 2015 +0000
[SCEV] Look at backedge dominating conditions.
Summary:
This change teaches ScalarEvolution::isLoopBackedgeGuardedByCond to look
at edges within the loop body that dominate the latch. We don't do an
exhaustive search for all possible edges, but only a quick walk up the
dom tree.
Re-applies 233447 with a compile-time fix.
Reviewers: atrick, hfinkel
Subscribers: llvm-commits
Differential Revision: http://reviews.llvm.org/D8627
diff --git a/lib/Analysis/ScalarEvolution.cpp b/lib/Analysis/ScalarEvolution.cpp
index 4e713fb..d9c9694 100644
--- a/lib/Analysis/ScalarEvolution.cpp
+++ b/lib/Analysis/ScalarEvolution.cpp
@@ -6661,6 +6661,24 @@ ScalarEvolution::isKnownPredicateWithRanges(ICmpInst::Predicate Pred,
return false;
}
+/// RAII wrapper to prevent recursive application of isImpliedCond.
+/// ScalarEvolution's PendingLoopPredicates set must be empty unless we are
+/// currently evaluating isImpliedCond.
+struct MarkPendingLoopPredicate {
+ Value *Cond;
+ DenseSet<Value*> &LoopPreds;
+ bool Pending;
+
+ MarkPendingLoopPredicate(Value *C, DenseSet<Value*> &LP)
+ : Cond(C), LoopPreds(LP) {
+ Pending = !LoopPreds.insert(Cond).second;
+ }
+ ~MarkPendingLoopPredicate() {
+ if (!Pending)
+ LoopPreds.erase(Cond);
+ }
+};
+
/// isLoopBackedgeGuardedByCond - Test whether the backedge of the loop is
/// protected by a conditional between LHS and RHS. This is used to
/// to eliminate casts.
@@ -6686,6 +6704,49 @@ ScalarEvolution::isLoopBackedgeGuardedByCond(const Loop *L,
LoopContinuePredicate->getSuccessor(0) != L->getHeader()))
return true;
+ // If the loop is not reachable from the entry block, we risk running into an
+ // infinite loop as we walk up into the dom tree. These loops do not matter
+ // anyway, so we just return a conservative answer when we see them.
+ if (!DT->isReachableFromEntry(L->getHeader()))
+ return false;
+
+ std::vector<MarkPendingLoopPredicate> PendingPredicates;
+
+ for (DomTreeNode *DTN = (*DT)[Latch], *HeaderDTN = (*DT)[L->getHeader()];
+ DTN != HeaderDTN;
+ DTN = DTN->getIDom()) {
+
+ assert(DTN && "should reach the loop header before reaching the root!");
+
+ BasicBlock *BB = DTN->getBlock();
+ BasicBlock *PBB = BB->getSinglePredecessor();
+ if (!PBB)
+ continue;
+
+ BranchInst *ContinuePredicate = dyn_cast<BranchInst>(PBB->getTerminator());
+ if (!ContinuePredicate || !ContinuePredicate->isConditional())
+ continue;
+
+ Value *Condition = ContinuePredicate->getCondition();
+
+ // If we have an edge `E` within the loop body that dominates the only
+ // latch, the condition guarding `E` also guards the backedge. This
+ // reasoning works only for loops with a single latch.
+
+ BasicBlockEdge DominatingEdge(PBB, BB);
+ if (DominatingEdge.isSingleEdge()) {
+ // We're constructively (and conservatively) enumerating edges within the
+ // loop body that dominate the latch. The dominator tree better agree
+ // with us on this:
+ assert(DT->dominates(DominatingEdge, Latch) && "should be!");
+
+ if (isImpliedCond(Pred, LHS, RHS, Condition,
+ BB != ContinuePredicate->getSuccessor(0)))
+ return true;
+ }
+ PendingPredicates.emplace_back(Condition, PendingLoopPredicates);
+ }
+
// Check conditions due to any @llvm.assume intrinsics.
for (auto &AssumeVH : AC->assumptions()) {
if (!AssumeVH)
@@ -6749,24 +6810,6 @@ ScalarEvolution::isLoopEntryGuardedByCond(const Loop *L,
return false;
}
-/// RAII wrapper to prevent recursive application of isImpliedCond.
-/// ScalarEvolution's PendingLoopPredicates set must be empty unless we are
-/// currently evaluating isImpliedCond.
-struct MarkPendingLoopPredicate {
- Value *Cond;
- DenseSet<Value*> &LoopPreds;
- bool Pending;
-
- MarkPendingLoopPredicate(Value *C, DenseSet<Value*> &LP)
- : Cond(C), LoopPreds(LP) {
- Pending = !LoopPreds.insert(Cond).second;
- }
- ~MarkPendingLoopPredicate() {
- if (!Pending)
- LoopPreds.erase(Cond);
- }
-};
-
/// isImpliedCond - Test whether the condition described by Pred, LHS,
/// and RHS is true whenever the given Cond value evaluates to true.
bool ScalarEvolution::isImpliedCond(ICmpInst::Predicate Pred,
diff --git a/test/Analysis/ScalarEvolution/latch-dominating-conditions.ll b/test/Analysis/ScalarEvolution/latch-dominating-conditions.ll
new file mode 100644
index 0000000..3f6f958
--- /dev/null
+++ b/test/Analysis/ScalarEvolution/latch-dominating-conditions.ll
@@ -0,0 +1,55 @@
+; RUN: opt -S -indvars < %s | FileCheck %s
+
+declare void @side_effect(i1)
+
+define void @latch_dominating_0(i8 %start) {
+; CHECK-LABEL: latch_dominating_0
+ entry:
+ %e = icmp slt i8 %start, 42
+ br i1 %e, label %loop, label %exit
+
+ loop:
+; CHECK-LABEL: loop
+ %idx = phi i8 [ %start, %entry ], [ %idx.inc, %be ]
+ %idx.inc = add i8 %idx, 1
+ %folds.to.true = icmp slt i8 %idx, 42
+; CHECK: call void @side_effect(i1 true)
+ call void @side_effect(i1 %folds.to.true)
+ %c0 = icmp slt i8 %idx.inc, 42
+ br i1 %c0, label %be, label %exit
+
+ be:
+; CHECK: call void @side_effect(i1 true)
+ call void @side_effect(i1 %folds.to.true)
+ %c1 = icmp slt i8 %idx.inc, 100
+ br i1 %c1, label %loop, label %exit
+
+ exit:
+ ret void
+}
+
+define void @latch_dominating_1(i8 %start) {
+; CHECK-LABEL: latch_dominating_1
+ entry:
+ %e = icmp slt i8 %start, 42
+ br i1 %e, label %loop, label %exit
+
+ loop:
+; CHECK-LABEL: loop
+ %idx = phi i8 [ %start, %entry ], [ %idx.inc, %be ]
+ %idx.inc = add i8 %idx, 1
+ %does.not.fold.to.true = icmp slt i8 %idx, 42
+; CHECK: call void @side_effect(i1 %does.not.fold.to.true)
+ call void @side_effect(i1 %does.not.fold.to.true)
+ %c0 = icmp slt i8 %idx.inc, 42
+ br i1 %c0, label %be, label %be
+
+ be:
+; CHECK: call void @side_effect(i1 %does.not.fold.to.true)
+ call void @side_effect(i1 %does.not.fold.to.true)
+ %c1 = icmp slt i8 %idx.inc, 100
+ br i1 %c1, label %loop, label %exit
+
+ exit:
+ ret void
+}
More information about the llvm-commits
mailing list