[llvm] 278299b - [SCCP] Reduce the number of times ResolvedUndefsIn is called for large modules.
Eli Friedman via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 9 15:26:46 PDT 2020
Author: Eli Friedman
Date: 2020-10-09T15:24:16-07:00
New Revision: 278299b0f0b0e47df95d526c95b42fa69667295c
URL: https://github.com/llvm/llvm-project/commit/278299b0f0b0e47df95d526c95b42fa69667295c
DIFF: https://github.com/llvm/llvm-project/commit/278299b0f0b0e47df95d526c95b42fa69667295c.diff
LOG: [SCCP] Reduce the number of times ResolvedUndefsIn is called for large modules.
If a module has many values that need to be resolved by
ResolvedUndefsIn, compilation takes quadratic time overall. Solve should
do a small amount of work, since not much is added to the worklists each
time markOverdefined is called. But ResolvedUndefsIn is linear over the
length of the function/module, so resolving one undef at a time is
quadratic in general.
To solve this, make ResolvedUndefsIn resolve every undef value at once,
instead of resolving them one at a time. This loses a little
optimization power, but can be a lot faster.
We still need a loop around ResolvedUndefsIn because markOverdefined
could change the set of blocks that are live. That should be uncommon,
hopefully. We could optimize it by tracking which blocks transition from
dead to live, instead of iterating over the whole module to find them.
But I'll leave that for later. (The whole function will become a lot
simpler once we start pruning branches on undef.)
The regression test changes seem minor. The specific cases in question
could probably be optimized with a bit more work, but they seem like
edge cases that don't really matter.
Fixes an "infinite" compile issue my team found on an internal workoad.
Differential Revision: https://reviews.llvm.org/D89080
Added:
Modified:
llvm/lib/Transforms/Scalar/SCCP.cpp
llvm/test/Transforms/SCCP/2004-12-10-UndefBranchBug.ll
llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp
index 33ab2907906e..5bd4a1dbcbe9 100644
--- a/llvm/lib/Transforms/Scalar/SCCP.cpp
+++ b/llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -1467,6 +1467,7 @@ void SCCPSolver::Solve() {
/// This scan also checks for values that use undefs. It conservatively marks
/// them as overdefined.
bool SCCPSolver::ResolvedUndefsIn(Function &F) {
+ bool MadeChange = false;
for (BasicBlock &BB : F) {
if (!BBExecutable.count(&BB))
continue;
@@ -1492,8 +1493,10 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) {
// more precise than this but it isn't worth bothering.
for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
ValueLatticeElement &LV = getStructValueState(&I, i);
- if (LV.isUnknownOrUndef())
+ if (LV.isUnknownOrUndef()) {
markOverdefined(LV, &I);
+ MadeChange = true;
+ }
}
continue;
}
@@ -1520,7 +1523,7 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) {
}
markOverdefined(&I);
- return true;
+ MadeChange = true;
}
// Check to see if we have a branch or switch on an undefined value. If so
@@ -1537,7 +1540,8 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) {
if (isa<UndefValue>(BI->getCondition())) {
BI->setCondition(ConstantInt::getFalse(BI->getContext()));
markEdgeExecutable(&BB, TI->getSuccessor(1));
- return true;
+ MadeChange = true;
+ continue;
}
// Otherwise, it is a branch on a symbolic value which is currently
@@ -1546,7 +1550,7 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) {
// FIXME: Distinguish between dead code and an LLVM "undef" value.
BasicBlock *DefaultSuccessor = TI->getSuccessor(1);
if (markEdgeExecutable(&BB, DefaultSuccessor))
- return true;
+ MadeChange = true;
continue;
}
@@ -1565,7 +1569,8 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) {
if (isa<UndefValue>(IBR->getAddress())) {
IBR->setAddress(BlockAddress::get(IBR->getSuccessor(0)));
markEdgeExecutable(&BB, IBR->getSuccessor(0));
- return true;
+ MadeChange = true;
+ continue;
}
// Otherwise, it is a branch on a symbolic value which is currently
@@ -1575,7 +1580,7 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) {
// we can assume the branch has undefined behavior instead.
BasicBlock *DefaultSuccessor = IBR->getSuccessor(0);
if (markEdgeExecutable(&BB, DefaultSuccessor))
- return true;
+ MadeChange = true;
continue;
}
@@ -1590,7 +1595,8 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) {
if (isa<UndefValue>(SI->getCondition())) {
SI->setCondition(SI->case_begin()->getCaseValue());
markEdgeExecutable(&BB, SI->case_begin()->getCaseSuccessor());
- return true;
+ MadeChange = true;
+ continue;
}
// Otherwise, it is a branch on a symbolic value which is currently
@@ -1599,13 +1605,13 @@ bool SCCPSolver::ResolvedUndefsIn(Function &F) {
// FIXME: Distinguish between dead code and an LLVM "undef" value.
BasicBlock *DefaultSuccessor = SI->case_begin()->getCaseSuccessor();
if (markEdgeExecutable(&BB, DefaultSuccessor))
- return true;
+ MadeChange = true;
continue;
}
}
- return false;
+ return MadeChange;
}
static bool tryToReplaceWithConstant(SCCPSolver &Solver, Value *V) {
@@ -1963,13 +1969,12 @@ bool llvm::runIPSCCP(
while (ResolvedUndefs) {
LLVM_DEBUG(dbgs() << "RESOLVING UNDEFS\n");
ResolvedUndefs = false;
- for (Function &F : M)
- if (Solver.ResolvedUndefsIn(F)) {
- // We run Solve() after we resolved an undef in a function, because
- // we might deduce a fact that eliminates an undef in another function.
- Solver.Solve();
+ for (Function &F : M) {
+ if (Solver.ResolvedUndefsIn(F))
ResolvedUndefs = true;
- }
+ }
+ if (ResolvedUndefs)
+ Solver.Solve();
}
bool MadeChanges = false;
diff --git a/llvm/test/Transforms/SCCP/2004-12-10-UndefBranchBug.ll b/llvm/test/Transforms/SCCP/2004-12-10-UndefBranchBug.ll
index c847b4eaca3d..24fecf8a4698 100644
--- a/llvm/test/Transforms/SCCP/2004-12-10-UndefBranchBug.ll
+++ b/llvm/test/Transforms/SCCP/2004-12-10-UndefBranchBug.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -sccp -S | grep "ret i32 1"
+; RUN: opt < %s -sccp -S | grep "ret i32 %X"
; This function definitely returns 1, even if we don't know the direction
; of the branch.
diff --git a/llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll b/llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll
index 9e9d1256c4cc..7c8bc16fd67b 100644
--- a/llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll
+++ b/llvm/test/Transforms/SCCP/resolvedundefsin-tracked-fn.ll
@@ -274,8 +274,9 @@ define internal void @test4_b(i8* %arg) {
; CHECK-SAME: (i8* [[ARG:%.*]])
; CHECK-NEXT: bb:
; CHECK-NEXT: [[TMP:%.*]] = bitcast i8* null to i16*
+; CHECK-NEXT: [[SEL:%.*]] = select i1 false, i8* null, i8* null
; CHECK-NEXT: call void @use.16(i16* [[TMP]])
-; CHECK-NEXT: call void @use.8(i8* null)
+; CHECK-NEXT: call void @use.8(i8* [[SEL]])
; CHECK-NEXT: ret void
;
bb:
@@ -384,7 +385,10 @@ define void @test3() {
; CHECK-NEXT: store i32 [[MUL]], i32* @pcount, align 4
; CHECK-NEXT: ret void
; CHECK: if.end24:
-; CHECK-NEXT: br label [[FOR_END:%.*]]
+; CHECK-NEXT: [[CMP25474:%.*]] = icmp sgt i32 [[TMP2]], 0
+; CHECK-NEXT: br i1 [[CMP25474]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+; CHECK: for.body:
+; CHECK-NEXT: ret void
; CHECK: for.end:
; CHECK-NEXT: ret void
;
More information about the llvm-commits
mailing list