Revert [DSE] Preserve basic block ordering using OrderedBasicBlock.

This reverts r357208 (git commit c0bfd37d38)

This causes a buildbot failure:  http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/16124

FAILED: lib/IR/CMakeFiles/LLVMCore.dir/IRBuilder.cpp.o
/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/install/stage2/bin/clang++   -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/IR -I/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/IR -Iinclude -I/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/include -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -flto=thin -O3    -UNDEBUG  -fno-exceptions -fno-rtti -MD -MT lib/IR/CMakeFiles/LLVMCore.dir/IRBuilder.cpp.o -MF lib/IR/CMakeFiles/LLVMCore.dir/IRBuilder.cpp.o.d -o lib/IR/CMakeFiles/LLVMCore.dir/IRBuilder.cpp.o -c /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/IR/IRBuilder.cpp
clang-9: /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Analysis/OrderedBasicBlock.cpp:38: bool llvm::OrderedBasicBlock::comesBefore(const llvm::Instruction *, const llvm::Instruction *): Assertion `!(LastInstFound == BB->end() && NextInstPos != 0) && "Instruction supposed to be in NumberedInsts"' failed.

llvm-svn: 357211
This commit is contained in:
Florian Hahn 2019-03-28 20:36:24 +00:00
parent a1f1ff8896
commit c87869ebea
3 changed files with 39 additions and 61 deletions

View File

@ -59,14 +59,6 @@ public:
/// only relevant to compare relative instructions positions inside \p BB.
/// Returns false for A == B.
bool dominates(const Instruction *A, const Instruction *B);
/// Remove \p from the ordering, if it is present.
void eraseInstruction(const Instruction *I);
/// Replace \p Old with \p New in the ordering. \p New is assigned the
/// numbering of \p Old, so it must be inserted at the same position in the
/// IR.
void replaceInstruction(const Instruction *Old, const Instruction *New);
};
} // End llvm namespace

View File

@ -85,26 +85,3 @@ bool OrderedBasicBlock::dominates(const Instruction *A, const Instruction *B) {
return comesBefore(A, B);
}
void OrderedBasicBlock::eraseInstruction(const Instruction *I) {
if (LastInstFound != BB->end() && I == &*LastInstFound) {
if (LastInstFound == BB->begin())
LastInstFound = BB->end();
else
LastInstFound--;
}
NumberedInsts.erase(I);
}
void OrderedBasicBlock::replaceInstruction(const Instruction *Old,
const Instruction *New) {
auto OI = NumberedInsts.find(Old);
if (OI == NumberedInsts.end())
return;
NumberedInsts[New] = OI->second;
if (LastInstFound != BB->end() && Old == &*LastInstFound)
LastInstFound = New->getIterator();
NumberedInsts.erase(Old);
}

View File

@ -28,8 +28,8 @@
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/MemoryDependenceAnalysis.h"
#include "llvm/Analysis/MemoryLocation.h"
#include "llvm/Analysis/OrderedBasicBlock.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/Transforms/Utils/Local.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/Argument.h"
#include "llvm/IR/BasicBlock.h"
@ -56,7 +56,6 @@
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Transforms/Scalar.h"
#include "llvm/Transforms/Utils/Local.h"
#include <algorithm>
#include <cassert>
#include <cstddef>
@ -98,7 +97,8 @@ using InstOverlapIntervalsTy = DenseMap<Instruction *, OverlapIntervalsTy>;
static void
deleteDeadInstruction(Instruction *I, BasicBlock::iterator *BBI,
MemoryDependenceResults &MD, const TargetLibraryInfo &TLI,
InstOverlapIntervalsTy &IOL, OrderedBasicBlock &OBB,
InstOverlapIntervalsTy &IOL,
DenseMap<Instruction*, size_t> *InstrOrdering,
SmallSetVector<Value *, 16> *ValueSet = nullptr) {
SmallVector<Instruction*, 32> NowDeadInsts;
@ -135,8 +135,8 @@ deleteDeadInstruction(Instruction *I, BasicBlock::iterator *BBI,
}
if (ValueSet) ValueSet->remove(DeadInst);
InstrOrdering->erase(DeadInst);
IOL.erase(DeadInst);
OBB.eraseInstruction(DeadInst);
if (NewIter == DeadInst->getIterator())
NewIter = DeadInst->eraseFromParent();
@ -656,7 +656,8 @@ static void findUnconditionalPreds(SmallVectorImpl<BasicBlock *> &Blocks,
static bool handleFree(CallInst *F, AliasAnalysis *AA,
MemoryDependenceResults *MD, DominatorTree *DT,
const TargetLibraryInfo *TLI,
InstOverlapIntervalsTy &IOL, OrderedBasicBlock &OBB) {
InstOverlapIntervalsTy &IOL,
DenseMap<Instruction*, size_t> *InstrOrdering) {
bool MadeChange = false;
MemoryLocation Loc = MemoryLocation(F->getOperand(0));
@ -690,7 +691,7 @@ static bool handleFree(CallInst *F, AliasAnalysis *AA,
// DCE instructions only used to calculate that store.
BasicBlock::iterator BBI(Dependency);
deleteDeadInstruction(Dependency, &BBI, *MD, *TLI, IOL, OBB);
deleteDeadInstruction(Dependency, &BBI, *MD, *TLI, IOL, InstrOrdering);
++NumFastStores;
MadeChange = true;
@ -745,10 +746,10 @@ static void removeAccessedObjects(const MemoryLocation &LoadedLoc,
/// store i32 1, i32* %A
/// ret void
static bool handleEndBlock(BasicBlock &BB, AliasAnalysis *AA,
MemoryDependenceResults *MD,
const TargetLibraryInfo *TLI,
InstOverlapIntervalsTy &IOL,
OrderedBasicBlock &OBB) {
MemoryDependenceResults *MD,
const TargetLibraryInfo *TLI,
InstOverlapIntervalsTy &IOL,
DenseMap<Instruction*, size_t> *InstrOrdering) {
bool MadeChange = false;
// Keep track of all of the stack objects that are dead at the end of the
@ -808,8 +809,7 @@ static bool handleEndBlock(BasicBlock &BB, AliasAnalysis *AA,
<< '\n');
// DCE instructions only used to calculate that store.
deleteDeadInstruction(Dead, &BBI, *MD, *TLI, IOL, OBB,
&DeadStackObjects);
deleteDeadInstruction(Dead, &BBI, *MD, *TLI, IOL, InstrOrdering, &DeadStackObjects);
++NumFastStores;
MadeChange = true;
continue;
@ -820,8 +820,7 @@ static bool handleEndBlock(BasicBlock &BB, AliasAnalysis *AA,
if (isInstructionTriviallyDead(&*BBI, TLI)) {
LLVM_DEBUG(dbgs() << "DSE: Removing trivially dead instruction:\n DEAD: "
<< *&*BBI << '\n');
deleteDeadInstruction(&*BBI, &BBI, *MD, *TLI, IOL, OBB,
&DeadStackObjects);
deleteDeadInstruction(&*BBI, &BBI, *MD, *TLI, IOL, InstrOrdering, &DeadStackObjects);
++NumFastOther;
MadeChange = true;
continue;
@ -1026,7 +1025,7 @@ static bool eliminateNoopStore(Instruction *Inst, BasicBlock::iterator &BBI,
const DataLayout &DL,
const TargetLibraryInfo *TLI,
InstOverlapIntervalsTy &IOL,
OrderedBasicBlock &OBB) {
DenseMap<Instruction*, size_t> *InstrOrdering) {
// Must be a store instruction.
StoreInst *SI = dyn_cast<StoreInst>(Inst);
if (!SI)
@ -1042,7 +1041,7 @@ static bool eliminateNoopStore(Instruction *Inst, BasicBlock::iterator &BBI,
dbgs() << "DSE: Remove Store Of Load from same pointer:\n LOAD: "
<< *DepLoad << "\n STORE: " << *SI << '\n');
deleteDeadInstruction(SI, &BBI, *MD, *TLI, IOL, OBB);
deleteDeadInstruction(SI, &BBI, *MD, *TLI, IOL, InstrOrdering);
++NumRedundantStores;
return true;
}
@ -1060,7 +1059,7 @@ static bool eliminateNoopStore(Instruction *Inst, BasicBlock::iterator &BBI,
dbgs() << "DSE: Remove null store to the calloc'ed object:\n DEAD: "
<< *Inst << "\n OBJECT: " << *UnderlyingPointer << '\n');
deleteDeadInstruction(SI, &BBI, *MD, *TLI, IOL, OBB);
deleteDeadInstruction(SI, &BBI, *MD, *TLI, IOL, InstrOrdering);
++NumRedundantStores;
return true;
}
@ -1074,8 +1073,11 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
const DataLayout &DL = BB.getModule()->getDataLayout();
bool MadeChange = false;
OrderedBasicBlock OBB(&BB);
Instruction *LastThrowing = nullptr;
// FIXME: Maybe change this to use some abstraction like OrderedBasicBlock?
// The current OrderedBasicBlock can't deal with mutation at the moment.
size_t LastThrowingInstIndex = 0;
DenseMap<Instruction*, size_t> InstrOrdering;
size_t InstrIndex = 1;
// A map of interval maps representing partially-overwritten value parts.
InstOverlapIntervalsTy IOL;
@ -1084,7 +1086,7 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
for (BasicBlock::iterator BBI = BB.begin(), BBE = BB.end(); BBI != BBE; ) {
// Handle 'free' calls specially.
if (CallInst *F = isFreeCall(&*BBI, TLI)) {
MadeChange |= handleFree(F, AA, MD, DT, TLI, IOL, OBB);
MadeChange |= handleFree(F, AA, MD, DT, TLI, IOL, &InstrOrdering);
// Increment BBI after handleFree has potentially deleted instructions.
// This ensures we maintain a valid iterator.
++BBI;
@ -1093,8 +1095,10 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
Instruction *Inst = &*BBI++;
size_t CurInstNumber = InstrIndex++;
InstrOrdering.insert(std::make_pair(Inst, CurInstNumber));
if (Inst->mayThrow()) {
LastThrowing = Inst;
LastThrowingInstIndex = CurInstNumber;
continue;
}
@ -1103,13 +1107,13 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
continue;
// eliminateNoopStore will update in iterator, if necessary.
if (eliminateNoopStore(Inst, BBI, AA, MD, DL, TLI, IOL, OBB)) {
if (eliminateNoopStore(Inst, BBI, AA, MD, DL, TLI, IOL, &InstrOrdering)) {
MadeChange = true;
continue;
}
// If we find something that writes memory, get its memory dependence.
MemDepResult InstDep = MD->getDependency(Inst, &OBB);
MemDepResult InstDep = MD->getDependency(Inst);
// Ignore any store where we can't find a local dependence.
// FIXME: cross-block DSE would be fun. :)
@ -1154,7 +1158,9 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
// If the underlying object is a non-escaping memory allocation, any store
// to it is dead along the unwind edge. Otherwise, we need to preserve
// the store.
if (LastThrowing && OBB.dominates(DepWrite, LastThrowing)) {
size_t DepIndex = InstrOrdering.lookup(DepWrite);
assert(DepIndex && "Unexpected instruction");
if (DepIndex <= LastThrowingInstIndex) {
const Value* Underlying = GetUnderlyingObject(DepLoc.Ptr, DL);
bool IsStoreDeadOnUnwind = isa<AllocaInst>(Underlying);
if (!IsStoreDeadOnUnwind) {
@ -1185,12 +1191,12 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
<< "\n KILLER: " << *Inst << '\n');
// Delete the store and now-dead instructions that feed it.
deleteDeadInstruction(DepWrite, &BBI, *MD, *TLI, IOL, OBB);
deleteDeadInstruction(DepWrite, &BBI, *MD, *TLI, IOL, &InstrOrdering);
++NumFastStores;
MadeChange = true;
// We erased DepWrite; start over.
InstDep = MD->getDependency(Inst, &OBB);
InstDep = MD->getDependency(Inst);
continue;
} else if ((OR == OW_End && isShortenableAtTheEnd(DepWrite)) ||
((OR == OW_Begin &&
@ -1258,11 +1264,14 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
++NumModifiedStores;
// Remove earlier, wider, store
OBB.replaceInstruction(DepWrite, SI);
size_t Idx = InstrOrdering.lookup(DepWrite);
InstrOrdering.erase(DepWrite);
InstrOrdering.insert(std::make_pair(SI, Idx));
// Delete the old stores and now-dead instructions that feed them.
deleteDeadInstruction(Inst, &BBI, *MD, *TLI, IOL, OBB);
deleteDeadInstruction(DepWrite, &BBI, *MD, *TLI, IOL, OBB);
deleteDeadInstruction(Inst, &BBI, *MD, *TLI, IOL, &InstrOrdering);
deleteDeadInstruction(DepWrite, &BBI, *MD, *TLI, IOL,
&InstrOrdering);
MadeChange = true;
// We erased DepWrite and Inst (Loc); start over.
@ -1297,7 +1306,7 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
// If this block ends in a return, unwind, or unreachable, all allocas are
// dead at its end, which means stores to them are also dead.
if (BB.getTerminator()->getNumSuccessors() == 0)
MadeChange |= handleEndBlock(BB, AA, MD, TLI, IOL, OBB);
MadeChange |= handleEndBlock(BB, AA, MD, TLI, IOL, &InstrOrdering);
return MadeChange;
}