Skip to content

Commit 8520120

Browse files
committed
SimplifyCFG: don't recalculate the dominator tree for each jump threaded checked_cast_br instruction.
This is done by splitting the transformation into an analysis phase and a transformation phase (which does not use the dominator tree anymore). The domintator tree is recalucated once after the whole function is processed. This change eventually solves the compile time problem of rdar://problem/24410167.
1 parent 6e00d8a commit 8520120

File tree

3 files changed

+258
-356
lines changed

3 files changed

+258
-356
lines changed

include/swift/SILOptimizer/Utils/Local.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,13 @@ SILLinkage getSpecializedLinkage(SILFunction *F, SILLinkage L);
158158
/// string literals. Returns a new instruction if optimization was possible.
159159
SILInstruction *tryToConcatenateStrings(ApplyInst *AI, SILBuilder &B);
160160

161-
/// Tries to perform jump-threading on a given checked_cast_br terminator.
162-
bool tryCheckedCastBrJumpThreading(TermInst *Term, DominanceInfo *DT,
163-
SmallVectorImpl<SILBasicBlock *> &BBs);
161+
162+
/// Tries to perform jump-threading on all checked_cast_br instruction in
163+
/// function \p Fn.
164+
bool tryCheckedCastBrJumpThreading(SILFunction *Fn, DominanceInfo *DT,
165+
SmallVectorImpl<SILBasicBlock *> &BlocksForWorklist);
166+
167+
void recalcDomTreeForCCBOpt(DominanceInfo *DT, SILFunction &F);
164168

165169
/// Checks if a symbol with a given linkage can be referenced from fragile
166170
/// functions.

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

Lines changed: 8 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,6 @@ namespace {
170170
bool simplifyProgramTerminationBlock(SILBasicBlock *BB);
171171
bool simplifyArgument(SILBasicBlock *BB, unsigned i);
172172
bool simplifyArgs(SILBasicBlock *BB);
173-
bool trySimplifyCheckedCastBr(TermInst *Term, DominanceInfo *DT);
174173
void findLoopHeaders();
175174
};
176175

@@ -239,28 +238,6 @@ void swift::updateSSAAfterCloning(BaseThreadingCloner &Cloner,
239238
}
240239
}
241240

242-
/// Perform a dominator-based jump-threading for checked_cast_br [exact]
243-
/// instructions if they use the same condition (modulo upcasts and downcasts).
244-
/// This is very beneficial for code that:
245-
/// - references the same object multiple times (e.g. x.f1() + x.f2())
246-
/// - and for method invocation chaining (e.g. x.f3().f4().f5())
247-
bool
248-
SimplifyCFG::trySimplifyCheckedCastBr(TermInst *Term, DominanceInfo *DT) {
249-
// Ignore unreachable blocks.
250-
if (!DT->getNode(Term->getParent()))
251-
return false;
252-
253-
SmallVector<SILBasicBlock *, 16> BBs;
254-
auto Result = tryCheckedCastBrJumpThreading(Term, DT, BBs);
255-
256-
if (Result) {
257-
for (auto BB: BBs)
258-
addToWorklist(BB);
259-
}
260-
261-
return Result;
262-
}
263-
264241
static SILValue getTerminatorCondition(TermInst *Term) {
265242
if (auto *CondBr = dyn_cast<CondBranchInst>(Term))
266243
return stripExpectIntrinsic(CondBr->getCondition());
@@ -670,6 +647,7 @@ bool SimplifyCFG::dominatorBasedSimplify(DominanceAnalysis *DA) {
670647
EnableJumpThread ? splitAllCriticalEdges(Fn, false, DT, nullptr) : false;
671648

672649
unsigned MaxIter = MaxIterationsOfDominatorBasedSimplify;
650+
SmallVector<SILBasicBlock *, 16> BlocksForWorklist;
673651

674652
bool HasChangedInCurrentIter;
675653
do {
@@ -678,26 +656,14 @@ bool SimplifyCFG::dominatorBasedSimplify(DominanceAnalysis *DA) {
678656
// Do dominator based simplification of terminator condition. This does not
679657
// and MUST NOT change the CFG without updating the dominator tree to
680658
// reflect such change.
681-
for (auto &BB : Fn) {
682-
// Any method called from this loop should update
683-
// the DT if it changes anything related to dominators.
684-
TermInst *Term = BB.getTerminator();
685-
switch (Term->getKind()) {
686-
case ValueKind::SwitchValueInst:
687-
// TODO: handle switch_value
688-
break;
689-
case ValueKind::CheckedCastBranchInst:
690-
if (trySimplifyCheckedCastBr(BB.getTerminator(), DT)) {
691-
HasChangedInCurrentIter = true;
692-
// FIXME: trySimplifyCheckedCastBr function should preserve the
693-
// dominator tree but its code to do so is buggy.
694-
DT->recalculate(Fn);
695-
}
696-
break;
697-
default:
698-
break;
699-
}
659+
if (tryCheckedCastBrJumpThreading(&Fn, DT, BlocksForWorklist)) {
660+
for (auto BB: BlocksForWorklist)
661+
addToWorklist(BB);
662+
663+
HasChangedInCurrentIter = true;
664+
DT->recalculate(Fn);
700665
}
666+
BlocksForWorklist.clear();
701667

702668
if (ShouldVerify)
703669
DT->verify();

0 commit comments

Comments
 (0)