Skip to content

[LiveDebugVariables] Fix a DBG_VALUE reordering issue #111124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 15, 2024
Merged

[LiveDebugVariables] Fix a DBG_VALUE reordering issue #111124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f719886
Add pre-commit test for LiveDebugVariables reorder issue
dstenb Sep 20, 2024
9786198
[LiveDebugVariables] Fix a DBG_VALUE reordering issue (#111124)
dstenb Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions llvm/lib/CodeGen/LiveDebugVariables.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/AMDGPU/debug-value2.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment on lines 14 to 21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard for me to analyse just from the github UI as this isn't a MIR test, but !120 = gScene, !121 = gSceneOffsets, which are ordered !120, !121 in the two pre-isel dbg.values. In the old check lines the DEBUG_VALUE lines come out in the right order, but now they're flipped?

Chances are I'm reading it wrong, or there's some other re-ordering happening somewhere in LLVM that LDVars was just accidentally undoing, seeing how the order in other test files changes is fixed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case the debug values get reordered at isel due to the order of the lowered parameters. I should perhaps have added an XXX note about that in the test. I can do that.

define <4 x float> @Scene_transformT(i32 %subshapeIdx, <4 x float> %v, float %time, ptr addrspace(1) %gScene, ptr addrspace(1) %gSceneOffsets) local_unnamed_addr !dbg !110 {
entry:
    #dbg_value(ptr addrspace(1) %gScene, !120, !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef), !154)
    #dbg_value(ptr addrspace(1) %gSceneOffsets, !121, !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef), !155)

# *** IR Dump After AMDGPU DAG->DAG Pattern Instruction Selection (amdgpu-isel) ***:
# Machine code for function Scene_transformT: IsSSA, TracksLiveness
Function Live Ins: $sgpr4_sgpr5 in %55, $sgpr8_sgpr9 in %56, $sgpr10_sgpr11 in %57, $sgpr12 in %58, $sgpr13 in %59, $sgpr14 in %60, $sgpr15 in %61, $vgpr0 in %62, $vgpr1 in %63, $vgpr2 in %64, $vgpr3 in %65, $vgpr4 in %66, $vgpr5 in %67, $vgpr6 in %68, $vgpr7 in %69, $vgpr8 in %70, $vgpr9 in %71, $sgpr6_sgpr7 in %73, $vgpr31 in %74

bb.0.entry:
  successors: %bb.1(0x40000000), %bb.9(0x40000000); %bb.1(50.00%), %bb.9(50.00%)
  liveins: $sgpr4_sgpr5, $sgpr8_sgpr9, $sgpr10_sgpr11, $sgpr12, $sgpr13, $sgpr14, $sgpr15, $vgpr0, $vgpr1, $vgpr2, $vgpr3, $vgpr4, $vgpr5, $vgpr6, $vgpr7, $vgpr8, $vgpr9, $sgpr6_sgpr7, $vgpr31
  %74:vgpr_32 = COPY $vgpr31
  %73:sgpr_64 = COPY $sgpr6_sgpr7
  %71:vgpr_32 = COPY $vgpr9
  DBG_VALUE %71:vgpr_32, $noreg, !"gSceneOffsets", !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment, 32, 32), debug-location !155; GraphMaterialSystemKernels1.cl:2182:102 line no:2182
  %70:vgpr_32 = COPY $vgpr8
  DBG_VALUE %70:vgpr_32, $noreg, !"gSceneOffsets", !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment, 0, 32), debug-location !155; GraphMaterialSystemKernels1.cl:2182:102 line no:2182
  %69:vgpr_32 = COPY $vgpr7
  DBG_VALUE %69:vgpr_32, $noreg, !"gScene", !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment, 32, 32), debug-location !154; GraphMaterialSystemKernels1.cl:2182:80 line no:2182
  %68:vgpr_32 = COPY $vgpr6
  DBG_VALUE %68:vgpr_32, $noreg, !"gScene", !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef, DW_OP_LLVM_fragment, 0, 32), debug-location !154; GraphMaterialSystemKernels1.cl:2182:80 line no:2182

%m_linearMotion = getelementptr inbounds %struct.ShapeData, ptr addrspace(1) %call, i64 0, i32 2
Expand Down
2 changes: 2 additions & 0 deletions llvm/test/DebugInfo/MIR/Mips/livedebugvars-stop-trimming-loc.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

--- |
Expand Down
84 changes: 84 additions & 0 deletions llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# RUN: llc -emit-call-site-info %s -mtriple=mips -start-before=register-coalescer -stop-after=virtregrewriter -o - | FileCheck %s

# 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 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 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"
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

...
4 changes: 2 additions & 2 deletions llvm/test/DebugInfo/X86/live-debug-vars-discard-invalid.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions llvm/test/DebugInfo/X86/sdag-salvage-add.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading