From 228c7780c56517a2454f8299ab08115b570d510c Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Thu, 29 Aug 2019 10:53:29 +0000 Subject: [PATCH] [DebugInfo] LiveDebugValues should always revisit backedges if it skips them The "join" method in LiveDebugValues does not attempt to join unseen predecessor blocks if their out-locations aren't yet initialized, instead the block should be re-visited later to see if any locations have changed validity. However, because the set of blocks were all being "process"'d once before "join" saw them, that logic in "join" was actually ignoring legitimate out-locations on the first pass through. This meant that some invalidated locations were not removed from the head of loops, allowing illegal locations to persist. Fix this by removing the run of "process" before the main join/process loop in ExtendRanges. Now the unseen predecessors that "join" skips truly are uninitialized, and we come back to the block at a later time to re-run "join", see the @baz function added. This also fixes another fault where stack/register transfers in the entry block (or any other before-any-loop-block) had their tranfers initially ignored, and were then never revisited. The MIR test added tests for this behaviour. XFail a test that exposes another bug; a fix for this is coming in D66895. Differential Revision: https://reviews.llvm.org/D66663 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370328 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/LiveDebugValues.cpp | 54 +++----- test/CodeGen/ARM/debug-info-blocks.ll | 3 + .../compiler-gen-bbs-livedebugvalues.mir | 1 + .../X86/live-debug-values-entry-transfer.mir | 122 ++++++++++++++++++ .../DebugInfo/X86/DW_AT_location-reference.ll | 5 +- .../X86/live-debug-values-remove-range.ll | 69 +++++++++- 6 files changed, 211 insertions(+), 43 deletions(-) create mode 100644 test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir diff --git a/lib/CodeGen/LiveDebugValues.cpp b/lib/CodeGen/LiveDebugValues.cpp index 1ddcceabe20..cd39950e392 100644 --- a/lib/CodeGen/LiveDebugValues.cpp +++ b/lib/CodeGen/LiveDebugValues.cpp @@ -385,7 +385,7 @@ private: void process(MachineInstr &MI, OpenRangesSet &OpenRanges, VarLocInMBB &OutLocs, VarLocMap &VarLocIDs, TransferMap &Transfers, DebugParamMap &DebugEntryVals, - bool transferChanges, OverlapMap &OverlapFragments, + OverlapMap &OverlapFragments, VarToFragments &SeenFragments); void accumulateFragmentMap(MachineInstr &MI, VarToFragments &SeenFragments, @@ -1015,21 +1015,15 @@ void LiveDebugValues::accumulateFragmentMap(MachineInstr &MI, /// This routine creates OpenRanges and OutLocs. void LiveDebugValues::process(MachineInstr &MI, OpenRangesSet &OpenRanges, VarLocInMBB &OutLocs, VarLocMap &VarLocIDs, - TransferMap &Transfers, DebugParamMap &DebugEntryVals, - bool transferChanges, + TransferMap &Transfers, + DebugParamMap &DebugEntryVals, OverlapMap &OverlapFragments, VarToFragments &SeenFragments) { transferDebugValue(MI, OpenRanges, VarLocIDs); transferRegisterDef(MI, OpenRanges, VarLocIDs, Transfers, DebugEntryVals); - if (transferChanges) { - transferRegisterCopy(MI, OpenRanges, VarLocIDs, Transfers); - transferSpillOrRestoreInst(MI, OpenRanges, VarLocIDs, Transfers); - } else { - // Build up a map of overlapping fragments on the first run through. - if (MI.isDebugValue()) - accumulateFragmentMap(MI, SeenFragments, OverlapFragments); - } + transferRegisterCopy(MI, OpenRanges, VarLocIDs, Transfers); + transferSpillOrRestoreInst(MI, OpenRanges, VarLocIDs, Transfers); } /// This routine joins the analysis results of all incoming edges in @MBB by @@ -1050,9 +1044,11 @@ bool LiveDebugValues::join( // can be joined. int NumVisited = 0; for (auto p : MBB.predecessors()) { - // Ignore unvisited predecessor blocks. As we are processing - // the blocks in reverse post-order any unvisited block can - // be considered to not remove any incoming values. + // Ignore backedges if we have not visited the predecessor yet. As the + // predecessor hasn't yet had locations propagated into it, most locations + // will not yet be valid, so treat them as all being uninitialized and + // potentially valid. If a location guessed to be correct here is + // invalidated later, we will remove it when we revisit this block. if (!Visited.count(p)) { LLVM_DEBUG(dbgs() << " ignoring unvisited pred MBB: " << p->getNumber() << "\n"); @@ -1215,8 +1211,6 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) { std::greater> Pending; - enum : bool { dontTransferChanges = false, transferChanges = true }; - // Besides parameter's modification, check whether a DBG_VALUE is inlined // in order to deduce whether the variable that it tracks comes from // a different function. If that is the case we can't track its entry value. @@ -1254,27 +1248,14 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) { !MI.getDebugExpression()->isFragment()) DebugEntryVals[MI.getDebugVariable()] = &MI; - // Initialize every mbb with OutLocs. - // We are not looking at any spill instructions during the initial pass - // over the BBs. The LiveDebugVariables pass has already created DBG_VALUE - // instructions for spills of registers that are known to be user variables - // within the BB in which the spill occurs. + // Initialize per-block structures and scan for fragment overlaps. for (auto &MBB : MF) { + PendingInLocs[&MBB] = VarLocSet(); + for (auto &MI : MBB) { - process(MI, OpenRanges, OutLocs, VarLocIDs, Transfers, DebugEntryVals, - dontTransferChanges, OverlapFragments, SeenFragments); - } - transferTerminator(&MBB, OpenRanges, OutLocs, VarLocIDs); - // Add any entry DBG_VALUE instructions necessitated by parameter - // clobbering. - for (auto &TR : Transfers) { - MBB.insertAfter(MachineBasicBlock::iterator(*TR.TransferInst), - TR.DebugInst); + if (MI.isDebugValue()) + accumulateFragmentMap(MI, SeenFragments, OverlapFragments); } - Transfers.clear(); - - // Initialize pending inlocs. - PendingInLocs[&MBB] = VarLocSet(); } auto hasNonArtificialLocation = [](const MachineInstr &MI) -> bool { @@ -1313,7 +1294,7 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) { Worklist.pop(); MBBJoined = join(*MBB, OutLocs, InLocs, VarLocIDs, Visited, ArtificialBlocks, PendingInLocs); - Visited.insert(MBB); + MBBJoined |= Visited.insert(MBB).second; if (MBBJoined) { MBBJoined = false; Changed = true; @@ -1324,8 +1305,7 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) { OpenRanges.insertFromLocSet(PendingInLocs[MBB], VarLocIDs); for (auto &MI : *MBB) process(MI, OpenRanges, OutLocs, VarLocIDs, Transfers, - DebugEntryVals, transferChanges, OverlapFragments, - SeenFragments); + DebugEntryVals, OverlapFragments, SeenFragments); OLChanged |= transferTerminator(MBB, OpenRanges, OutLocs, VarLocIDs); // Add any DBG_VALUE instructions necessitated by spills. diff --git a/test/CodeGen/ARM/debug-info-blocks.ll b/test/CodeGen/ARM/debug-info-blocks.ll index cc1a45f23da..4f80db48279 100644 --- a/test/CodeGen/ARM/debug-info-blocks.ll +++ b/test/CodeGen/ARM/debug-info-blocks.ll @@ -1,5 +1,8 @@ ; RUN: llc -filetype=obj -O0 < %s | llvm-dwarfdump -v - | FileCheck %s +; XFAIL: * +; PR43058 + ; debug_info content ; CHECK: DW_AT_name {{.*}} "foobar_func_block_invoke_0" ; CHECK-NOT: DW_TAG_subprogram diff --git a/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir b/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir index 31ef5b48a24..24dbb26b4ca 100644 --- a/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir +++ b/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir @@ -89,6 +89,7 @@ body: | ; CHECK: DBG_VALUE renamable $w0, $noreg, !9, !DIExpression(), debug-location !12 ; CHECK-NEXT: STRWui killed $w0, $sp, 3 :: (store 4 into %stack.0) ; CHECK-NEXT: DBG_VALUE $sp, 0, !9, !DIExpression(DW_OP_plus_uconst, 12) + ; CHECK-NEXT: DBG_VALUE $sp, 0, !9, !DIExpression(DW_OP_plus_uconst, 12) bb.1.artificial-bb-1: ; CHECK-LABEL: bb.1.artificial-bb-1: diff --git a/test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir b/test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir new file mode 100644 index 00000000000..2d83940ceec --- /dev/null +++ b/test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir @@ -0,0 +1,122 @@ +# RUN: llc %s -o - -run-pass=livedebugvalues -mtriple=x86_64-unknown-unknown | FileCheck %s +# +# In this lightly modified test case, the transfer in the entry block from +# geti32's return value in $eax to the non-volatile $ebx should be recognized, +# and propagated throughout the whole function. +# +# CHECK-LABEL: bb.0.entry +# CHECK: DBG_VALUE $eax +# CHECK: DBG_VALUE $ebx +# CHECK-LABEL: bb.1.loop2 +# CHECK: DBG_VALUE $ebx +# CHECK-LABEL: bb.2.loop +# CHECK: DBG_VALUE $ebx +# CHECK-LABEL: bb.3.exit +# CHECK: DBG_VALUE $ebx +--- | + ; ModuleID = 'asdf' + source_filename = "asdf.ll" + target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" + target triple = "x86_64-unknown-unknown" + + @glob = global i32 0 + + declare i1 @booler() + + declare i32 @geti32() + + declare void @escape(i32) + + ; Function Attrs: nounwind readnone speculatable willreturn + declare void @llvm.dbg.value(metadata, metadata, metadata) + + define i32 @foo() !dbg !4 { + entry: + %bar = call i32 @geti32(), !dbg !10 + call void @llvm.dbg.value(metadata i32 %bar, metadata !9, metadata !DIExpression()), !dbg !10 + br label %loop + + loop: ; preds = %loop2, %entry + call void @escape(i32 %bar) + %retval = call i1 @booler(), !dbg !10 + br i1 %retval, label %loop2, label %exit + + loop2: ; preds = %loop + store i32 %bar, i32* @glob + br label %loop + + exit: ; preds = %loop + ret i32 %bar + } + + ; Function Attrs: nounwind + declare void @llvm.stackprotector(i8*, i8**) + + !llvm.module.flags = !{!0, !1} + !llvm.dbg.cu = !{!2} + + !0 = !{i32 2, !"Debug Info Version", i32 3} + !1 = !{i32 2, !"Dwarf Version", i32 4} + !2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "beards", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug) + !3 = !DIFile(filename: "bees.cpp", directory: ".") + !4 = distinct !DISubprogram(name: "nope", scope: !3, file: !3, line: 1, type: !5, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !8) + !5 = !DISubroutineType(types: !6) + !6 = !{!7} + !7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) + !8 = !{!9} + !9 = !DILocalVariable(name: "toast", scope: !4, file: !3, line: 1, type: !7) + !10 = !DILocation(line: 1, scope: !4) + +... +--- +name: foo +tracksRegLiveness: true +frameInfo: + stackSize: 8 + offsetAdjustment: -8 + adjustsStack: true + hasCalls: true + cvBytesOfCalleeSavedRegisters: 8 +fixedStack: + - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, stack-id: default, + callee-saved-register: '$rbx', callee-saved-restored: true, debug-info-variable: '', + debug-info-expression: '', debug-info-location: '' } +stack: [] +body: | + bb.0.entry: + successors: %bb.1(0x80000000) + liveins: $rbx + + frame-setup PUSH64r killed $rbx, implicit-def $rsp, implicit $rsp, debug-location !10 + CFI_INSTRUCTION def_cfa_offset 16 + CFI_INSTRUCTION offset $rbx, -16 + CALL64pcrel32 @geti32, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $eax, debug-location !10 + DBG_VALUE $eax, $noreg, !9, !DIExpression(), debug-location !10 + $ebx = MOV32rr killed $eax, debug-location !10 + JMP_1 %bb.1 + + bb.1.loop2: + successors: %bb.2 + liveins: $ebx + + MOV32mr $rip, 1, $noreg, @glob, $noreg, renamable $ebx :: (store 4 into @glob) + + bb.2.loop: + successors: %bb.1, %bb.3 + liveins: $ebx + + $edi = MOV32rr $ebx + CALL64pcrel32 @escape, csr_64, implicit $rsp, implicit $ssp, implicit killed $edi, implicit-def $rsp, implicit-def $ssp + CALL64pcrel32 @booler, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $al, debug-location !10 + TEST8ri killed renamable $al, 1, implicit-def $eflags + JCC_1 %bb.1, 5, implicit $eflags + + bb.3.exit: + liveins: $ebx + + $eax = MOV32rr killed $ebx + $rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp + CFI_INSTRUCTION def_cfa_offset 8 + RETQ $eax + +... diff --git a/test/DebugInfo/X86/DW_AT_location-reference.ll b/test/DebugInfo/X86/DW_AT_location-reference.ll index 3ea6aea002b..dbcae5d3a0b 100644 --- a/test/DebugInfo/X86/DW_AT_location-reference.ll +++ b/test/DebugInfo/X86/DW_AT_location-reference.ll @@ -32,10 +32,7 @@ ; CHECK: .debug_info contents: ; CHECK: DW_TAG_variable ; CHECK-NEXT: DW_AT_location [DW_FORM_sec_offset] (0x00000000 -; Check that the location contains only 4 ranges - this verifies that the 4th -; and 5th ranges were successfully merged into a single range. -; CHECK-NEXT: [0x{{[0-9a-f]*}}, 0x{{[0-9a-f]*}}): -; CHECK-NEXT: [0x{{[0-9a-f]*}}, 0x{{[0-9a-f]*}}): +; Check that the location contains only 2 ranges. ; CHECK-NEXT: [0x{{[0-9a-f]*}}, 0x{{[0-9a-f]*}}): ; CHECK-NEXT: [0x{{[0-9a-f]*}}, 0x{{[0-9a-f]*}}): {{.*}}) ; CHECK-NEXT: DW_AT_name {{.*}} "x" diff --git a/test/DebugInfo/X86/live-debug-values-remove-range.ll b/test/DebugInfo/X86/live-debug-values-remove-range.ll index 3d5219e74c1..6fec3f8150e 100644 --- a/test/DebugInfo/X86/live-debug-values-remove-range.ll +++ b/test/DebugInfo/X86/live-debug-values-remove-range.ll @@ -5,12 +5,43 @@ ; know the location of "toast" at the start of the %loop block. Test that no ; location is given until after the call to @booler. ; -; CHECK: ![[VARNUM:[0-9]+]] = !DILocalVariable(name: "toast" +; Second function @baz added with an even tighter loop -- this tests different +; code-paths through LiveDebugValues. Any blocks with an incoming backedge need +; reconsideration after the parent of the backedge has had its OutLocs +; initialized, even if OutLocs hasn't changed. ; +; Third function @quux tests that we don't delete too many variable locations. +; A variable that is live across the body of the loop should maintain its +; location across that loop, and not be invalidated. +; +; CHECK: ![[FOOVARNUM:[0-9]+]] = !DILocalVariable(name: "toast" +; CHECK: ![[BAZVARNUM:[0-9]+]] = !DILocalVariable(name: "crumpets" +; CHECK: ![[QUUXVARNUM:[0-9]+]] = !DILocalVariable(name: "teacake" +; +; foo tests +; CHECK-LABEL: bb.1.loop +; CHECK-NOT: DBG_VALUE +; CHECK-LABEL: CALL64pcrel32 @booler +; CHECK: DBG_VALUE 0, $noreg, ![[FOOVARNUM]] +; +; baz tests +; CHECK-LABEL: name: baz ; CHECK-LABEL: bb.1.loop ; CHECK-NOT: DBG_VALUE ; CHECK-LABEL: CALL64pcrel32 @booler -; CHECK: DBG_VALUE 0, $noreg, ![[VARNUM]] +; CHECK: DBG_VALUE 0, $noreg, ![[BAZVARNUM]] +; +; quux tests -- the variable arrives in $edi, should get a non-undef location +; before the loop, and its position re-stated in each block. +; CHECK-LABEL: name: quux +; CHECK: DBG_VALUE $edi, $noreg, ![[QUUXVARNUM]] +; CHECK: DBG_VALUE [[QUUXLOC:[a-zA-Z0-9$_]+]], $noreg, ![[QUUXVARNUM]] +; CHECK-LABEL: bb.1.loop +; CHECK: DBG_VALUE [[QUUXLOC]], $noreg, ![[QUUXVARNUM]] +; CHECK-NOT: DBG_VALUE $noreg +; CHECK-LABEL: bb.2.exit +; CHECK: DBG_VALUE [[QUUXLOC]], $noreg, ![[QUUXVARNUM]] +; CHECK-NOT: DBG_VALUE $noreg declare i1 @booler() declare void @escape(i32) @@ -33,6 +64,32 @@ exit: ret i32 %bar } +define i32 @baz(i32 %bar) !dbg !104 { +entry: + call void @llvm.dbg.value(metadata i32 %bar, metadata !103, metadata !DIExpression()), !dbg !106 + br label %loop +loop: + call void @escape(i32 %bar) + %retval = call i1 @booler(), !dbg !106 + call void @llvm.dbg.value(metadata i32 0, metadata !103, metadata !DIExpression()), !dbg !106 + br i1 %retval, label %loop, label %exit +exit: + ret i32 %bar +} + +define i32 @quux(i32 %bar) !dbg !204 { +entry: + ; %bar will be placed in a nonvolatile or spill location for the loop, + ; before being returned later. + call void @llvm.dbg.value(metadata i32 %bar, metadata !203, metadata !DIExpression()), !dbg !206 + br label %loop +loop: + %retval = call i1 @booler(), !dbg !206 + br i1 %retval, label %loop, label %exit +exit: + ret i32 %bar +} + !llvm.module.flags = !{!0, !100} !llvm.dbg.cu = !{!1} @@ -47,3 +104,11 @@ exit: !14 = !DISubroutineType(types: !15) !15 = !{!16} !16 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) +!103 = !DILocalVariable(name: "crumpets", scope: !104, file: !2, line: 1, type: !16) +!104 = distinct !DISubprogram(name: "ribbit", scope: !2, file: !2, line: 1, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !113, type: !14, isDefinition: true) +!106 = !DILocation(line: 1, scope: !104) +!113 = !{!103} +!203 = !DILocalVariable(name: "teacake", scope: !204, file: !2, line: 1, type: !16) +!204 = distinct !DISubprogram(name: "toad", scope: !2, file: !2, line: 1, spFlags: DISPFlagDefinition, unit: !1, retainedNodes: !113, type: !14, isDefinition: true) +!206 = !DILocation(line: 1, scope: !204) +!213 = !{!203} -- 2.40.0