[llvm] 37b96d5 - CodeGenPrep: remove AssertingVH references before deleting dead instructions.
Tim Northover via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 15 07:19:38 PDT 2020
Author: Tim Northover
Date: 2020-07-15T15:19:21+01:00
New Revision: 37b96d51d0cfc82a64598aaae2a567fa77e44de9
URL: https://github.com/llvm/llvm-project/commit/37b96d51d0cfc82a64598aaae2a567fa77e44de9
DIFF: https://github.com/llvm/llvm-project/commit/37b96d51d0cfc82a64598aaae2a567fa77e44de9.diff
LOG: CodeGenPrep: remove AssertingVH references before deleting dead instructions.
CodeGenPrepare keeps fairly close track of various instructions it's
seen, particularly GEPs, in maps and vectors. However, sometimes those
instructions become dead and get removed while it's still executing.
This triggers AssertingVH references to them in an asserts build and
could lead to miscompiles in a release build (I've only seen a later
segfault though).
So this patch adds a callback to
RecursivelyDeleteTriviallyDeadInstructions which can make sure the
instruction about to be deleted is removed from CodeGenPrepare's data
structures.
Added:
llvm/test/Transforms/CodeGenPrepare/ARM/dead-gep.ll
Modified:
llvm/include/llvm/Transforms/Utils/Local.h
llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/lib/Transforms/Utils/Local.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index f55e336f1f6a..3fab3bc21a07 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -160,7 +160,9 @@ bool wouldInstructionBeTriviallyDead(Instruction *I,
/// recursively. Return true if any instructions were deleted.
bool RecursivelyDeleteTriviallyDeadInstructions(
Value *V, const TargetLibraryInfo *TLI = nullptr,
- MemorySSAUpdater *MSSAU = nullptr);
+ MemorySSAUpdater *MSSAU = nullptr,
+ std::function<void(Value *)> AboutToDeleteCallback =
+ std::function<void(Value *)>());
/// Delete all of the instructions in `DeadInsts`, and all other instructions
/// that deleting these in turn causes to be trivially dead.
@@ -172,7 +174,9 @@ bool RecursivelyDeleteTriviallyDeadInstructions(
/// empty afterward.
void RecursivelyDeleteTriviallyDeadInstructions(
SmallVectorImpl<WeakTrackingVH> &DeadInsts,
- const TargetLibraryInfo *TLI = nullptr, MemorySSAUpdater *MSSAU = nullptr);
+ const TargetLibraryInfo *TLI = nullptr, MemorySSAUpdater *MSSAU = nullptr,
+ std::function<void(Value *)> AboutToDeleteCallback =
+ std::function<void(Value *)>());
/// Same functionality as RecursivelyDeleteTriviallyDeadInstructions, but allow
/// instructions that are not trivially dead. These will be ignored.
@@ -180,7 +184,9 @@ void RecursivelyDeleteTriviallyDeadInstructions(
/// were found and deleted.
bool RecursivelyDeleteTriviallyDeadInstructionsPermissive(
SmallVectorImpl<WeakTrackingVH> &DeadInsts,
- const TargetLibraryInfo *TLI = nullptr, MemorySSAUpdater *MSSAU = nullptr);
+ const TargetLibraryInfo *TLI = nullptr, MemorySSAUpdater *MSSAU = nullptr,
+ std::function<void(Value *)> AboutToDeleteCallback =
+ std::function<void(Value *)>());
/// If the specified value is an effectively dead PHI node, due to being a
/// def-use chain of single-use nodes that either forms a cycle or is terminated
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index e8b8e6c93cf0..465ba08dbfcd 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -376,6 +376,7 @@ class TypePromotionTransaction;
return *DT;
}
+ void removeAllAssertingVHReferences(Value *V);
bool eliminateFallThrough(Function &F);
bool eliminateMostlyEmptyBlocks(Function &F);
BasicBlock *findDestBlockOfMergeableEmptyBlock(BasicBlock *BB);
@@ -383,6 +384,7 @@ class TypePromotionTransaction;
void eliminateMostlyEmptyBlock(BasicBlock *BB);
bool isMergingEmptyBlockProfitable(BasicBlock *BB, BasicBlock *DestBB,
bool isPreheader);
+ bool makeBitReverse(Instruction &I);
bool optimizeBlock(BasicBlock &BB, bool &ModifiedDT);
bool optimizeInst(Instruction *I, bool &ModifiedDT);
bool optimizeMemoryInst(Instruction *MemoryInst, Value *Addr,
@@ -601,6 +603,33 @@ bool CodeGenPrepare::runOnFunction(Function &F) {
return EverMadeChange;
}
+/// An instruction is about to be deleted, so remove all references to it in our
+/// GEP-tracking data strcutures.
+void CodeGenPrepare::removeAllAssertingVHReferences(Value *V) {
+ LargeOffsetGEPMap.erase(V);
+ NewGEPBases.erase(V);
+
+ auto GEP = dyn_cast<GetElementPtrInst>(V);
+ if (!GEP)
+ return;
+
+ LargeOffsetGEPID.erase(GEP);
+
+ auto VecI = LargeOffsetGEPMap.find(GEP->getPointerOperand());
+ if (VecI == LargeOffsetGEPMap.end())
+ return;
+
+ auto &GEPVector = VecI->second;
+ const auto &I = std::find_if(GEPVector.begin(), GEPVector.end(),
+ [=](auto &Elt) { return Elt.first == GEP; });
+ if (I == GEPVector.end())
+ return;
+
+ GEPVector.erase(I);
+ if (GEPVector.empty())
+ LargeOffsetGEPMap.erase(VecI);
+}
+
// Verify BFI has been updated correctly by recomputing BFI and comparing them.
void LLVM_ATTRIBUTE_UNUSED CodeGenPrepare::verifyBFIUpdates(Function &F) {
DominatorTree NewDT(F);
@@ -5242,7 +5271,9 @@ bool CodeGenPrepare::optimizeMemoryInst(Instruction *MemoryInst, Value *Addr,
WeakTrackingVH IterHandle(CurValue);
BasicBlock *BB = CurInstIterator->getParent();
- RecursivelyDeleteTriviallyDeadInstructions(Repl, TLInfo);
+ RecursivelyDeleteTriviallyDeadInstructions(
+ Repl, TLInfo, nullptr,
+ [&](Value *V) { removeAllAssertingVHReferences(V); });
if (IterHandle != CurValue) {
// If the iterator instruction was recursively deleted, start over at the
@@ -5363,7 +5394,9 @@ bool CodeGenPrepare::optimizeGatherScatterInst(Instruction *MemoryInst,
// If we have no uses, recursively delete the value and all dead instructions
// using it.
if (Ptr->use_empty())
- RecursivelyDeleteTriviallyDeadInstructions(Ptr, TLInfo);
+ RecursivelyDeleteTriviallyDeadInstructions(
+ Ptr, TLInfo, nullptr,
+ [&](Value *V) { removeAllAssertingVHReferences(V); });
return true;
}
@@ -6647,7 +6680,8 @@ bool CodeGenPrepare::optimizeShuffleVectorInst(ShuffleVectorInst *SVI) {
Value *BC2 = Builder.CreateBitCast(Shuffle, SVIVecType);
SVI->replaceAllUsesWith(BC2);
- RecursivelyDeleteTriviallyDeadInstructions(SVI);
+ RecursivelyDeleteTriviallyDeadInstructions(
+ SVI, TLInfo, nullptr, [&](Value *V) { removeAllAssertingVHReferences(V); });
// Also hoist the bitcast up to its operand if it they are not in the same
// block.
@@ -7604,11 +7638,10 @@ bool CodeGenPrepare::optimizeInst(Instruction *I, bool &ModifiedDT) {
/// Given an OR instruction, check to see if this is a bitreverse
/// idiom. If so, insert the new intrinsic and return true.
-static bool makeBitReverse(Instruction &I, const DataLayout &DL,
- const TargetLowering &TLI) {
+bool CodeGenPrepare::makeBitReverse(Instruction &I) {
if (!I.getType()->isIntegerTy() ||
- !TLI.isOperationLegalOrCustom(ISD::BITREVERSE,
- TLI.getValueType(DL, I.getType(), true)))
+ !TLI->isOperationLegalOrCustom(ISD::BITREVERSE,
+ TLI->getValueType(*DL, I.getType(), true)))
return false;
SmallVector<Instruction*, 4> Insts;
@@ -7616,7 +7649,8 @@ static bool makeBitReverse(Instruction &I, const DataLayout &DL,
return false;
Instruction *LastInst = Insts.back();
I.replaceAllUsesWith(LastInst);
- RecursivelyDeleteTriviallyDeadInstructions(&I);
+ RecursivelyDeleteTriviallyDeadInstructions(
+ &I, TLInfo, nullptr, [&](Value *V) { removeAllAssertingVHReferences(V); });
return true;
}
@@ -7638,7 +7672,7 @@ bool CodeGenPrepare::optimizeBlock(BasicBlock &BB, bool &ModifiedDT) {
while (MadeBitReverse) {
MadeBitReverse = false;
for (auto &I : reverse(BB)) {
- if (makeBitReverse(I, *DL, *TLI)) {
+ if (makeBitReverse(I)) {
MadeBitReverse = MadeChange = true;
break;
}
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index da40c342af3a..3d163b8a86bc 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -453,21 +453,24 @@ bool llvm::wouldInstructionBeTriviallyDead(Instruction *I,
/// trivially dead, delete them too, recursively. Return true if any
/// instructions were deleted.
bool llvm::RecursivelyDeleteTriviallyDeadInstructions(
- Value *V, const TargetLibraryInfo *TLI, MemorySSAUpdater *MSSAU) {
+ Value *V, const TargetLibraryInfo *TLI, MemorySSAUpdater *MSSAU,
+ std::function<void(Value *)> AboutToDeleteCallback) {
Instruction *I = dyn_cast<Instruction>(V);
if (!I || !isInstructionTriviallyDead(I, TLI))
return false;
SmallVector<WeakTrackingVH, 16> DeadInsts;
DeadInsts.push_back(I);
- RecursivelyDeleteTriviallyDeadInstructions(DeadInsts, TLI, MSSAU);
+ RecursivelyDeleteTriviallyDeadInstructions(DeadInsts, TLI, MSSAU,
+ AboutToDeleteCallback);
return true;
}
bool llvm::RecursivelyDeleteTriviallyDeadInstructionsPermissive(
SmallVectorImpl<WeakTrackingVH> &DeadInsts, const TargetLibraryInfo *TLI,
- MemorySSAUpdater *MSSAU) {
+ MemorySSAUpdater *MSSAU,
+ std::function<void(Value *)> AboutToDeleteCallback) {
unsigned S = 0, E = DeadInsts.size(), Alive = 0;
for (; S != E; ++S) {
auto *I = cast<Instruction>(DeadInsts[S]);
@@ -478,13 +481,15 @@ bool llvm::RecursivelyDeleteTriviallyDeadInstructionsPermissive(
}
if (Alive == E)
return false;
- RecursivelyDeleteTriviallyDeadInstructions(DeadInsts, TLI, MSSAU);
+ RecursivelyDeleteTriviallyDeadInstructions(DeadInsts, TLI, MSSAU,
+ AboutToDeleteCallback);
return true;
}
void llvm::RecursivelyDeleteTriviallyDeadInstructions(
SmallVectorImpl<WeakTrackingVH> &DeadInsts, const TargetLibraryInfo *TLI,
- MemorySSAUpdater *MSSAU) {
+ MemorySSAUpdater *MSSAU,
+ std::function<void(Value *)> AboutToDeleteCallback) {
// Process the dead instruction list until empty.
while (!DeadInsts.empty()) {
Value *V = DeadInsts.pop_back_val();
@@ -498,6 +503,9 @@ void llvm::RecursivelyDeleteTriviallyDeadInstructions(
// Don't lose the debug info while deleting the instructions.
salvageDebugInfo(*I);
+ if (AboutToDeleteCallback)
+ AboutToDeleteCallback(I);
+
// Null out all of the instruction's operands to see if any operand becomes
// dead as we go.
for (Use &OpU : I->operands()) {
diff --git a/llvm/test/Transforms/CodeGenPrepare/ARM/dead-gep.ll b/llvm/test/Transforms/CodeGenPrepare/ARM/dead-gep.ll
new file mode 100644
index 000000000000..a82cce01a29f
--- /dev/null
+++ b/llvm/test/Transforms/CodeGenPrepare/ARM/dead-gep.ll
@@ -0,0 +1,19 @@
+; RUN: opt -codegenprepare -S %s -o - | FileCheck %s
+target triple = "thumbv7-apple-ios7.0.0"
+
+
+%struct = type [1000 x i32]
+
+define void @test_dead_gep(%struct* %t0) {
+; CHECK-LABEL: define void @test_dead_gep
+; CHECK-NOT: getelementptr
+; CHECK: %t16 = load i32, i32* undef
+; CHECK: ret void
+
+ %t12 = getelementptr inbounds %struct, %struct* %t0, i32 1, i32 500
+ %t13 = load i32, i32* %t12, align 4
+ %t14 = icmp eq i32 %t13, 2
+ %t15 = select i1 %t14, i32* undef, i32* undef
+ %t16 = load i32, i32* %t15, align 4
+ ret void
+}
More information about the llvm-commits
mailing list