[llvm] r370328 - [DebugInfo] LiveDebugValues should always revisit backedges if it skips them
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 29 03:53:30 PDT 2019
Author: jmorse
Date: Thu Aug 29 03:53:29 2019
New Revision: 370328
URL: http://llvm.org/viewvc/llvm-project?rev=370328&view=rev
Log:
[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
Added:
llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir
Modified:
llvm/trunk/lib/CodeGen/LiveDebugValues.cpp
llvm/trunk/test/CodeGen/ARM/debug-info-blocks.ll
llvm/trunk/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir
llvm/trunk/test/DebugInfo/X86/DW_AT_location-reference.ll
llvm/trunk/test/DebugInfo/X86/live-debug-values-remove-range.ll
Modified: llvm/trunk/lib/CodeGen/LiveDebugValues.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveDebugValues.cpp?rev=370328&r1=370327&r2=370328&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/LiveDebugValues.cpp (original)
+++ llvm/trunk/lib/CodeGen/LiveDebugValues.cpp Thu Aug 29 03:53:29 2019
@@ -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::accumulateFragment
/// 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(Machi
std::greater<unsigned int>>
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(Machi
!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(Machi
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(Machi
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.
Modified: llvm/trunk/test/CodeGen/ARM/debug-info-blocks.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/debug-info-blocks.ll?rev=370328&r1=370327&r2=370328&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/debug-info-blocks.ll (original)
+++ llvm/trunk/test/CodeGen/ARM/debug-info-blocks.ll Thu Aug 29 03:53:29 2019
@@ -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
Modified: llvm/trunk/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir?rev=370328&r1=370327&r2=370328&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir (original)
+++ llvm/trunk/test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir Thu Aug 29 03:53:29 2019
@@ -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:
Added: llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir?rev=370328&view=auto
==============================================================================
--- llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir (added)
+++ llvm/trunk/test/DebugInfo/MIR/X86/live-debug-values-entry-transfer.mir Thu Aug 29 03:53:29 2019
@@ -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
+
+...
Modified: llvm/trunk/test/DebugInfo/X86/DW_AT_location-reference.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/DW_AT_location-reference.ll?rev=370328&r1=370327&r2=370328&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/X86/DW_AT_location-reference.ll (original)
+++ llvm/trunk/test/DebugInfo/X86/DW_AT_location-reference.ll Thu Aug 29 03:53:29 2019
@@ -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"
Modified: llvm/trunk/test/DebugInfo/X86/live-debug-values-remove-range.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/live-debug-values-remove-range.ll?rev=370328&r1=370327&r2=370328&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/X86/live-debug-values-remove-range.ll (original)
+++ llvm/trunk/test/DebugInfo/X86/live-debug-values-remove-range.ll Thu Aug 29 03:53:29 2019
@@ -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}
More information about the llvm-commits
mailing list