[llvm] 1907117 - Revert "[DSE] Fix for a dangling point bug in DeadStoreElimination."
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 5 11:30:11 PST 2019
Author: Florian Hahn
Date: 2019-12-05T19:29:21Z
New Revision: 19071173fc2e0c30c898a8cf8e42a0eaf3a3a35c
URL: https://github.com/llvm/llvm-project/commit/19071173fc2e0c30c898a8cf8e42a0eaf3a3a35c
DIFF: https://github.com/llvm/llvm-project/commit/19071173fc2e0c30c898a8cf8e42a0eaf3a3a35c.diff
LOG: Revert "[DSE] Fix for a dangling point bug in DeadStoreElimination."
The commit causes a failure:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/20911
This reverts commit 1847fd9d85506ecee692230cb2500e3774ec628e.
Added:
Modified:
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
Removed:
llvm/test/Transforms/DeadStoreElimination/DeleteThrowableInst.ll
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index c4c9a960c591..e607ad13447c 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -17,7 +17,6 @@
#include "llvm/Transforms/Scalar/DeadStoreElimination.h"
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
@@ -101,7 +100,6 @@ static void
deleteDeadInstruction(Instruction *I, BasicBlock::iterator *BBI,
MemoryDependenceResults &MD, const TargetLibraryInfo &TLI,
InstOverlapIntervalsTy &IOL, OrderedBasicBlock &OBB,
- MapVector<Instruction *, bool> &ThrowableInst,
SmallSetVector<const Value *, 16> *ValueSet = nullptr) {
SmallVector<Instruction*, 32> NowDeadInsts;
@@ -115,10 +113,6 @@ deleteDeadInstruction(Instruction *I, BasicBlock::iterator *BBI,
// Before we touch this instruction, remove it from memdep!
do {
Instruction *DeadInst = NowDeadInsts.pop_back_val();
- // Mark the DeadInst as dead in the list of throwable instructions.
- auto It = ThrowableInst.find(DeadInst);
- if (It != ThrowableInst.end())
- ThrowableInst[It->first] = false;
++NumFastOther;
// Try to preserve debug information attached to the dead instruction.
@@ -151,12 +145,6 @@ deleteDeadInstruction(Instruction *I, BasicBlock::iterator *BBI,
DeadInst->eraseFromParent();
} while (!NowDeadInsts.empty());
*BBI = NewIter;
- // Pop dead entries from back of ThrowableInst till we find an alive entry.
- for (auto It = ThrowableInst.rbegin();
- It != ThrowableInst.rend() && !It->second;) {
- ++It;
- ThrowableInst.pop_back();
- }
}
/// Does this instruction write some memory? This only returns true for things
@@ -669,8 +657,7 @@ static void findUnconditionalPreds(SmallVectorImpl<BasicBlock *> &Blocks,
static bool handleFree(CallInst *F, AliasAnalysis *AA,
MemoryDependenceResults *MD, DominatorTree *DT,
const TargetLibraryInfo *TLI,
- InstOverlapIntervalsTy &IOL, OrderedBasicBlock &OBB,
- MapVector<Instruction *, bool> &ThrowableInst) {
+ InstOverlapIntervalsTy &IOL, OrderedBasicBlock &OBB) {
bool MadeChange = false;
MemoryLocation Loc = MemoryLocation(F->getOperand(0));
@@ -704,8 +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,
- ThrowableInst);
+ deleteDeadInstruction(Dependency, &BBI, *MD, *TLI, IOL, OBB);
++NumFastStores;
MadeChange = true;
@@ -762,8 +748,8 @@ static void removeAccessedObjects(const MemoryLocation &LoadedLoc,
static bool handleEndBlock(BasicBlock &BB, AliasAnalysis *AA,
MemoryDependenceResults *MD,
const TargetLibraryInfo *TLI,
- InstOverlapIntervalsTy &IOL, OrderedBasicBlock &OBB,
- MapVector<Instruction *, bool> &ThrowableInst) {
+ InstOverlapIntervalsTy &IOL,
+ OrderedBasicBlock &OBB) {
bool MadeChange = false;
// Keep track of all of the stack objects that are dead at the end of the
@@ -824,7 +810,7 @@ static bool handleEndBlock(BasicBlock &BB, AliasAnalysis *AA,
<< '\n');
// DCE instructions only used to calculate that store.
- deleteDeadInstruction(Dead, &BBI, *MD, *TLI, IOL, OBB, ThrowableInst,
+ deleteDeadInstruction(Dead, &BBI, *MD, *TLI, IOL, OBB,
&DeadStackObjects);
++NumFastStores;
MadeChange = true;
@@ -836,7 +822,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, ThrowableInst,
+ deleteDeadInstruction(&*BBI, &BBI, *MD, *TLI, IOL, OBB,
&DeadStackObjects);
++NumFastOther;
MadeChange = true;
@@ -1043,8 +1029,7 @@ static bool eliminateNoopStore(Instruction *Inst, BasicBlock::iterator &BBI,
const DataLayout &DL,
const TargetLibraryInfo *TLI,
InstOverlapIntervalsTy &IOL,
- OrderedBasicBlock &OBB,
- MapVector<Instruction *, bool> &ThrowableInst) {
+ OrderedBasicBlock &OBB) {
// Must be a store instruction.
StoreInst *SI = dyn_cast<StoreInst>(Inst);
if (!SI)
@@ -1060,7 +1045,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, ThrowableInst);
+ deleteDeadInstruction(SI, &BBI, *MD, *TLI, IOL, OBB);
++NumRedundantStores;
return true;
}
@@ -1078,7 +1063,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, ThrowableInst);
+ deleteDeadInstruction(SI, &BBI, *MD, *TLI, IOL, OBB);
++NumRedundantStores;
return true;
}
@@ -1093,7 +1078,7 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
bool MadeChange = false;
OrderedBasicBlock OBB(&BB);
- MapVector<Instruction *, bool> ThrowableInst;
+ Instruction *LastThrowing = nullptr;
// A map of interval maps representing partially-overwritten value parts.
InstOverlapIntervalsTy IOL;
@@ -1102,7 +1087,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, ThrowableInst);
+ MadeChange |= handleFree(F, AA, MD, DT, TLI, IOL, OBB);
// Increment BBI after handleFree has potentially deleted instructions.
// This ensures we maintain a valid iterator.
++BBI;
@@ -1112,7 +1097,7 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
Instruction *Inst = &*BBI++;
if (Inst->mayThrow()) {
- ThrowableInst[Inst] = true;
+ LastThrowing = Inst;
continue;
}
@@ -1121,8 +1106,7 @@ 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,
- ThrowableInst)) {
+ if (eliminateNoopStore(Inst, BBI, AA, MD, DL, TLI, IOL, OBB)) {
MadeChange = true;
continue;
}
@@ -1165,12 +1149,6 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
if (!DepLoc.Ptr)
break;
- // Find the last throwable instruction not removed by call to
- // deleteDeadInstruction.
- Instruction *LastThrowing = nullptr;
- if (!ThrowableInst.empty())
- LastThrowing = ThrowableInst.back().first;
-
// Make sure we don't look past a call which might throw. This is an
// issue because MemoryDependenceAnalysis works in the wrong direction:
// it finds instructions which dominate the current instruction, rather than
@@ -1210,8 +1188,7 @@ 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,
- ThrowableInst);
+ deleteDeadInstruction(DepWrite, &BBI, *MD, *TLI, IOL, OBB);
++NumFastStores;
MadeChange = true;
@@ -1293,10 +1270,8 @@ static bool eliminateDeadStores(BasicBlock &BB, AliasAnalysis *AA,
OBB.replaceInstruction(DepWrite, SI);
// Delete the old stores and now-dead instructions that feed them.
- deleteDeadInstruction(Inst, &BBI, *MD, *TLI, IOL, OBB,
- ThrowableInst);
- deleteDeadInstruction(DepWrite, &BBI, *MD, *TLI, IOL, OBB,
- ThrowableInst);
+ deleteDeadInstruction(Inst, &BBI, *MD, *TLI, IOL, OBB);
+ deleteDeadInstruction(DepWrite, &BBI, *MD, *TLI, IOL, OBB);
MadeChange = true;
// We erased DepWrite and Inst (Loc); start over.
@@ -1331,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, ThrowableInst);
+ MadeChange |= handleEndBlock(BB, AA, MD, TLI, IOL, OBB);
return MadeChange;
}
diff --git a/llvm/test/Transforms/DeadStoreElimination/DeleteThrowableInst.ll b/llvm/test/Transforms/DeadStoreElimination/DeleteThrowableInst.ll
deleted file mode 100644
index f392b2e946a8..000000000000
--- a/llvm/test/Transforms/DeadStoreElimination/DeleteThrowableInst.ll
+++ /dev/null
@@ -1,41 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -basicaa -dse -S | FileCheck %s
-
-declare i8* @_Znwj(i32) local_unnamed_addr
-declare void @foo() readnone
-
-define void @test1(i8** %ptr) {
-; CHECK-LABEL: @test1(
-; CHECK-NEXT: [[VAL:%.*]] = inttoptr i64 23452 to i8*
-; CHECK-NEXT: store i8* [[VAL]], i8** [[PTR:%.*]]
-; CHECK-NEXT: ret void
-;
- %val = inttoptr i64 23452 to i8*
- store i8* %val, i8** %ptr
- %call = call i8* @_Znwj(i32 1)
- store i8* %call, i8** %ptr
- store i8* %val, i8** %ptr
- ret void
-}
-
-define void @test2(i8** %ptr, i8* %p1, i8* %p2) {
-; CHECK-LABEL: @test2(
-; CHECK-NEXT: [[VAL:%.*]] = inttoptr i64 23452 to i8*
-; CHECK-NEXT: store i8* [[VAL]], i8** [[PTR:%.*]]
-; CHECK-NEXT: call void @foo()
-; CHECK-NEXT: store i8* [[P1:%.*]], i8** [[PTR]]
-; CHECK-NEXT: call void @foo()
-; CHECK-NEXT: store i8* [[VAL]], i8** [[PTR]]
-; CHECK-NEXT: ret void
-;
- %val = inttoptr i64 23452 to i8*
- store i8* %val, i8** %ptr
- call void @foo()
- store i8* %p1, i8** %ptr
- call void @foo()
- store i8* %p2, i8** %ptr
- %call = call i8* @_Znwj(i32 1)
- store i8* %call, i8** %ptr
- store i8* %val, i8** %ptr
- ret void
-}
More information about the llvm-commits
mailing list