[llvm] r357211 - Revert [DSE] Preserve basic block ordering using OrderedBasicBlock.
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 28 13:36:24 PDT 2019
Author: fhahn
Date: Thu Mar 28 13:36:24 2019
New Revision: 357211
URL: http://llvm.org/viewvc/llvm-project?rev=357211&view=rev
Log:
Revert [DSE] Preserve basic block ordering using OrderedBasicBlock.
This reverts r357208 (git commit c0bfd37d385c93711ef3a349599dba20e6b101ef)
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.
Modified:
llvm/trunk/include/llvm/Analysis/OrderedBasicBlock.h
llvm/trunk/lib/Analysis/OrderedBasicBlock.cpp
llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp
Modified: llvm/trunk/include/llvm/Analysis/OrderedBasicBlock.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/OrderedBasicBlock.h?rev=357211&r1=357210&r2=357211&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/OrderedBasicBlock.h (original)
+++ llvm/trunk/include/llvm/Analysis/OrderedBasicBlock.h Thu Mar 28 13:36:24 2019
@@ -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
Modified: llvm/trunk/lib/Analysis/OrderedBasicBlock.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/OrderedBasicBlock.cpp?rev=357211&r1=357210&r2=357211&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/OrderedBasicBlock.cpp (original)
+++ llvm/trunk/lib/Analysis/OrderedBasicBlock.cpp Thu Mar 28 13:36:24 2019
@@ -85,26 +85,3 @@ bool OrderedBasicBlock::dominates(const
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);
-}
Modified: llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp?rev=357211&r1=357210&r2=357211&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp Thu Mar 28 13:36:24 2019
@@ -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<
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, Ba
}
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(Small
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, Alia
// 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
/// 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 &B
<< '\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 &B
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(Instructi
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(Instructi
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(Instructi
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(BasicBlo
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(BasicBlo
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(BasicBlo
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(BasicBlo
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(BasicBlo
// 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(BasicBlo
<< "\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(BasicBlo
++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(BasicBlo
// 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;
}
More information about the llvm-commits
mailing list