Index: llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp =================================================================== --- llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp +++ llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp @@ -65,14 +65,18 @@ /// 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 *ValueSet = nullptr) { SmallVector 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 @@ 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 @@ 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 @@ // 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 @@ } 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 @@ 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 @@ // 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 @@ // 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 @@ // 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(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(SI->getValueOperand())) { if (SI->getPointerOperand() == DepLoad->getPointerOperand() && isRemovable(SI) && @@ -854,7 +848,9 @@ 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 @@ << "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 @@ << *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(*(--BBI))) - ; - break; + // We erased DepWrite; start over. + InstDep = MD->getDependency(Inst); + continue; } else if ((OR == OverwriteEnd && isShortenableAtTheEnd(DepWrite)) || ((OR == OverwriteBegin && isShortenableAtTheBeginning(DepWrite)))) { Index: llvm/trunk/test/Transforms/DeadStoreElimination/dse_with_dbg_value.ll =================================================================== --- llvm/trunk/test/Transforms/DeadStoreElimination/dse_with_dbg_value.ll +++ llvm/trunk/test/Transforms/DeadStoreElimination/dse_with_dbg_value.ll @@ -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 - -@g_31 = global i32 0 -@g_30 = global i16* null -@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)