[llvm] r274660 - [DSE] Avoid iterator invalidation bugs.
Chad Rosier via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 6 12:48:53 PDT 2016
Author: mcrosier
Date: Wed Jul 6 14:48:52 2016
New Revision: 274660
URL: http://llvm.org/viewvc/llvm-project?rev=274660&view=rev
Log:
[DSE] Avoid iterator invalidation bugs.
The dse_with_dbg_value.ll test committed with r273141 is removed because this
we no longer performs any type of back tracking, which is what was causing the
codegen differences with and without debug information.
Differential Revision: http://reviews.llvm.org/D21613
Removed:
llvm/trunk/test/Transforms/DeadStoreElimination/dse_with_dbg_value.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp
Modified: llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp?rev=274660&r1=274659&r2=274660&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp Wed Jul 6 14:48:52 2016
@@ -65,14 +65,18 @@ EnablePartialOverwriteTracking("enable-d
/// the computation tree that feeds them.
/// If ValueSet is non-null, remove any deleted instructions from it as well.
static void
-deleteDeadInstruction(Instruction *I, MemoryDependenceResults &MD,
- const TargetLibraryInfo &TLI,
+deleteDeadInstruction(Instruction *I, BasicBlock::iterator *BBI,
+ MemoryDependenceResults &MD, const TargetLibraryInfo &TLI,
SmallSetVector<Value *, 16> *ValueSet = nullptr) {
SmallVector<Instruction*, 32> NowDeadInsts;
NowDeadInsts.push_back(I);
--NumFastOther;
+ // Keeping the iterator straight is a pain, so we let this routine tell the
+ // caller what the next instruction is after we're done mucking about.
+ BasicBlock::iterator NewIter = *BBI;
+
// Before we touch this instruction, remove it from memdep!
do {
Instruction *DeadInst = NowDeadInsts.pop_back_val();
@@ -95,10 +99,15 @@ deleteDeadInstruction(Instruction *I, Me
NowDeadInsts.push_back(OpI);
}
- DeadInst->eraseFromParent();
+
+ if (NewIter == DeadInst->getIterator())
+ NewIter = DeadInst->eraseFromParent();
+ else
+ DeadInst->eraseFromParent();
if (ValueSet) ValueSet->remove(DeadInst);
} while (!NowDeadInsts.empty());
+ *BBI = NewIter;
}
/// Does this instruction write some memory? This only returns true for things
@@ -603,10 +612,9 @@ static bool handleFree(CallInst *F, Alia
if (!AA->isMustAlias(F->getArgOperand(0), DepPointer))
break;
- auto Next = ++Dependency->getIterator();
-
// DCE instructions only used to calculate that store.
- deleteDeadInstruction(Dependency, *MD, *TLI);
+ BasicBlock::iterator BBI(Dependency);
+ deleteDeadInstruction(Dependency, &BBI, *MD, *TLI);
++NumFastStores;
MadeChange = true;
@@ -615,7 +623,7 @@ static bool handleFree(CallInst *F, Alia
// s[0] = 0;
// s[1] = 0; // This has just been deleted.
// free(s);
- Dep = MD->getPointerDependencyFrom(Loc, false, Next, BB);
+ Dep = MD->getPointerDependencyFrom(Loc, false, BBI, BB);
}
if (Dep.isNonLocal())
@@ -707,7 +715,7 @@ static bool handleEndBlock(BasicBlock &B
}
if (AllDead) {
- Instruction *Dead = &*BBI++;
+ Instruction *Dead = &*BBI;
DEBUG(dbgs() << "DSE: Dead Store at End of Block:\n DEAD: "
<< *Dead << "\n Objects: ";
@@ -720,7 +728,7 @@ static bool handleEndBlock(BasicBlock &B
dbgs() << '\n');
// DCE instructions only used to calculate that store.
- deleteDeadInstruction(Dead, *MD, *TLI, &DeadStackObjects);
+ deleteDeadInstruction(Dead, &BBI, *MD, *TLI, &DeadStackObjects);
++NumFastStores;
MadeChange = true;
continue;
@@ -729,8 +737,7 @@ static bool handleEndBlock(BasicBlock &B
// Remove any dead non-memory-mutating instructions.
if (isInstructionTriviallyDead(&*BBI, TLI)) {
- Instruction *Inst = &*BBI++;
- deleteDeadInstruction(Inst, *MD, *TLI, &DeadStackObjects);
+ deleteDeadInstruction(&*BBI, &BBI, *MD, *TLI, &DeadStackObjects);
++NumFastOther;
MadeChange = true;
continue;
@@ -815,14 +822,17 @@ static bool eliminateDeadStores(BasicBlo
// Do a top-down walk on the BB.
for (BasicBlock::iterator BBI = BB.begin(), BBE = BB.end(); BBI != BBE; ) {
- Instruction *Inst = &*BBI++;
-
// Handle 'free' calls specially.
- if (CallInst *F = isFreeCall(Inst, TLI)) {
+ if (CallInst *F = isFreeCall(&*BBI, TLI)) {
MadeChange |= handleFree(F, AA, MD, DT, TLI);
+ // Increment BBI after handleFree has potentially deleted instructions.
+ // This ensures we maintain a valid iterator.
+ ++BBI;
continue;
}
+ Instruction *Inst = &*BBI++;
+
// If we find something that writes memory, get its memory dependence.
if (!hasMemoryWrite(Inst, *TLI))
continue;
@@ -830,22 +840,6 @@ static bool eliminateDeadStores(BasicBlo
// If we're storing the same value back to a pointer that we just
// loaded from, then the store can be removed.
if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
-
- auto RemoveDeadInstAndUpdateBBI = [&](Instruction *DeadInst) {
- // deleteDeadInstruction can delete the current instruction. Save BBI
- // in case we need it.
- WeakVH NextInst(&*BBI);
-
- deleteDeadInstruction(DeadInst, *MD, *TLI);
-
- if (!NextInst) // Next instruction deleted.
- BBI = BB.begin();
- else if (BBI != BB.begin()) // Revisit this instruction if possible.
- --BBI;
- ++NumRedundantStores;
- MadeChange = true;
- };
-
if (LoadInst *DepLoad = dyn_cast<LoadInst>(SI->getValueOperand())) {
if (SI->getPointerOperand() == DepLoad->getPointerOperand() &&
isRemovable(SI) &&
@@ -854,7 +848,9 @@ static bool eliminateDeadStores(BasicBlo
DEBUG(dbgs() << "DSE: Remove Store Of Load from same pointer:\n "
<< "LOAD: " << *DepLoad << "\n STORE: " << *SI << '\n');
- RemoveDeadInstAndUpdateBBI(SI);
+ deleteDeadInstruction(SI, &BBI, *MD, *TLI);
+ ++NumRedundantStores;
+ MadeChange = true;
continue;
}
}
@@ -873,7 +869,9 @@ static bool eliminateDeadStores(BasicBlo
<< "DSE: Remove null store to the calloc'ed object:\n DEAD: "
<< *Inst << "\n OBJECT: " << *UnderlyingPointer << '\n');
- RemoveDeadInstAndUpdateBBI(SI);
+ deleteDeadInstruction(SI, &BBI, *MD, *TLI);
+ ++NumRedundantStores;
+ MadeChange = true;
continue;
}
}
@@ -921,17 +919,13 @@ static bool eliminateDeadStores(BasicBlo
<< *DepWrite << "\n KILLER: " << *Inst << '\n');
// Delete the store and now-dead instructions that feed it.
- deleteDeadInstruction(DepWrite, *MD, *TLI);
+ deleteDeadInstruction(DepWrite, &BBI, *MD, *TLI);
++NumFastStores;
MadeChange = true;
- // deleteDeadInstruction can delete the current instruction in loop
- // cases, reset BBI.
- BBI = Inst->getIterator();
- auto BBBegin = BB.begin();
- while (BBI != BBBegin && isa<DbgInfoIntrinsic>(*(--BBI)))
- ;
- break;
+ // We erased DepWrite; start over.
+ InstDep = MD->getDependency(Inst);
+ continue;
} else if ((OR == OverwriteEnd && isShortenableAtTheEnd(DepWrite)) ||
((OR == OverwriteBegin &&
isShortenableAtTheBeginning(DepWrite)))) {
Removed: llvm/trunk/test/Transforms/DeadStoreElimination/dse_with_dbg_value.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadStoreElimination/dse_with_dbg_value.ll?rev=274659&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/DeadStoreElimination/dse_with_dbg_value.ll (original)
+++ llvm/trunk/test/Transforms/DeadStoreElimination/dse_with_dbg_value.ll (removed)
@@ -1,83 +0,0 @@
-; RUN: opt -basicaa -dse -S < %s | FileCheck %s
-; RUN: opt -strip-debug -basicaa -dse -S < %s | FileCheck %s
-
-; Test that stores are removed both with and without debug info.
-
-; CHECK-NOT: store i32 4, i32* @g_31, align 1
-; CHECK-NOT: %_tmp17.i.i = load i16, i16* %_tmp16.i.i, align 1
-; CHECK-NOT: store i16 %_tmp17.i.i, i16* @g_118, align 1
-; CHECK: store i32 0, i32* @g_31, align 1
-
- at g_31 = global i32 0
- at g_30 = global i16* null
- at g_118 = global i16 0
-
-define i16 @S0() !dbg !17 {
-bb1:
- store i32 4, i32* @g_31, align 1, !dbg !20
- %_tmp16.i.i = load volatile i16*, i16** @g_30, align 1, !dbg !28
- %_tmp17.i.i = load i16, i16* %_tmp16.i.i, align 1, !dbg !28
- store i16 %_tmp17.i.i, i16* @g_118, align 1, !dbg !20
- store i32 0, i32* @g_31, align 1, !dbg !31
- tail call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !40, metadata !41), !dbg !42
- store i16 0, i16* @g_118, align 1, !dbg !43
- br label %bb2.i, !dbg !44
-
-bb2.i:
- br label %bb2.i, !dbg !44
-}
-
-; Function Attrs: nounwind readnone
-declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #0
-
-attributes #0 = { nounwind readnone }
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!14, !15}
-!llvm.ident = !{!16}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "FlexASIC FlexC Compiler v6.38 for FADER (LLVM)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2, globals: !3)
-!1 = !DIFile(filename: "csmith23219270180033.c", directory: "/local/repo/uabsson/llvm")
-!2 = !{}
-!3 = !{!4, !9, !13}
-!4 = !DIGlobalVariable(name: "g_31", scope: null, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, variable: i32* @g_31)
-!5 = !DIDerivedType(tag: DW_TAG_typedef, name: "int32_t", file: !6, line: 104, baseType: !7)
-!6 = !DIFile(filename: "/local/repo/uabsson/llvm/sdk-bin/cosy/fader2_sdk/compiler/fader2_arch/fader2_2/include/stdint.h", directory: "/local/repo/uabsson/llvm")
-!7 = !DIDerivedType(tag: DW_TAG_typedef, name: "__i32_t", file: !1, baseType: !8)
-!8 = !DIBasicType(name: "signed long", size: 32, align: 16, encoding: DW_ATE_signed)
-!9 = !DIGlobalVariable(name: "g_30", scope: null, file: !1, line: 4, type: !10, isLocal: false, isDefinition: true, variable: i16** @g_30)
-!10 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !11)
-!11 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !12, size: 16, align: 16)
-!12 = !DIBasicType(name: "int", size: 16, align: 16, encoding: DW_ATE_signed)
-!13 = !DIGlobalVariable(name: "g_118", scope: null, file: !1, line: 5, type: !12, isLocal: false, isDefinition: true, variable: i16* @g_118)
-!14 = !{i32 2, !"Dwarf Version", i32 4}
-!15 = !{i32 2, !"Debug Info Version", i32 3}
-!16 = !{!"FlexASIC FlexC Compiler v6.38 for FADER (CoSy 6231.35) (LLVM)"}
-!17 = distinct !DISubprogram(name: "S0", scope: !1, file: !1, line: 10, type: !18, isLocal: false, isDefinition: true, scopeLine: 10, isOptimized: false, unit: !0, variables: !2)
-!18 = !DISubroutineType(types: !19)
-!19 = !{!12}
-!20 = !DILocation(line: 14, column: 3, scope: !21, inlinedAt: !27)
-!21 = distinct !DISubprogram(name: "func_54", scope: !1, file: !1, line: 12, type: !22, isLocal: false, isDefinition: true, scopeLine: 12, isOptimized: false, unit: !0, variables: !2)
-!22 = !DISubroutineType(types: !23)
-!23 = !{!24, !12, !12}
-!24 = !DIDerivedType(tag: DW_TAG_typedef, name: "uint64_t", file: !6, line: 107, baseType: !25)
-!25 = !DIDerivedType(tag: DW_TAG_typedef, name: "__u64_t", file: !1, baseType: !26)
-!26 = !DIBasicType(name: "unsigned long long", size: 64, align: 16, encoding: DW_ATE_unsigned)
-!27 = distinct !DILocation(line: 10, column: 8, scope: !17)
-!28 = !DILocation(line: 8, column: 12, scope: !29, inlinedAt: !30)
-!29 = distinct !DISubprogram(name: "func_9", scope: !1, file: !1, line: 8, type: !18, isLocal: false, isDefinition: true, scopeLine: 8, isOptimized: false, unit: !0, variables: !2)
-!30 = distinct !DILocation(line: 14, column: 3, scope: !21, inlinedAt: !27)
-!31 = !DILocation(line: 20, column: 8, scope: !32, inlinedAt: !39)
-!32 = distinct !DISubprogram(name: "func_61", scope: !1, file: !1, line: 19, type: !33, isLocal: false, isDefinition: true, scopeLine: 19, isOptimized: false, unit: !0, variables: !2)
-!33 = !DISubroutineType(types: !34)
-!34 = !{!35, !36, !5, !35, !35}
-!35 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !5, size: 16, align: 16)
-!36 = !DIDerivedType(tag: DW_TAG_typedef, name: "uint32_t", file: !6, line: 105, baseType: !37)
-!37 = !DIDerivedType(tag: DW_TAG_typedef, name: "__u32_t", file: !1, baseType: !38)
-!38 = !DIBasicType(name: "unsigned long", size: 32, align: 16, encoding: DW_ATE_unsigned)
-!39 = distinct !DILocation(line: 14, column: 3, scope: !21, inlinedAt: !27)
-!40 = !DILocalVariable(name: "p_63", arg: 2, scope: !32, line: 19, type: !5)
-!41 = !DIExpression()
-!42 = !DILocation(line: 19, column: 41, scope: !32, inlinedAt: !39)
-!43 = !DILocation(line: 15, column: 10, scope: !21, inlinedAt: !27)
-!44 = !DILocation(line: 15, column: 20, scope: !21, inlinedAt: !27)
More information about the llvm-commits
mailing list