diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -2582,10 +2582,10 @@ // If we are removing an alloca with a dbg.declare, insert dbg.value calls // before each store. - TinyPtrVector DIIs; + SmallVector DVIs; std::unique_ptr DIB; if (isa(MI)) { - DIIs = FindDbgAddrUses(&MI); + findDbgUsers(DVIs, &MI); DIB.reset(new DIBuilder(*MI.getModule(), /*AllowUnresolved=*/false)); } @@ -2619,8 +2619,9 @@ ConstantInt::get(Type::getInt1Ty(C->getContext()), C->isFalseWhenEqual())); } else if (auto *SI = dyn_cast(I)) { - for (auto *DII : DIIs) - ConvertDebugDeclareToDebugValue(DII, SI, *DIB); + for (auto *DVI : DVIs) + if (DVI->isAddressOfVariable()) + ConvertDebugDeclareToDebugValue(DVI, SI, *DIB); } else { // Casts, GEP, or anything else: we're about to delete this instruction, // so it can not have any valid uses. @@ -2637,8 +2638,52 @@ None, "", II->getParent()); } - for (auto *DII : DIIs) - eraseInstFromFunction(*DII); + // Remove debug intrinsics which describe the value contained within the + // alloca. In addition to removing dbg.{delcare,addr} which simply point to + // the alloca, remove dbg.value(, ..., DW_OP_deref)'s as well. + // + // This effectively undoes work performed in an InstCombine run earlier in + // the pipeline by LowerDbgDeclare, which inserts DW_OP_deref dbg.values + // before CallInst users of an alloca. The motivating example looks like: + // + // ``` + // define void @foo(i32 %0) { + // %a = alloca i32 ; This alloca is erased. + // store i32 %0, i32* %a + // dbg.value(i32 %0, "arg0") ; This dbg.value survives. + // dbg.value(i32* %a, "arg0", DW_OP_deref) + // call void @trivially_inlinable_no_op(i32* %a) + // ret void + // } + // ``` + // + // If the DW_OP_deref dbg.value is not erased, it becomes dbg.value(undef) + // after inlining, making "arg0" unavailable. But we already have dbg.value + // descriptions of the alloca's value (from LowerDbgDeclare), so the + // DW_OP_deref dbg.value cannot serve its purpose of describing an + // initialization of the alloca by some callee. It invalidates other useful + // dbg.values for no reason, so it should be deleted. + // + // OTOH, it wouldn't be correct to delete all dbg.value descriptions of an + // alloca. Note that it's possible to describe a variable that takes on + // different pointer values, e.g.: + // + // ``` + // void use(int *); + // void t(int a, int b) { + // int *local = &a; // dbg.value(i32* %a.addr, "local") + // local = &b; // dbg.value(i32* undef, "local") + // use(&a); // (note: %b.addr is optimized out) + // local = &a; // dbg.value(i32* %a.addr, "local") + // } + // ``` + // + // In this example, the alloca for "b" is erased, but we need to describe + // the value of "local" as before the call to "use". This + // prevents "local" from appearing to be equal to "&a" at the callsite. + for (auto *DVI : DVIs) + if (DVI->isAddressOfVariable() || DVI->getExpression()->startsWithDeref()) + DVI->eraseFromParent(); return eraseInstFromFunction(MI); } diff --git a/llvm/test/Transforms/InstCombine/erase-dbg-values-at-dead-alloc-site.ll b/llvm/test/Transforms/InstCombine/erase-dbg-values-at-dead-alloc-site.ll new file mode 100644 --- /dev/null +++ b/llvm/test/Transforms/InstCombine/erase-dbg-values-at-dead-alloc-site.ll @@ -0,0 +1,85 @@ +; RUN: opt -S -instcombine %s | FileCheck %s -check-prefix=RUN-ONCE +; RUN: opt -S -instcombine -inline -instcombine %s | FileCheck %s -check-prefix=RUN-TWICE + +; This example was reduced from a test case in which InstCombine ran at least +; twice: +; - The first InstCombine run converted dbg.declares to dbg.values using the +; LowerDbgDeclare utility. This produced a dbg.value(i32* %2, DW_OP_deref) +; (this happens when the contents of an alloca are passed by-value), and a +; dbg.value(i32 %0) (due to the store of %0 into the alloca). +; - The second InstCombine run deleted the alloca (%2). +; Check that the DW_OP_deref dbg.value is deleted, just like a dbg.declare would +; be. +; +; RUN-ONCE-LABEL: @t1( +; RUN-ONCE-NEXT: llvm.dbg.value(metadata i32 %0, metadata [[t1_arg0:![0-9]+]], metadata !DIExpression()) +; RUN-ONCE-NEXT: llvm.dbg.value(metadata i32* undef, metadata [[t1_fake_ptr:![0-9]+]], metadata !DIExpression()) +; RUN-ONCE-NEXT: ret void +define void @t1(i32) !dbg !9 { + %2 = alloca i32, align 4 + store i32 %0, i32* %2, align 4 + call void @llvm.dbg.value(metadata i32 %0, metadata !14, metadata !DIExpression()), !dbg !15 + call void @llvm.dbg.value(metadata i32* %2, metadata !14, metadata !DIExpression(DW_OP_deref)), !dbg !15 + call void @llvm.dbg.value(metadata i32* %2, metadata !20, metadata !DIExpression()), !dbg !15 + ret void +} + +; This example is closer to an end-to-end test: the IR looks like it could have +; been produced by a frontend compiling at -O0. +; +; Here's what happens: +; 1) We run InstCombine. This puts a dbg.value(i32* %x.addr, DW_OP_deref) +; before the call to @use, and a dbg.value(i32 %x) after the store. +; 2) We inline @use. +; 3) We run InstCombine again. The alloca %x.addr is erased. We should just get +; dbg.value(i32 %x). There should be no leftover dbg.value(metadata i32* +; undef). +; +; RUN-ONCE-LABEL: @t2( +; RUN-ONCE: call void @llvm.dbg.value(metadata i32 %x, metadata [[x:![0-9]+]], metadata !DIExpression()) +; RUN-ONCE: call void @llvm.dbg.value(metadata i32* %x.addr, metadata [[x]], metadata !DIExpression(DW_OP_deref)) +; RUN-ONCE-NEXT: call void @use(i32* {{.*}}%x.addr) +; +; RUN-TWICE-LABEL: @t2( +; RUN-TWICE-NEXT: llvm.dbg.value(metadata i32 %x, {{.*}}!DIExpression()) +; RUN-TWICE-NEXT: ret void +define void @use(i32* %addr) alwaysinline { ret void } +define void @t2(i32 %x) !dbg !17 { + %x.addr = alloca i32, align 4 + store i32 %x, i32* %x.addr, align 4 + call void @llvm.dbg.declare(metadata i32* %x.addr, metadata !18, metadata !DIExpression()), !dbg !19 + call void @use(i32* %x.addr) + ret void +} + +declare void @llvm.dbg.value(metadata, metadata, metadata) +declare void @llvm.dbg.declare(metadata, metadata, metadata) + +!llvm.module.flags = !{!0, !1, !2, !3, !4} +!llvm.dbg.cu = !{!5} +!llvm.ident = !{!8} + +; CHECK: [[t1_arg0]] = !DILocalVariable(name: "a" +; CHECK: [[t1_fake_ptr]] = !DILocalVariable(name: "fake_ptr" + +!0 = !{i32 2, !"SDK Version", [2 x i32] [i32 10, i32 14]} +!1 = !{i32 2, !"Dwarf Version", i32 4} +!2 = !{i32 2, !"Debug Info Version", i32 3} +!3 = !{i32 1, !"wchar_size", i32 4} +!4 = !{i32 7, !"PIC Level", i32 2} +!5 = distinct !DICompileUnit(language: DW_LANG_C99, file: !6, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !7, nameTableKind: GNU) +!6 = !DIFile(filename: "-", directory: "/") +!7 = !{} +!8 = !{!""} +!9 = distinct !DISubprogram(name: "t1", scope: !10, file: !10, line: 1, type: !11, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !5, retainedNodes: !7) +!10 = !DIFile(filename: "", directory: "/") +!11 = !DISubroutineType(types: !12) +!12 = !{null, !13} +!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!14 = !DILocalVariable(name: "a", arg: 1, scope: !9, file: !10, line: 1, type: !13) +!15 = !DILocation(line: 1, column: 13, scope: !9) +!16 = !DILocation(line: 1, column: 17, scope: !9) +!17 = distinct !DISubprogram(name: "t2", scope: !10, file: !10, line: 1, type: !11, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !5, retainedNodes: !7) +!18 = !DILocalVariable(name: "x", arg: 1, scope: !17, file: !10, line: 1, type: !13) +!19 = !DILocation(line: 1, column: 1, scope: !17) +!20 = !DILocalVariable(name: "fake_ptr", scope: !9, file: !10, line: 1, type: !13)