Skip to content

Commit b0d35cf

Browse files
[SSAUpdater] Avoid scanning basic blocks to find instruction order. (#123803)
This fixes a compile-time regression caused by #116645, where an entry basic block with a very large number of allocas and other instructions caused SROA to take ~100× its expected runtime, as every alloca (with ~2 uses) now calls this method to find the order of those few instructions, rescanning the very large basic block every single time. Since this code was originally written, Instructions now have ordering numbers available to determine relative order without unnecessarily scanning the basic block.
1 parent 6b486f4 commit b0d35cf

File tree

2 files changed

+12
-29
lines changed

2 files changed

+12
-29
lines changed

llvm/include/llvm/Transforms/Utils/SSAUpdater.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,6 @@ class LoadAndStorePromoter {
164164
/// removed from the code.
165165
void run(const SmallVectorImpl<Instruction *> &Insts);
166166

167-
/// Return true if the specified instruction is in the Inst list.
168-
///
169-
/// The Insts list is the one passed into the constructor. Clients should
170-
/// implement this with a more efficient version if possible.
171-
virtual bool isInstInList(Instruction *I,
172-
const SmallVectorImpl<Instruction *> &Insts) const;
173-
174167
/// This hook is invoked after all the stores are found and inserted as
175168
/// available values.
176169
virtual void doExtraRewritesBeforeFinalDeletion() {}

llvm/lib/Transforms/Utils/SSAUpdater.cpp

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -432,27 +432,28 @@ void LoadAndStorePromoter::run(const SmallVectorImpl<Instruction *> &Insts) {
432432
}
433433
}
434434

435-
// If so, we can queue them all as live in loads. We don't have an
436-
// efficient way to tell which on is first in the block and don't want to
437-
// scan large blocks, so just add all loads as live ins.
435+
// If so, we can queue them all as live in loads.
438436
if (!HasStore) {
439437
for (Instruction *I : BlockUses)
440438
LiveInLoads.push_back(cast<LoadInst>(I));
441439
BlockUses.clear();
442440
continue;
443441
}
444442

443+
// Sort all of the interesting instructions in the block so that we don't
444+
// have to scan a large block just to find a few instructions.
445+
llvm::sort(
446+
BlockUses.begin(), BlockUses.end(),
447+
[](Instruction *A, Instruction *B) { return A->comesBefore(B); });
448+
445449
// Otherwise, we have mixed loads and stores (or just a bunch of stores).
446450
// Since SSAUpdater is purely for cross-block values, we need to determine
447451
// the order of these instructions in the block. If the first use in the
448452
// block is a load, then it uses the live in value. The last store defines
449-
// the live out value. We handle this by doing a linear scan of the block.
453+
// the live out value.
450454
Value *StoredValue = nullptr;
451-
for (Instruction &I : *BB) {
452-
if (LoadInst *L = dyn_cast<LoadInst>(&I)) {
453-
// If this is a load from an unrelated pointer, ignore it.
454-
if (!isInstInList(L, Insts)) continue;
455-
455+
for (Instruction *I : BlockUses) {
456+
if (LoadInst *L = dyn_cast<LoadInst>(I)) {
456457
// If we haven't seen a store yet, this is a live in use, otherwise
457458
// use the stored value.
458459
if (StoredValue) {
@@ -465,18 +466,14 @@ void LoadAndStorePromoter::run(const SmallVectorImpl<Instruction *> &Insts) {
465466
continue;
466467
}
467468

468-
if (StoreInst *SI = dyn_cast<StoreInst>(&I)) {
469-
// If this is a store to an unrelated pointer, ignore it.
470-
if (!isInstInList(SI, Insts)) continue;
469+
if (StoreInst *SI = dyn_cast<StoreInst>(I)) {
471470
updateDebugInfo(SI);
472471

473472
// Remember that this is the active value in the block.
474473
StoredValue = SI->getOperand(0);
475-
} else if (auto *AI = dyn_cast<AllocaInst>(&I)) {
474+
} else if (auto *AI = dyn_cast<AllocaInst>(I)) {
476475
// Check if this an alloca, in which case we treat it as a store of
477476
// getValueToUseForAlloca.
478-
if (!isInstInList(AI, Insts))
479-
continue;
480477
StoredValue = getValueToUseForAlloca(AI);
481478
}
482479
}
@@ -533,10 +530,3 @@ void LoadAndStorePromoter::run(const SmallVectorImpl<Instruction *> &Insts) {
533530
User->eraseFromParent();
534531
}
535532
}
536-
537-
bool
538-
LoadAndStorePromoter::isInstInList(Instruction *I,
539-
const SmallVectorImpl<Instruction *> &Insts)
540-
const {
541-
return is_contained(Insts, I);
542-
}

0 commit comments

Comments
 (0)