Skip to content

Commit f5b5147

Browse files
committed
Fix the SimplifyCFG ThreadInfo broken abstraction.
In a blatant abuse of OO style, the logic for manipulating the CFG was encapsulated inside a descriptor of the CFG edge, making it impossible to work with.
1 parent 51dfc63 commit f5b5147

File tree

1 file changed

+63
-57
lines changed

1 file changed

+63
-57
lines changed

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

Lines changed: 63 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ STATISTIC(NumSROAArguments, "Number of aggregate argument levels split by "
6060
static unsigned MaxIterationsOfDominatorBasedSimplify = 10;
6161

6262
namespace {
63+
64+
struct ThreadInfo;
65+
6366
class SimplifyCFG {
6467
SILOptFunctionBuilder FuncBuilder;
6568
SILFunction &Fn;
@@ -174,6 +177,7 @@ class SimplifyCFG {
174177
bool simplifyThreadedTerminators();
175178
bool dominatorBasedSimplifications(SILFunction &Fn, DominanceInfo *DT);
176179
bool dominatorBasedSimplify(DominanceAnalysis *DA);
180+
bool threadEdge(const ThreadInfo &ti);
177181

178182
/// Remove the basic block if it has no predecessors. Returns true
179183
/// If the block was removed.
@@ -245,6 +249,7 @@ static bool isThreadableBlock(SILBasicBlock *BB,
245249
return true;
246250
}
247251

252+
namespace {
248253

249254
/// A description of an edge leading to a conditionally branching (or switching)
250255
/// block and the successor block to thread to.
@@ -260,13 +265,12 @@ static bool isThreadableBlock(SILBasicBlock *BB,
260265
/// / \
261266
/// ... v
262267
/// EnumCase/ThreadedSuccessorIdx
263-
class ThreadInfo {
268+
struct ThreadInfo {
264269
SILBasicBlock *Src;
265270
SILBasicBlock *Dest;
266271
EnumElementDecl *EnumCase;
267272
unsigned ThreadedSuccessorIdx;
268273

269-
public:
270274
ThreadInfo(SILBasicBlock *Src, SILBasicBlock *Dest,
271275
unsigned ThreadedBlockSuccessorIdx)
272276
: Src(Src), Dest(Dest), EnumCase(nullptr),
@@ -276,69 +280,71 @@ class ThreadInfo {
276280
: Src(Src), Dest(Dest), EnumCase(EnumCase), ThreadedSuccessorIdx(0) {}
277281

278282
ThreadInfo() = default;
283+
};
279284

280-
bool threadEdge() {
281-
LLVM_DEBUG(llvm::dbgs() << "thread edge from bb" << Src->getDebugID()
282-
<< " to bb" << Dest->getDebugID() << '\n');
283-
auto *SrcTerm = cast<BranchInst>(Src->getTerminator());
285+
} // end anonymous namespace
284286

285-
BasicBlockCloner Cloner(SrcTerm->getDestBB());
286-
if (!Cloner.canCloneBlock())
287-
return false;
287+
bool SimplifyCFG::threadEdge(const ThreadInfo &ti) {
288+
LLVM_DEBUG(llvm::dbgs() << "thread edge from bb" << ti.Src->getDebugID()
289+
<< " to bb" << ti.Dest->getDebugID() << '\n');
290+
auto *SrcTerm = cast<BranchInst>(ti.Src->getTerminator());
288291

289-
Cloner.cloneBranchTarget(SrcTerm);
292+
BasicBlockCloner Cloner(SrcTerm->getDestBB());
293+
if (!Cloner.canCloneBlock())
294+
return false;
290295

291-
// We have copied the threaded block into the edge.
292-
Src = Cloner.getNewBB();
296+
Cloner.cloneBranchTarget(SrcTerm);
293297

294-
// Rewrite the cloned branch to eliminate the non-taken path.
295-
if (auto *CondTerm = dyn_cast<CondBranchInst>(Src->getTerminator())) {
296-
// We know the direction this conditional branch is going to take thread
297-
// it.
298-
assert(Src->getSuccessors().size() > ThreadedSuccessorIdx &&
299-
"Threaded terminator does not have enough successors");
298+
// We have copied the threaded block into the edge.
299+
auto *clonedSrc = Cloner.getNewBB();
300300

301-
auto *ThreadedSuccessorBlock =
302-
Src->getSuccessors()[ThreadedSuccessorIdx].getBB();
303-
auto Args = ThreadedSuccessorIdx == 0 ? CondTerm->getTrueArgs()
304-
: CondTerm->getFalseArgs();
301+
// Rewrite the cloned branch to eliminate the non-taken path.
302+
if (auto *CondTerm = dyn_cast<CondBranchInst>(clonedSrc->getTerminator())) {
303+
// We know the direction this conditional branch is going to take thread
304+
// it.
305+
assert(clonedSrc->getSuccessors().size() > ti.ThreadedSuccessorIdx &&
306+
"Threaded terminator does not have enough successors");
305307

306-
SILBuilderWithScope(CondTerm)
307-
.createBranch(CondTerm->getLoc(), ThreadedSuccessorBlock, Args);
308+
auto *ThreadedSuccessorBlock =
309+
clonedSrc->getSuccessors()[ti.ThreadedSuccessorIdx].getBB();
310+
auto Args = ti.ThreadedSuccessorIdx == 0 ? CondTerm->getTrueArgs()
311+
: CondTerm->getFalseArgs();
308312

309-
CondTerm->eraseFromParent();
310-
} else {
311-
// Get the enum element and the destination block of the block we jump
312-
// thread.
313-
auto *SEI = cast<SwitchEnumInst>(Src->getTerminator());
314-
auto *ThreadedSuccessorBlock = SEI->getCaseDestination(EnumCase);
315-
316-
// Instantiate the payload if necessary.
317-
SILBuilderWithScope Builder(SEI);
318-
if (!ThreadedSuccessorBlock->args_empty()) {
319-
auto EnumVal = SEI->getOperand();
320-
auto EnumTy = EnumVal->getType();
321-
auto Loc = SEI->getLoc();
322-
auto Ty = EnumTy.getEnumElementType(EnumCase, SEI->getModule(),
323-
Builder.getTypeExpansionContext());
324-
SILValue UED(
325-
Builder.createUncheckedEnumData(Loc, EnumVal, EnumCase, Ty));
326-
assert(UED->getType() ==
327-
(*ThreadedSuccessorBlock->args_begin())->getType() &&
328-
"Argument types must match");
329-
Builder.createBranch(SEI->getLoc(), ThreadedSuccessorBlock, {UED});
330-
} else
331-
Builder.createBranch(SEI->getLoc(), ThreadedSuccessorBlock,
332-
ArrayRef<SILValue>());
333-
SEI->eraseFromParent();
334-
}
335-
// After rewriting the cloned branch, split the critical edge.
336-
// This does not currently update DominanceInfo.
337-
Cloner.splitCriticalEdges(nullptr, nullptr);
338-
Cloner.updateSSAAfterCloning();
339-
return true;
313+
SILBuilderWithScope(CondTerm)
314+
.createBranch(CondTerm->getLoc(), ThreadedSuccessorBlock, Args);
315+
316+
CondTerm->eraseFromParent();
317+
} else {
318+
// Get the enum element and the destination block of the block we jump
319+
// thread.
320+
auto *SEI = cast<SwitchEnumInst>(clonedSrc->getTerminator());
321+
auto *ThreadedSuccessorBlock = SEI->getCaseDestination(ti.EnumCase);
322+
323+
// Instantiate the payload if necessary.
324+
SILBuilderWithScope Builder(SEI);
325+
if (!ThreadedSuccessorBlock->args_empty()) {
326+
auto EnumVal = SEI->getOperand();
327+
auto EnumTy = EnumVal->getType();
328+
auto Loc = SEI->getLoc();
329+
auto Ty = EnumTy.getEnumElementType(ti.EnumCase, SEI->getModule(),
330+
Builder.getTypeExpansionContext());
331+
SILValue UED(
332+
Builder.createUncheckedEnumData(Loc, EnumVal, ti.EnumCase, Ty));
333+
assert(UED->getType() ==
334+
(*ThreadedSuccessorBlock->args_begin())->getType() &&
335+
"Argument types must match");
336+
Builder.createBranch(SEI->getLoc(), ThreadedSuccessorBlock, {UED});
337+
} else
338+
Builder.createBranch(SEI->getLoc(), ThreadedSuccessorBlock,
339+
ArrayRef<SILValue>());
340+
SEI->eraseFromParent();
340341
}
341-
};
342+
// After rewriting the cloned branch, split the critical edge.
343+
// This does not currently update DominanceInfo.
344+
Cloner.splitCriticalEdges(nullptr, nullptr);
345+
Cloner.updateSSAAfterCloning();
346+
return true;
347+
}
342348

343349
/// Give a cond_br or switch_enum instruction and one successor block return
344350
/// true if we can infer the value of the condition/enum along the edge to this
@@ -558,7 +564,7 @@ bool SimplifyCFG::dominatorBasedSimplifications(SILFunction &Fn,
558564
return Changed;
559565

560566
for (auto &ThreadInfo : JumpThreadableEdges) {
561-
if (ThreadInfo.threadEdge())
567+
if (threadEdge(ThreadInfo))
562568
Changed = true;
563569
}
564570

0 commit comments

Comments
 (0)