Skip to content

Commit 4106b76

Browse files
committed
Fix ArrayPropertyOpt dominator update
Pass the DomTree into SILCloner so that edge splitting properly creates new domtree nodes. The confusing edge splitting code that was in ArrayPropertyOpt is no longer needed. We could cleanup ArrayPropertyOpt even more by moving fixDomTree into SILCloner. But there's already too much going on in this patch.
1 parent b06aea0 commit 4106b76

File tree

1 file changed

+14
-27
lines changed

1 file changed

+14
-27
lines changed

lib/SILOptimizer/LoopTransforms/ArrayPropertyOpt.cpp

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -430,37 +430,24 @@ namespace {
430430
/// in a set of basic blocks. Updates the dominator tree with the cloned blocks.
431431
/// However, the client needs to update the dominator of the exit blocks.
432432
///
433-
/// FIXME: SILCloner is used to cloned CFG regions by multiple clients. All
434-
/// functionality for generating valid SIL (including the DomTree) should be
435-
/// handled by the common SILCloner.
433+
/// FIXME: All functionality for generating valid SIL (including the DomTree)
434+
/// should be handled by the common SILCloner. Currently, SILCloner only updates
435+
/// the DomTree for original (non-cloned) blocks when splitting edges. The
436+
/// cloned blocks won't be mapped to dominator nodes until fixDomTree()
437+
/// runs. However, since SILCloner always handles single-entry regions,
438+
/// fixDomTree() could be part of SILCloner itself.
436439
class RegionCloner : public SILCloner<RegionCloner> {
437-
DominanceInfo &DomTree;
438440
SILBasicBlock *StartBB;
439441

440442
friend class SILInstructionVisitor<RegionCloner>;
441443
friend class SILCloner<RegionCloner>;
442444

443445
public:
444446
RegionCloner(SILBasicBlock *EntryBB, DominanceInfo &DT)
445-
: SILCloner<RegionCloner>(*EntryBB->getParent()), DomTree(DT),
446-
StartBB(EntryBB) {}
447+
: SILCloner<RegionCloner>(*EntryBB->getParent(), &DT), StartBB(EntryBB) {}
447448

448449
SILBasicBlock *cloneRegion(ArrayRef<SILBasicBlock *> exitBBs) {
449-
assert (DomTree.getNode(StartBB) != nullptr && "Can't cloned dead code");
450-
451-
// We need to split any edge from a non cond_br basic block leading to a
452-
// exit block. After cloning this edge will become critical if it came from
453-
// inside the cloned region. The SSAUpdater can't handle critical non
454-
// cond_br edges.
455-
//
456-
// FIXME: remove this in the next commit. The SILCloner will always do it.
457-
for (auto *BB : exitBBs) {
458-
SmallVector<SILBasicBlock *, 8> Preds(BB->getPredecessorBlocks());
459-
for (auto *Pred : Preds)
460-
if (!isa<CondBranchInst>(Pred->getTerminator()) &&
461-
!isa<BranchInst>(Pred->getTerminator()))
462-
splitEdgesFromTo(Pred, BB, &DomTree, nullptr);
463-
}
450+
assert(DomTree->getNode(StartBB) != nullptr && "Can't cloned dead code");
464451

465452
cloneReachableBlocks(StartBB, exitBBs);
466453

@@ -477,25 +464,25 @@ class RegionCloner : public SILCloner<RegionCloner> {
477464
void fixDomTree() {
478465
for (auto *BB : originalPreorderBlocks()) {
479466
auto *ClonedBB = getOpBasicBlock(BB);
480-
auto *OrigDomBB = DomTree.getNode(BB)->getIDom()->getBlock();
467+
auto *OrigDomBB = DomTree->getNode(BB)->getIDom()->getBlock();
481468
if (BB == StartBB) {
482469
// The cloned start node shares the same dominator as the original node.
483-
auto *ClonedNode = DomTree.addNewBlock(ClonedBB, OrigDomBB);
470+
auto *ClonedNode = DomTree->addNewBlock(ClonedBB, OrigDomBB);
484471
(void)ClonedNode;
485472
assert(ClonedNode);
486473
continue;
487474
}
488475
// Otherwise, map the dominator structure using the mapped block.
489-
DomTree.addNewBlock(ClonedBB, getOpBasicBlock(OrigDomBB));
476+
DomTree->addNewBlock(ClonedBB, getOpBasicBlock(OrigDomBB));
490477
}
491478
}
492479

493480
SILValue getMappedValue(SILValue V) {
494481
if (auto *BB = V->getParentBlock()) {
495-
if (!DomTree.dominates(StartBB, BB)) {
482+
if (!DomTree->dominates(StartBB, BB)) {
496483
// Must be a value that dominates the start basic block.
497-
assert(DomTree.dominates(BB, StartBB) &&
498-
"Must dominated the start of the cloned region");
484+
assert(DomTree->dominates(BB, StartBB)
485+
&& "Must dominated the start of the cloned region");
499486
return V;
500487
}
501488
}

0 commit comments

Comments
 (0)