[llvm] [LiveDebugVariables] Fix a DBG_VALUE reordering issue (PR #111124)
David Stenberg via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 15 02:19:34 PDT 2024
https://github.com/dstenb updated https://github.com/llvm/llvm-project/pull/111124
>From 8c96e21d4af78e4e54b14ec809034e0f90bbd01e Mon Sep 17 00:00:00 2001
From: David Stenberg <david.stenberg at ericsson.com>
Date: Fri, 20 Sep 2024 12:37:39 +0200
Subject: [PATCH 1/2] Add pre-commit test for LiveDebugVariables reorder issue
---
.../Mips/livedebugvariables-reorder.mir | 84 +++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100644 llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir
diff --git a/llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir b/llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir
new file mode 100644
index 00000000000000..6b35af4a738c64
--- /dev/null
+++ b/llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir
@@ -0,0 +1,84 @@
+# RUN: llc -emit-call-site-info %s -mtriple=mips -start-before=register-coalescer -stop-after=virtregrewriter -o - | FileCheck %s
+
+# FIXME: LiveDebugVariables should not reorder the stack location DBG_VALUE and
+# the fragmented DBG_VALUEs for aaa, as the latter may represent a stale value.
+# It should also not reorder the DBG_VALUEs for the different variables, as
+# that results in noise in pass before/after diffs.
+
+# CHECK-DAG: ![[aaa:[0-9]+]] = !DILocalVariable(name: "aaa"
+# CHECK-DAG: ![[bbb:[0-9]+]] = !DILocalVariable(name: "bbb"
+# CHECK-DAG: ![[ccc:[0-9]+]] = !DILocalVariable(name: "ccc"
+# CHECK-DAG: ![[ddd:[0-9]+]] = !DILocalVariable(name: "ddd"
+
+# CHECK: DBG_VALUE %stack.0, 0, ![[aaa]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE 444, $noreg, ![[ddd]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE 333, $noreg, ![[ccc]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE 222, $noreg, ![[bbb]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE 1002, $noreg, ![[aaa]], !DIExpression(DW_OP_LLVM_fragment, 16, 16)
+# CHECK-NEXT: DBG_VALUE 1001, $noreg, ![[aaa]], !DIExpression(DW_OP_LLVM_fragment, 0, 16)
+
+--- |
+ target datalayout = "E-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
+ target triple = "mips"
+
+ define dso_local i32 @main() local_unnamed_addr #0 !dbg !3 {
+ entry:
+ ret i32 0, !dbg !12
+ }
+
+ declare !dbg !14 dso_local i32 @foo() local_unnamed_addr
+
+ declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+ attributes #0 = { "frame-pointer"="all" }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!2}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 19.0.0git", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+ !1 = !DIFile(filename: "main.c", directory: "/", checksumkind: CSK_MD5, checksum: "9edfcd32ce51b21ab508a4a0755aa78b")
+ !2 = !{i32 2, !"Debug Info Version", i32 3}
+ !3 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 12, type: !4, scopeLine: 12, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !7)
+ !4 = !DISubroutineType(types: !5)
+ !5 = !{!6}
+ !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+ !7 = !{!8, !9, !10, !11}
+ !8 = !DILocalVariable(name: "aaa", scope: !3, file: !1, line: 13, type: !6)
+ !9 = !DILocalVariable(name: "bbb", scope: !3, file: !1, line: 13, type: !6)
+ !10 = !DILocalVariable(name: "ccc", scope: !3, file: !1, line: 13, type: !6)
+ !11 = !DILocalVariable(name: "ddd", scope: !3, file: !1, line: 13, type: !6)
+ !12 = !DILocation(line: 13, scope: !3)
+ !13 = !{i64 2147496201}
+ !14 = !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !4, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+
+...
+---
+name: main
+alignment: 4
+tracksRegLiveness: true
+registers:
+ - { id: 0, class: gpr32 }
+frameInfo:
+ maxAlignment: 1
+ adjustsStack: true
+ hasCalls: true
+callSites:
+ - { bb: 0, offset: 1 }
+machineFunctionInfo: {}
+body: |
+ bb.0.entry:
+ ADJCALLSTACKDOWN 16, 0, implicit-def dead $sp, implicit $sp, debug-location !12
+ JAL @foo, csr_o32_fpxx, implicit-def dead $ra, implicit-def $sp, implicit-def $v0, debug-location !12
+ ADJCALLSTACKUP 16, 0, implicit-def dead $sp, implicit $sp, debug-location !12
+ %0:gpr32 = COPY killed $v0, debug-location !12
+ DBG_VALUE 1001, $noreg, !8, !DIExpression(DW_OP_LLVM_fragment, 0, 16), debug-location !12
+ DBG_VALUE 1002, $noreg, !8, !DIExpression(DW_OP_LLVM_fragment, 16, 16), debug-location !12
+ DBG_VALUE 222, $noreg, !9, !DIExpression(), debug-location !12
+ DBG_VALUE 333, $noreg, !10, !DIExpression(), debug-location !12
+ DBG_VALUE 444, $noreg, !11, !DIExpression(), debug-location !12
+ DBG_VALUE %0, $noreg, !8, !DIExpression(), debug-location !12
+ INLINEASM &"", 1 /* sideeffect attdialect */, 12 /* clobber */, implicit-def dead early-clobber $v0, 12 /* clobber */, implicit-def dead early-clobber $v1, 12 /* clobber */, implicit-def dead early-clobber $a0, 12 /* clobber */, implicit-def dead early-clobber $a1, 12 /* clobber */, implicit-def dead early-clobber $a2, 12 /* clobber */, implicit-def dead early-clobber $a3, 12 /* clobber */, implicit-def dead early-clobber $t0, 12 /* clobber */, implicit-def dead early-clobber $t1, 12 /* clobber */, implicit-def dead early-clobber $t2, 12 /* clobber */, implicit-def dead early-clobber $t3, 12 /* clobber */, implicit-def dead early-clobber $t4, 12 /* clobber */, implicit-def dead early-clobber $t5, 12 /* clobber */, implicit-def dead early-clobber $t6, 12 /* clobber */, implicit-def dead early-clobber $t7, 12 /* clobber */, implicit-def dead early-clobber $t8, 12 /* clobber */, implicit-def dead early-clobber $t9, 12 /* clobber */, implicit-def dead early-clobber $gp, 12 /* clobber */, implicit-def dead early-clobber $ra, 12 /* clobber */, implicit-def dead early-clobber $s0, 12 /* clobber */, implicit-def dead early-clobber $s1, 12 /* clobber */, implicit-def dead early-clobber $s2, 12 /* clobber */, implicit-def dead early-clobber $s3, 12 /* clobber */, implicit-def dead early-clobber $s4, 12 /* clobber */, implicit-def dead early-clobber $s5, 12 /* clobber */, implicit-def dead early-clobber $s6, 12 /* clobber */, implicit-def dead early-clobber $s7, 12 /* clobber */, implicit-def dead early-clobber $at, !13, debug-location !12
+ $v0 = COPY killed %0, debug-location !12
+ RetRA implicit killed $v0, debug-location !12
+
+...
>From 5457024cf90d840ec69785945b764bfd1b7319ec Mon Sep 17 00:00:00 2001
From: David Stenberg <david.stenberg at ericsson.com>
Date: Fri, 20 Sep 2024 13:04:45 +0200
Subject: [PATCH 2/2] [LiveDebugVariables] Fix a DBG_VALUE reordering issue
(#111124)
LDV could reorder reinserted fragment and non-fragment debug values for
the same variable (compared to the input order), potentially resulting
in stale values being presented.
For example, before:
DBG_VALUE 1001, $noreg, !13, !DIExpression(DW_OP_LLVM_fragment, 0, 16)
DBG_VALUE 1002, $noreg, !13, !DIExpression(DW_OP_LLVM_fragment, 16, 16)
DBG_VALUE %0, $noreg, !13, !DIExpression()
After (without this patch):
DBG_VALUE %stack.0, 0, !13, !DIExpression()
DBG_VALUE 1002, $noreg, !13, !DIExpression(DW_OP_LLVM_fragment, 16, 16)
DBG_VALUE 1001, $noreg, !13, !DIExpression(DW_OP_LLVM_fragment, 0, 16)
It would also reorder DBG_VALUEs for different variables. Although that
does not matter for the debug information output, it resulted in some
noise in before/after pass diffs.
This should hopefully align so that instruction referencing and
DBG_VALUE emit debug instructions in the same order (see the
sdag-salvage-add.ll change).
---
llvm/lib/CodeGen/LiveDebugVariables.cpp | 5 +++--
llvm/test/CodeGen/AMDGPU/debug-value2.ll | 6 +++---
.../Mips/livedebugvars-stop-trimming-loc.mir | 2 ++
.../Mips/livedebugvariables-reorder.mir | 18 +++++++++---------
.../X86/live-debug-vars-discard-invalid.mir | 4 ++--
llvm/test/DebugInfo/X86/sdag-salvage-add.ll | 7 ++-----
6 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 822a1beb489592..2ff346d3fd0223 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -1625,8 +1625,9 @@ findInsertLocation(MachineBasicBlock *MBB, SlotIndex Idx, LiveIntervals &LIS,
}
// Don't insert anything after the first terminator, though.
- return MI->isTerminator() ? MBB->getFirstTerminator() :
- std::next(MachineBasicBlock::iterator(MI));
+ auto It = MI->isTerminator() ? MBB->getFirstTerminator()
+ : std::next(MachineBasicBlock::iterator(MI));
+ return skipDebugInstructionsForward(It, MBB->end());
}
/// Find an iterator for inserting the next DBG_VALUE instruction
diff --git a/llvm/test/CodeGen/AMDGPU/debug-value2.ll b/llvm/test/CodeGen/AMDGPU/debug-value2.ll
index 1d4c11de4076cb..b09d540dd6d7b5 100644
--- a/llvm/test/CodeGen/AMDGPU/debug-value2.ll
+++ b/llvm/test/CodeGen/AMDGPU/debug-value2.ll
@@ -12,11 +12,11 @@ define <4 x float> @Scene_transformT(i32 %subshapeIdx, <4 x float> %v, float %ti
entry:
; CHECK: v_mov_b32_e32 v[[COPIED_ARG_PIECE:[0-9]+]], v9
- ; CHECK: ;DEBUG_VALUE: Scene_transformT:gScene <- [DW_OP_constu 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment 0 32] $vgpr6
+ ; CHECK: ;DEBUG_VALUE: Scene_transformT:gSceneOffsets <- [DW_OP_constu 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment 32 32] $vgpr[[COPIED_ARG_PIECE]]
+ ; CHECK: ;DEBUG_VALUE: Scene_transformT:gSceneOffsets <- [DW_OP_constu 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment 0 32] $vgpr8
; CHECK: ;DEBUG_VALUE: Scene_transformT:gScene <- [DW_OP_constu 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment 32 32] $vgpr7
+ ; CHECK: ;DEBUG_VALUE: Scene_transformT:gScene <- [DW_OP_constu 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment 0 32] $vgpr6
call void @llvm.dbg.value(metadata ptr addrspace(1) %gScene, metadata !120, metadata !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef)), !dbg !154
- ; CHECK: ;DEBUG_VALUE: Scene_transformT:gSceneOffsets <- [DW_OP_constu 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment 0 32] $vgpr8
- ; CHECK: ;DEBUG_VALUE: Scene_transformT:gSceneOffsets <- [DW_OP_constu 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment 32 32] $vgpr[[COPIED_ARG_PIECE]]
call void @llvm.dbg.value(metadata ptr addrspace(1) %gSceneOffsets, metadata !121, metadata !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef)), !dbg !155
%call = tail call ptr addrspace(1) @Scene_getSubShapeData(i32 %subshapeIdx, ptr addrspace(1) %gScene, ptr addrspace(1) %gSceneOffsets)
%m_linearMotion = getelementptr inbounds %struct.ShapeData, ptr addrspace(1) %call, i64 0, i32 2
diff --git a/llvm/test/DebugInfo/MIR/Mips/livedebugvars-stop-trimming-loc.mir b/llvm/test/DebugInfo/MIR/Mips/livedebugvars-stop-trimming-loc.mir
index 5df70096e9305d..d6171ec30b1271 100644
--- a/llvm/test/DebugInfo/MIR/Mips/livedebugvars-stop-trimming-loc.mir
+++ b/llvm/test/DebugInfo/MIR/Mips/livedebugvars-stop-trimming-loc.mir
@@ -3,11 +3,13 @@
## This tests that LiveDebugVariables does not trim non-inlined variable
## location.
+# CHECK: ![[VARA:.*]] = !DILocalVariable(name: "a"
# CHECK: ![[VARB:.*]] = !DILocalVariable(name: "b"
# CHECK: ![[VARC:.*]] = !DILocalVariable(name: "c"
# CHECK: $at = COPY $a2
# CHECK-NEXT: DBG_VALUE $at, $noreg, ![[VARC]], !DIExpression(), debug-location
# CHECK: $s0 = COPY $a1
+# CHECK-NEXT: DBG_VALUE $a0, $noreg, ![[VARA]], !DIExpression(), debug-location
# CHECK-NEXT: DBG_VALUE $s0, $noreg, ![[VARB]], !DIExpression(), debug-location
--- |
diff --git a/llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir b/llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir
index 6b35af4a738c64..bc24c5272d3187 100644
--- a/llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir
+++ b/llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir
@@ -1,21 +1,21 @@
# RUN: llc -emit-call-site-info %s -mtriple=mips -start-before=register-coalescer -stop-after=virtregrewriter -o - | FileCheck %s
-# FIXME: LiveDebugVariables should not reorder the stack location DBG_VALUE and
-# the fragmented DBG_VALUEs for aaa, as the latter may represent a stale value.
-# It should also not reorder the DBG_VALUEs for the different variables, as
-# that results in noise in pass before/after diffs.
+# Verify that LiveDebugVariables does not reorder the stack location DBG_VALUE
+# and the fragmented DBG_VALUEs for aaa, as the latter may represent a stale
+# value. It should also not reorder the DBG_VALUEs for the different variables,
+# as that results in noise in pass before/after diffs.
# CHECK-DAG: ![[aaa:[0-9]+]] = !DILocalVariable(name: "aaa"
# CHECK-DAG: ![[bbb:[0-9]+]] = !DILocalVariable(name: "bbb"
# CHECK-DAG: ![[ccc:[0-9]+]] = !DILocalVariable(name: "ccc"
# CHECK-DAG: ![[ddd:[0-9]+]] = !DILocalVariable(name: "ddd"
-# CHECK: DBG_VALUE %stack.0, 0, ![[aaa]], !DIExpression()
-# CHECK-NEXT: DBG_VALUE 444, $noreg, ![[ddd]], !DIExpression()
-# CHECK-NEXT: DBG_VALUE 333, $noreg, ![[ccc]], !DIExpression()
-# CHECK-NEXT: DBG_VALUE 222, $noreg, ![[bbb]], !DIExpression()
+# CHECK: DBG_VALUE 1001, $noreg, ![[aaa]], !DIExpression(DW_OP_LLVM_fragment, 0, 16)
# CHECK-NEXT: DBG_VALUE 1002, $noreg, ![[aaa]], !DIExpression(DW_OP_LLVM_fragment, 16, 16)
-# CHECK-NEXT: DBG_VALUE 1001, $noreg, ![[aaa]], !DIExpression(DW_OP_LLVM_fragment, 0, 16)
+# CHECK-NEXT: DBG_VALUE 222, $noreg, ![[bbb]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE 333, $noreg, ![[ccc]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE 444, $noreg, ![[ddd]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE %stack.0, 0, ![[aaa]], !DIExpression()
--- |
target datalayout = "E-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
diff --git a/llvm/test/DebugInfo/X86/live-debug-vars-discard-invalid.mir b/llvm/test/DebugInfo/X86/live-debug-vars-discard-invalid.mir
index 08130b47b4c5c9..7c8aa966d3d643 100644
--- a/llvm/test/DebugInfo/X86/live-debug-vars-discard-invalid.mir
+++ b/llvm/test/DebugInfo/X86/live-debug-vars-discard-invalid.mir
@@ -118,15 +118,15 @@ body: |
# CHECK-LABEL: bb.3:
# CHECK: dead renamable $rcx = IMPLICIT_DEF
-# CHECK-NEXT: DBG_VALUE 0, $noreg, !23, !DIExpression()
# CHECK-NEXT: DBG_VALUE $rcx, $noreg, !18, !DIExpression()
+# CHECK-NEXT: DBG_VALUE 0, $noreg, !23, !DIExpression()
# CHECK-LABEL: bb.4:
# CHECK: liveins: $rax
# CHECK: DBG_VALUE $rax, $noreg, !18, !DIExpression()
# CHECK-NEXT: renamable $rax = BTS64ri8 killed renamable $rax, 0, implicit-def $eflags
-# CHECK-NEXT: DBG_VALUE 0, $noreg, !23, !DIExpression()
# CHECK-NEXT: DBG_VALUE $rax, $noreg, !18, !DIExpression()
+# CHECK-NEXT: DBG_VALUE 0, $noreg, !23, !DIExpression()
# CHECK-NEXT: renamable $rax = BTS64ri8 killed renamable $rax, 0, implicit-def $eflags
# CHECK-NEXT: DBG_VALUE $rax, $noreg, !18, !DIExpression()
# CHECK-NEXT: dead renamable $rax = BTS64ri8 killed renamable $rax, 0, implicit-def $eflags
diff --git a/llvm/test/DebugInfo/X86/sdag-salvage-add.ll b/llvm/test/DebugInfo/X86/sdag-salvage-add.ll
index 5a77b9504206cb..bd3ff01f9a8266 100644
--- a/llvm/test/DebugInfo/X86/sdag-salvage-add.ll
+++ b/llvm/test/DebugInfo/X86/sdag-salvage-add.ll
@@ -27,21 +27,18 @@
; instruction selection as it is folded into the load. As a result, we should
; refer to s4 and myVar with complex expressions.
;
-; NB: instruction referencing and DBG_VALUE modes produce debug insts in a
-; different order.
-;
; CHECK: ![[S4:.*]] = !DILocalVariable(name: "s4",
; CHECK: ![[MYVAR:.*]] = !DILocalVariable(name: "myVar",
; CHECK: $rax = MOV64rm
; INSTRREF-SAME: debug-instr-number 2,
; INSTRREF-NEXT: DBG_INSTR_REF ![[S4]],
-; DBGVALUE-NEXT: DBG_VALUE $rax, $noreg, ![[MYVAR]],
+; DBGVALUE-NEXT: DBG_VALUE $rax, $noreg, ![[S4]],
; DBGVALUE-SAME: !DIExpression(DW_OP_plus_uconst, 4096, DW_OP_stack_value)
; INSTRREF-SAME: !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_plus_uconst, 4096, DW_OP_stack_value)
; INSTRREF-SAME: dbg-instr-ref(2, 0)
; INSTRREF: DBG_INSTR_REF ![[MYVAR]],
-; DBGVALUE: DBG_VALUE $rax, $noreg, ![[S4]],
+; DBGVALUE: DBG_VALUE $rax, $noreg, ![[MYVAR]],
; DBGVALUE-SAME: !DIExpression(DW_OP_plus_uconst, 4096, DW_OP_stack_value)
; INSTRREF-SAME: !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_plus_uconst, 4096, DW_OP_stack_value)
; INSTRREF-SAME: dbg-instr-ref(2, 0)
More information about the llvm-commits
mailing list