From 4b8314b0f01488be055c7c94ff295147cee1716b Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson Date: Thu, 28 Sep 2017 13:10:06 +0000 Subject: [PATCH] [DebugInfo] Do not extend range for physreg in LiveDebugVariables Summary: A DBG_VALUE that is referring to a physical register is valid up until the next def of the register, or the end of the basic block that it belongs to. LiveDebugVariables is computing live intervals (slot index ranges) for DBG_VALUE instructions, before regalloc, in order to be able to re-insert DBG_VALUE instructions again after regalloc. When the DBG_VALUE is mapping a variable to a physical register we do not need to compute the range. We should simply re-insert the DBG_VALUE at the start position. The problem that was found, resulting in this patch, was a situation when the DBG_VALUE was the last real use of the physical register. The computeIntervals/extendDef methods extended the range to cover the whole basic block, even though the physical register very well could be allocated to some virtual register inside the basic block. So the extended range could not be trusted. This patch is a preparation for https://reviews.llvm.org/D38229, where the goal is to insert DBG_VALUE after each new definition of a variable, even if the virtual registers that the variable was connected to has been coalesced into using the same physical register (e.g. due to two address instructions). For more info see https://bugs.llvm.org/show_bug.cgi?id=34545 Reviewers: aprantl, rnk, echristo Reviewed By: aprantl Subscribers: Ka-Ka, llvm-commits Differential Revision: https://reviews.llvm.org/D38140 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@314414 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/LiveDebugVariables.cpp | 12 +- .../live-debug-vars-unused-arg-debugonly.mir | 163 ++++++++++++++++++ .../MIR/X86/live-debug-vars-unused-arg.mir | 158 +++++++++++++++++ 3 files changed, 327 insertions(+), 6 deletions(-) create mode 100644 test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir create mode 100644 test/DebugInfo/MIR/X86/live-debug-vars-unused-arg.mir diff --git a/lib/CodeGen/LiveDebugVariables.cpp b/lib/CodeGen/LiveDebugVariables.cpp index 9a82e3c42dd..d7345b0446a 100644 --- a/lib/CodeGen/LiveDebugVariables.cpp +++ b/lib/CodeGen/LiveDebugVariables.cpp @@ -662,12 +662,12 @@ void UserValue::computeIntervals(MachineRegisterInfo &MRI, continue; } - // For physregs, use the live range of the first regunit as a guide. - unsigned Unit = *MCRegUnitIterator(Loc.getReg(), &TRI); - LiveRange *LR = &LIS.getRegUnit(Unit); - const VNInfo *VNI = LR->getVNInfoAt(Idx); - // Don't track copies from physregs, it is too expensive. - extendDef(Idx, LocNo, LR, VNI, nullptr, LIS); + // For physregs, we only mark the start slot idx. DwarfDebug will see it + // as if the DBG_VALUE is valid up until the end of the basic block, or + // the next def of the physical register. So we do not need to extend the + // range. It might actually happen that the DBG_VALUE is the last use of + // the physical register (e.g. if this is an unused input argument to a + // function). } // Erase all the undefs. diff --git a/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir b/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir new file mode 100644 index 00000000000..1389ad44317 --- /dev/null +++ b/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir @@ -0,0 +1,163 @@ +# RUN: llc -O1 -start-before=greedy -stop-after=virtregrewriter -o /dev/null %s -debug-only=livedebugvars 2>&1 | FileCheck -check-prefix=CHECKDBG %s + +# REQUIRES: asserts + +# This test case was generated by using the following c program: +# extern void foo(int, int); +# +# int bar[2] = {1, 2}; +# +# int main(int argc, char** argv) +# { +# int a0 = bar[0]; +# int a1 = bar[1]; +# foo(a0, a1); +# return 0; +# } +# +# It was compiled with -g and -O1, and the mir was dumped before ra greedy. + +--- | + ; ModuleID = 'live-debug-vars-unused-arg.ll' + source_filename = "live-debug-vars-unused-arg.c" + target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" + target triple = "x86_64-unknown-linux-gnu" + + @bar = local_unnamed_addr global [2 x i32] [i32 1, i32 2], align 4, !dbg !0 + + ; Function Attrs: nounwind uwtable + define i32 @main(i32 %argc, i8** nocapture readnone %argv) local_unnamed_addr #0 !dbg !14 { + entry: + tail call void @llvm.dbg.value(metadata i32 %argc, metadata !21, metadata !DIExpression()), !dbg !25 + tail call void @llvm.dbg.value(metadata i8** %argv, metadata !22, metadata !DIExpression()), !dbg !26 + %0 = load i32, i32* getelementptr inbounds ([2 x i32], [2 x i32]* @bar, i64 0, i64 0), align 4, !dbg !27, !tbaa !28 + tail call void @llvm.dbg.value(metadata i32 %0, metadata !23, metadata !DIExpression()), !dbg !32 + %1 = load i32, i32* getelementptr inbounds ([2 x i32], [2 x i32]* @bar, i64 0, i64 1), align 4, !dbg !33, !tbaa !28 + tail call void @llvm.dbg.value(metadata i32 %1, metadata !24, metadata !DIExpression()), !dbg !34 + tail call void @foo(i32 %0, i32 %1) #2, !dbg !35 + ret i32 0, !dbg !36 + } + + declare void @foo(i32, i32) local_unnamed_addr + + ; Function Attrs: nounwind readnone speculatable + declare void @llvm.dbg.value(metadata, metadata, metadata) #1 + + ; Function Attrs: nounwind + declare void @llvm.stackprotector(i8*, i8**) #2 + + attributes #0 = { nounwind uwtable } + attributes #1 = { nounwind readnone speculatable } + attributes #2 = { nounwind } + + !llvm.dbg.cu = !{!2} + !llvm.module.flags = !{!10, !11, !12} + !llvm.ident = !{!13} + + !0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression()) + !1 = distinct !DIGlobalVariable(name: "bar", scope: !2, file: !3, line: 3, type: !6, isLocal: false, isDefinition: true) + !2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 6.0.0 (trunk 313866) (llvm/trunk 313875)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5) + !3 = !DIFile(filename: "live-debug-vars-unused-arg.c", directory: "/repo/uabbpet/master") + !4 = !{} + !5 = !{!0} + !6 = !DICompositeType(tag: DW_TAG_array_type, baseType: !7, size: 64, elements: !8) + !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) + !8 = !{!9} + !9 = !DISubrange(count: 2) + !10 = !{i32 2, !"Dwarf Version", i32 4} + !11 = !{i32 2, !"Debug Info Version", i32 3} + !12 = !{i32 1, !"wchar_size", i32 4} + !13 = !{!"clang version 6.0.0 (trunk 313866) (llvm/trunk 313875)"} + !14 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 5, type: !15, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: true, unit: !2, variables: !20) + !15 = !DISubroutineType(types: !16) + !16 = !{!7, !7, !17} + !17 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !18, size: 64) + !18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !19, size: 64) + !19 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) + !20 = !{!21, !22, !23, !24} + !21 = !DILocalVariable(name: "argc", arg: 1, scope: !14, file: !3, line: 5, type: !7) + !22 = !DILocalVariable(name: "argv", arg: 2, scope: !14, file: !3, line: 5, type: !17) + !23 = !DILocalVariable(name: "a0", scope: !14, file: !3, line: 7, type: !7) + !24 = !DILocalVariable(name: "a1", scope: !14, file: !3, line: 8, type: !7) + !25 = !DILocation(line: 5, column: 14, scope: !14) + !26 = !DILocation(line: 5, column: 27, scope: !14) + !27 = !DILocation(line: 7, column: 12, scope: !14) + !28 = !{!29, !29, i64 0} + !29 = !{!"int", !30, i64 0} + !30 = !{!"omnipotent char", !31, i64 0} + !31 = !{!"Simple C/C++ TBAA"} + !32 = !DILocation(line: 7, column: 7, scope: !14) + !33 = !DILocation(line: 8, column: 12, scope: !14) + !34 = !DILocation(line: 8, column: 7, scope: !14) + !35 = !DILocation(line: 9, column: 3, scope: !14) + !36 = !DILocation(line: 10, column: 3, scope: !14) + +... +--- +name: main +alignment: 4 +exposesReturnsTwice: false +legalized: false +regBankSelected: false +selected: false +tracksRegLiveness: true +registers: + - { id: 0, class: gr32, preferred-register: '' } + - { id: 1, class: gr64, preferred-register: '' } + - { id: 2, class: gr32, preferred-register: '' } + - { id: 3, class: gr32, preferred-register: '' } + - { id: 4, class: gr32, preferred-register: '' } +liveins: +frameInfo: + isFrameAddressTaken: false + isReturnAddressTaken: false + hasStackMap: false + hasPatchPoint: false + stackSize: 0 + offsetAdjustment: 0 + maxAlignment: 0 + adjustsStack: false + hasCalls: true + stackProtector: '' + maxCallFrameSize: 4294967295 + hasOpaqueSPAdjustment: false + hasVAStart: false + hasMustTailInVarArgFunc: false + savePoint: '' + restorePoint: '' +fixedStack: +stack: +constants: +body: | + bb.0.entry: + DBG_VALUE debug-use %edi, debug-use _, !21, !DIExpression(), debug-location !25 + DBG_VALUE debug-use %rsi, debug-use _, !22, !DIExpression(), debug-location !26 + %2 = MOV32rm %rip, 1, _, @bar, _, debug-location !27 :: (dereferenceable load 4 from `i32* getelementptr inbounds ([2 x i32], [2 x i32]* @bar, i64 0, i64 0)`, !tbaa !28) + DBG_VALUE debug-use %2, debug-use _, !23, !DIExpression(), debug-location !32 + %3 = MOV32rm %rip, 1, _, @bar + 4, _, debug-location !33 :: (dereferenceable load 4 from `i32* getelementptr inbounds ([2 x i32], [2 x i32]* @bar, i64 0, i64 1)`, !tbaa !28) + DBG_VALUE debug-use %3, debug-use _, !24, !DIExpression(), debug-location !34 + ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !35 + %edi = COPY %2, debug-location !35 + %esi = COPY %3, debug-location !35 + CALL64pcrel32 @foo, csr_64, implicit %rsp, implicit killed %edi, implicit killed %esi, implicit-def %rsp, debug-location !35 + ADJCALLSTACKUP64 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !35 + %eax = MOV32r0 implicit-def dead %eflags, debug-location !36 + RET 0, killed %eax, debug-location !36 + +... + +# Let's verify that the slot index ranges for the unused variables argc/argv, +# connected to physical regs %EDI and %RSI, does not overlap with the ranges +# for %vreg2 and %vreg3. The register allocator is actually allocating the +# virtual registers # to %EDI and %ESI, so the ranges for argc/argv should +# not cover the whole BB. +# +# CHECKDBG-LABEL: ********** EMITTING LIVE DEBUG VARIABLES ********** +# CHECKDBG-NEXT: !"argc,5" [0B;0e):0 Loc0=%EDI +# CHECKDBG-NEXT: [0B;0e):0 BB#0-160B +# CHECKDBG-NEXT: !"argv,5" [0B;0e):0 Loc0=%RSI +# CHECKDBG-NEXT: [0B;0e):0 BB#0-160B +# CHECKDBG-NEXT: !"a0,7" [16r;64r):0 Loc0=%vreg2 +# CHECKDBG-NEXT: [16r;64r):0 BB#0-160B +# CHECKDBG-NEXT: !"a1,8" [32r;80r):0 Loc0=%vreg3 +# CHECKDBG-NEXT: [32r;80r):0 BB#0-160B diff --git a/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg.mir b/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg.mir new file mode 100644 index 00000000000..84c2194f4a3 --- /dev/null +++ b/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg.mir @@ -0,0 +1,158 @@ +# RUN: llc -O1 -start-before=greedy -stop-after=virtregrewriter -o - %s | FileCheck -check-prefix=CHECKMIR %s + +# This test case was generated by using the following c program: +# extern void foo(int, int); +# +# int bar[2] = {1, 2}; +# +# int main(int argc, char** argv) +# { +# int a0 = bar[0]; +# int a1 = bar[1]; +# foo(a0, a1); +# return 0; +# } +# +# It was compiled with -g and -O1, and the mir was dumped before ra greedy. + +--- | + ; ModuleID = 'live-debug-vars-unused-arg.ll' + source_filename = "live-debug-vars-unused-arg.c" + target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" + target triple = "x86_64-unknown-linux-gnu" + + @bar = local_unnamed_addr global [2 x i32] [i32 1, i32 2], align 4, !dbg !0 + + ; Function Attrs: nounwind uwtable + define i32 @main(i32 %argc, i8** nocapture readnone %argv) local_unnamed_addr #0 !dbg !14 { + entry: + tail call void @llvm.dbg.value(metadata i32 %argc, metadata !21, metadata !DIExpression()), !dbg !25 + tail call void @llvm.dbg.value(metadata i8** %argv, metadata !22, metadata !DIExpression()), !dbg !26 + %0 = load i32, i32* getelementptr inbounds ([2 x i32], [2 x i32]* @bar, i64 0, i64 0), align 4, !dbg !27, !tbaa !28 + tail call void @llvm.dbg.value(metadata i32 %0, metadata !23, metadata !DIExpression()), !dbg !32 + %1 = load i32, i32* getelementptr inbounds ([2 x i32], [2 x i32]* @bar, i64 0, i64 1), align 4, !dbg !33, !tbaa !28 + tail call void @llvm.dbg.value(metadata i32 %1, metadata !24, metadata !DIExpression()), !dbg !34 + tail call void @foo(i32 %0, i32 %1) #2, !dbg !35 + ret i32 0, !dbg !36 + } + + declare void @foo(i32, i32) local_unnamed_addr + + ; Function Attrs: nounwind readnone speculatable + declare void @llvm.dbg.value(metadata, metadata, metadata) #1 + + ; Function Attrs: nounwind + declare void @llvm.stackprotector(i8*, i8**) #2 + + attributes #0 = { nounwind uwtable } + attributes #1 = { nounwind readnone speculatable } + attributes #2 = { nounwind } + + !llvm.dbg.cu = !{!2} + !llvm.module.flags = !{!10, !11, !12} + !llvm.ident = !{!13} + + !0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression()) + !1 = distinct !DIGlobalVariable(name: "bar", scope: !2, file: !3, line: 3, type: !6, isLocal: false, isDefinition: true) + !2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 6.0.0 (trunk 313866) (llvm/trunk 313875)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5) + !3 = !DIFile(filename: "live-debug-vars-unused-arg.c", directory: "/repo/uabbpet/master") + !4 = !{} + !5 = !{!0} + !6 = !DICompositeType(tag: DW_TAG_array_type, baseType: !7, size: 64, elements: !8) + !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) + !8 = !{!9} + !9 = !DISubrange(count: 2) + !10 = !{i32 2, !"Dwarf Version", i32 4} + !11 = !{i32 2, !"Debug Info Version", i32 3} + !12 = !{i32 1, !"wchar_size", i32 4} + !13 = !{!"clang version 6.0.0 (trunk 313866) (llvm/trunk 313875)"} + !14 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 5, type: !15, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: true, unit: !2, variables: !20) + !15 = !DISubroutineType(types: !16) + !16 = !{!7, !7, !17} + !17 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !18, size: 64) + !18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !19, size: 64) + !19 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) + !20 = !{!21, !22, !23, !24} + !21 = !DILocalVariable(name: "argc", arg: 1, scope: !14, file: !3, line: 5, type: !7) + !22 = !DILocalVariable(name: "argv", arg: 2, scope: !14, file: !3, line: 5, type: !17) + !23 = !DILocalVariable(name: "a0", scope: !14, file: !3, line: 7, type: !7) + !24 = !DILocalVariable(name: "a1", scope: !14, file: !3, line: 8, type: !7) + !25 = !DILocation(line: 5, column: 14, scope: !14) + !26 = !DILocation(line: 5, column: 27, scope: !14) + !27 = !DILocation(line: 7, column: 12, scope: !14) + !28 = !{!29, !29, i64 0} + !29 = !{!"int", !30, i64 0} + !30 = !{!"omnipotent char", !31, i64 0} + !31 = !{!"Simple C/C++ TBAA"} + !32 = !DILocation(line: 7, column: 7, scope: !14) + !33 = !DILocation(line: 8, column: 12, scope: !14) + !34 = !DILocation(line: 8, column: 7, scope: !14) + !35 = !DILocation(line: 9, column: 3, scope: !14) + !36 = !DILocation(line: 10, column: 3, scope: !14) + +... +--- +name: main +alignment: 4 +exposesReturnsTwice: false +legalized: false +regBankSelected: false +selected: false +tracksRegLiveness: true +registers: + - { id: 0, class: gr32, preferred-register: '' } + - { id: 1, class: gr64, preferred-register: '' } + - { id: 2, class: gr32, preferred-register: '' } + - { id: 3, class: gr32, preferred-register: '' } + - { id: 4, class: gr32, preferred-register: '' } +liveins: +frameInfo: + isFrameAddressTaken: false + isReturnAddressTaken: false + hasStackMap: false + hasPatchPoint: false + stackSize: 0 + offsetAdjustment: 0 + maxAlignment: 0 + adjustsStack: false + hasCalls: true + stackProtector: '' + maxCallFrameSize: 4294967295 + hasOpaqueSPAdjustment: false + hasVAStart: false + hasMustTailInVarArgFunc: false + savePoint: '' + restorePoint: '' +fixedStack: +stack: +constants: +body: | + bb.0.entry: + DBG_VALUE debug-use %edi, debug-use _, !21, !DIExpression(), debug-location !25 + DBG_VALUE debug-use %rsi, debug-use _, !22, !DIExpression(), debug-location !26 + %2 = MOV32rm %rip, 1, _, @bar, _, debug-location !27 :: (dereferenceable load 4 from `i32* getelementptr inbounds ([2 x i32], [2 x i32]* @bar, i64 0, i64 0)`, !tbaa !28) + DBG_VALUE debug-use %2, debug-use _, !23, !DIExpression(), debug-location !32 + %3 = MOV32rm %rip, 1, _, @bar + 4, _, debug-location !33 :: (dereferenceable load 4 from `i32* getelementptr inbounds ([2 x i32], [2 x i32]* @bar, i64 0, i64 1)`, !tbaa !28) + DBG_VALUE debug-use %3, debug-use _, !24, !DIExpression(), debug-location !34 + ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !35 + %edi = COPY %2, debug-location !35 + %esi = COPY %3, debug-location !35 + CALL64pcrel32 @foo, csr_64, implicit %rsp, implicit killed %edi, implicit killed %esi, implicit-def %rsp, debug-location !35 + ADJCALLSTACKUP64 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !35 + %eax = MOV32r0 implicit-def dead %eflags, debug-location !36 + RET 0, killed %eax, debug-location !36 + +... + +# Verify that we only get one DBG_VALUE for argc and one for argv. +# +# CHECKMIR: ![[ARGC:[0-9]+]] = !DILocalVariable(name: "argc", arg: 1 +# CHECKMIR: ![[ARGV:[0-9]+]] = !DILocalVariable(name: "argv", arg: 2 +# CHECKMIR: name: main +# CHECKMIR: body: +# CHECKMIR: DBG_VALUE debug-use %edi, debug-use _, ![[ARGC]] +# CHECKMIR-NOT: DBG_VALUE debug-use %{{.*}}, debug-use _, ![[ARGC]] +# CHECKMIR: DBG_VALUE debug-use %rsi, debug-use _, ![[ARGV]] +# CHECKMIR-NOT: DBG_VALUE debug-use %{{.*}}, debug-use _, ![[ARGC]] +# CHECKMIR-NOT: DBG_VALUE debug-use %{{.*}}, debug-use _, ![[ARGV]] + -- 2.50.0