[llvm] 9786198 - [LiveDebugVariables] Fix a DBG_VALUE reordering issue (#111124)

David Stenberg via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 15 02:36:46 PDT 2024


Author: David Stenberg
Date: 2024-10-15T11:36:24+02:00
New Revision: 97861981cccce546b37f56e3e99e68d37c7586c7

URL: https://github.com/llvm/llvm-project/commit/97861981cccce546b37f56e3e99e68d37c7586c7
DIFF: https://github.com/llvm/llvm-project/commit/97861981cccce546b37f56e3e99e68d37c7586c7.diff

LOG: [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).

Added: 
    

Modified: 
    llvm/lib/CodeGen/LiveDebugVariables.cpp
    llvm/test/CodeGen/AMDGPU/debug-value2.ll
    llvm/test/DebugInfo/MIR/Mips/livedebugvars-stop-trimming-loc.mir
    llvm/test/DebugInfo/Mips/livedebugvariables-reorder.mir
    llvm/test/DebugInfo/X86/live-debug-vars-discard-invalid.mir
    llvm/test/DebugInfo/X86/sdag-salvage-add.ll

Removed: 
    


################################################################################
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 
diff erent variables, as
-# that results in noise in pass before/after 
diff 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 
diff erent variables,
+# as that results in noise in pass before/after 
diff s.
 
 # 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
-; 
diff erent 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