Skip to content

Commit 6529787

Browse files
committed
SILVerifier: fix quadratic behavior for large basic blocks.
For large basic blocks the dominance check between two instructions in the same block was very expensive. Although the verifier does not run in no-assert compiler builds, we don't want it to be extra slow for assert builds. https://bugs.swift.org/browse/SR-7632
1 parent 0910d7f commit 6529787

File tree

1 file changed

+33
-3
lines changed

1 file changed

+33
-3
lines changed

lib/SIL/SILVerifier.cpp

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
437437
SmallVector<StringRef, 16> DebugVars;
438438
const SILInstruction *CurInstruction = nullptr;
439439
DominanceInfo *Dominance = nullptr;
440+
441+
// Used for dominance checking within a basic block.
442+
llvm::DenseMap<const SILInstruction *, unsigned> InstNumbers;
443+
440444
DeadEndBlocks DEBlocks;
441445
bool SingleFunction = true;
442446

@@ -615,20 +619,32 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
615619
}
616620
}
617621

622+
static unsigned numInstsInFunction(const SILFunction &F) {
623+
unsigned numInsts = 0;
624+
for (auto &BB : F) {
625+
numInsts += std::distance(BB.begin(), BB.end());
626+
}
627+
return numInsts;
628+
}
629+
618630
SILVerifier(const SILFunction &F, bool SingleFunction = true)
619631
: M(F.getModule().getSwiftModule()), F(F),
620632
fnConv(F.getLoweredFunctionType(), F.getModule()),
621633
TC(F.getModule().Types), OpenedArchetypes(&F), Dominance(nullptr),
634+
InstNumbers(numInstsInFunction(F)),
622635
DEBlocks(&F), SingleFunction(SingleFunction) {
623636
if (F.isExternalDeclaration())
624637
return;
625638

626639
// Check to make sure that all blocks are well formed. If not, the
627640
// SILVerifier object will explode trying to compute dominance info.
641+
unsigned InstIdx = 0;
628642
for (auto &BB : F) {
629643
require(!BB.empty(), "Basic blocks cannot be empty");
630644
require(isa<TermInst>(BB.back()),
631645
"Basic blocks must end with a terminator instruction");
646+
for (auto &I : BB)
647+
InstNumbers[&I] = InstIdx++;
632648
}
633649

634650
Dominance = new DominanceInfo(const_cast<SILFunction *>(&F));
@@ -644,6 +660,20 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
644660
delete Dominance;
645661
}
646662

663+
// Checks dominance between two instructions.
664+
// This does not use DominanceInfo.properlyDominates, because for large basic
665+
// blocks it would result in quadratic behavior.
666+
bool properlyDominates(SILInstruction *a, SILInstruction *b) {
667+
auto aBlock = a->getParent(), bBlock = b->getParent();
668+
669+
// If the blocks are different, it's as easy as whether A's block
670+
// dominates B's block.
671+
if (aBlock != bBlock)
672+
return Dominance->properlyDominates(aBlock, bBlock);
673+
674+
return InstNumbers[a] < InstNumbers[b];
675+
}
676+
647677
// FIXME: For sanity, address-type block args should be prohibited at all SIL
648678
// stages. However, the optimizer currently breaks the invariant in three
649679
// places:
@@ -775,7 +805,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
775805
} else {
776806
require(valueI->getFunction() == &F,
777807
"instruction uses value of instruction from another function");
778-
require(Dominance->properlyDominates(valueI, I),
808+
require(properlyDominates(valueI, I),
779809
"instruction isn't dominated by its operand");
780810
}
781811
}
@@ -939,7 +969,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
939969
auto Def = OpenedArchetypes.getOpenedArchetypeDef(OpenedA);
940970
require (Def, "Opened archetype should be registered in SILFunction");
941971
require(I == nullptr || Def == I ||
942-
Dominance->properlyDominates(cast<SILInstruction>(Def), I),
972+
properlyDominates(cast<SILInstruction>(Def), I),
943973
"Use of an opened archetype should be dominated by a "
944974
"definition of this opened archetype");
945975
}
@@ -1071,7 +1101,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
10711101
auto Def = OpenedArchetypes.getOpenedArchetypeDef(A);
10721102
require(Def, "Opened archetype should be registered in SILFunction");
10731103
require(Def == AI ||
1074-
Dominance->properlyDominates(cast<SILInstruction>(Def), AI),
1104+
properlyDominates(cast<SILInstruction>(Def), AI),
10751105
"Use of an opened archetype should be dominated by a "
10761106
"definition of this opened archetype");
10771107
}

0 commit comments

Comments
 (0)