[DebugInfo] Remove `dbg.addr` from CodeGen

[jryans]

Summary

As part of this work, removing SDDbgValue::clearIsEmitted originally added for
dbg.addr in 045c67769d7fe577fc38cccb6fb40fd814437447 was attempted, but it
appears some tests for DBG_INSTR_REF now depend on that behaviour as well, so
it was kept and comments were updated instead.

Part of dbg.addr removal
Discussed in https://discourse.llvm.org/t/what-is-the-status-of-dbg-addr/62898

Depends on D144799

[StephenTozer]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

6090

👍

llvm/test/DebugInfo/X86/merge-equivalent-ranges.ll

31

For the purposes of this test, we can also remove this line and the test for "Var1" above - since this test is testing that this range merges with the one that is being deleted, the test isn't meaningful anymore; ideally we'd have a good way to test this otherwise, but don't consider it a blocker on this patch.

[jryans]

llvm/test/DebugInfo/X86/merge-equivalent-ranges.ll

31

Ah hmm, what if instead of removing the dbg.addr below, I were to replace it with dbg.declare...? Would that preserve the full meaning of this test?

[StephenTozer]

llvm/test/DebugInfo/X86/merge-equivalent-ranges.ll

31

If a dbg.declare exists for a variable, there should be no other debug intrinsics for that variable; I'm not sure if that would be validated, but it *shouldn't* happen naturally, so best not to test against it imo.

[jryans]

llvm/test/DebugInfo/X86/merge-equivalent-ranges.ll

31

Ah hmm, perhaps then we could repeat the dbg.value + deref from line 31 at the removed line 40 (effectively that's what the auto-upgraded dbg.addr would turn into anyway)...?

[StephenTozer]

llvm/test/DebugInfo/X86/merge-equivalent-ranges.ll

31

Also sadly not - the difference with dbg.addr is that we have DBG_VALUE %r, $noreg, !"var1", !DIExpr(DW_OP_deref) and DBG_VALUE %r, 0, !"var1", !DIExpr(); identical meanings, but different representations. That's what's being tested for here - that at some point the compiler recognizes the debug values mean the same thing and merges them. I wouldn't worry about this test blocking the patch - the right move might be to convert this to a MIR test with the instructions being explicit, but that can be done separately.

[jryans]

Removed surrounding test checks following review comments

llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp

LLVM_DEBUG(dbgs() << "   LiveLoc " << locStr(getLocKind(LiveSet, Var))
                     << " -> Val, dbg.value override");
 
   setLocKind(LiveSet, Var, LocKind::Val);
   emitDbgValue(LocKind::Val, &DVI, &DVI);
 }
 
 void AssignmentTrackingLowering::processDbgInstruction(
     Instruction &I, AssignmentTrackingLowering::BlockInfo *LiveSet) {
-  assert(!isa<DbgAddrIntrinsic>(&I) && "unexpected dbg.addr");
   if (auto *DAI = dyn_cast<DbgAssignIntrinsic>(&I))
     processDbgAssign(*DAI, LiveSet);
   else if (auto *DVI = dyn_cast<DbgValueInst>(&I))
     processDbgValue(*DVI, LiveSet);
 }
 
 void AssignmentTrackingLowering::resetInsertionPoint(Instruction &After) {
   assert(!After.isTerminator() && "Can't insert after a terminator");

llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h

public:
   bool isInvalidated() const { return Invalid; }
 
   /// setIsEmitted / isEmitted - Getter/Setter for flag indicating that this
   /// SDDbgValue has been emitted to an MBB.
   void setIsEmitted() { Emitted = true; }
   bool isEmitted() const { return Emitted; }
 
   /// clearIsEmitted - Reset Emitted flag, for certain special cases where
-  /// dbg.addr is emitted twice.
+  /// SDDbgValue is emitted twice. DBG_INSTR_REF depends on this behaviour.
   void clearIsEmitted() { Emitted = false; }
 
   LLVM_DUMP_METHOD void dump() const;
   LLVM_DUMP_METHOD void print(raw_ostream &OS) const;
 };
 
 /// Holds the information from a dbg_label node through SDISel.
 /// We do not use SDValue here to avoid including its header.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Ops[2] = DAG.getTargetConstant(*cast<ConstantInt>(I.getArgOperand(1)), sdl,
                                    MVT::i32); // arg index
     SDValue Res = DAG.getNode(
         ISD::PREALLOCATED_ARG, sdl,
         DAG.getVTList(TLI.getPointerTy(DAG.getDataLayout()), MVT::Other), Ops);
     setValue(&I, Res);
     DAG.setRoot(Res.getValue(1));
     return;
   }
-  case Intrinsic::dbg_addr:
   case Intrinsic::dbg_declare: {
-    // Debug intrinsics are handled seperately in assignment tracking mode.
+    // Debug intrinsics are handled separately in assignment tracking mode.
     if (isAssignmentTrackingEnabled(*I.getFunction()->getParent()))
       return;
-    // Assume dbg.addr and dbg.declare can not currently use DIArgList, i.e.
-    // they are non-variadic.
+    // Assume dbg.declare can not currently use DIArgList, i.e.
+    // it is non-variadic.
     const auto &DI = cast<DbgVariableIntrinsic>(I);
     assert(!DI.hasArgList() && "Only dbg.value should currently use DIArgList");
     DILocalVariable *Variable = DI.getVariable();
     DIExpression *Expression = DI.getExpression();
     dropDanglingDebugInfo(Variable, Expression);
     assert(Variable && "Missing variable");
     LLVM_DEBUG(dbgs() << "SelectionDAG visiting debug intrinsic: " << DI
                       << "\n");
if (const auto *AI =
         if (I != FuncInfo.StaticAllocaMap.end())
           FI = I->second;
       }
     } else if (const auto *Arg = dyn_cast<Argument>(
                    Address->stripInBoundsConstantOffsets())) {
       FI = FuncInfo.getArgumentFrameIndex(Arg);
     }
 
-    // llvm.dbg.addr is control dependent and always generates indirect
-    // DBG_VALUE instructions. llvm.dbg.declare is handled as a frame index in
-    // the MachineFunction variable table.
+    // llvm.dbg.declare is handled as a frame index in the MachineFunction
+    // variable table.
     if (FI != std::numeric_limits<int>::max()) {
-      if (Intrinsic == Intrinsic::dbg_addr) {
-        SDDbgValue *SDV = DAG.getFrameIndexDbgValue(
-            Variable, Expression, FI, getRoot().getNode(), /*IsIndirect*/ true,
-            dl, SDNodeOrder);
-        DAG.AddDbgValue(SDV, isParameter);
-      } else {
-        LLVM_DEBUG(dbgs() << "Skipping " << DI
-                          << " (variable info stashed in MF side table)\n");
-      }
       return;
     }
 
     SDValue &N = NodeMap[Address];
     if (!N.getNode() && isa<Argument>(Address))
       // Check unused arguments map.
       N = UnusedArgNodeMap[Address];
     SDDbgValue *SDV;

llvm/test/DebugInfo/X86/dbg-addr-dse.ll

-; RUN: llc %s -o %t.s -experimental-debug-variable-locations=true
-; RUN: llvm-mc %t.s -filetype=obj -triple=x86_64-windows-msvc -o %t.o
-; RUN: FileCheck %s < %t.s --check-prefix=ASM
-; RUN: llvm-dwarfdump %t.o | FileCheck %s --check-prefix=DWARF
-
-; In this example, the variable lives mostly in memory, but at the point of the
-; assignment to global, it lives nowhere, and is described as the constant
-; value 1.
-
-; C source:
-;
-; void escape(int *);
-; extern int global;
-; void f(int x) {
-;   escape(&x);
-;   x = 1; // DSE should delete and insert dbg.value(i32 1)
-;   global = x;
-;   x = 2; // DSE should insert dbg.addr
-;   escape(&x);
-; }
-
-; ModuleID = 'dse.c'
-source_filename = "dse.c"
-target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-windows-msvc19.0.24215"
-
-declare void @llvm.dbg.addr(metadata, metadata, metadata) #2
-declare void @llvm.dbg.value(metadata, metadata, metadata) #2
-declare void @escape(ptr)
-
-@global = external global i32, align 4
-
-; Function Attrs: nounwind uwtable
-define void @f(i32 %x) #0 !dbg !8 {
-entry:
-  %x.addr = alloca i32, align 4
-  store i32 %x, ptr %x.addr, align 4
-  call void @llvm.dbg.addr(metadata ptr %x.addr, metadata !13, metadata !DIExpression()), !dbg !18
-  call void @escape(ptr %x.addr), !dbg !19
-  call void @llvm.dbg.value(metadata i32 1, metadata !13, metadata !DIExpression()), !dbg !20
-  store i32 1, ptr @global, align 4, !dbg !22
-  call void @llvm.dbg.addr(metadata ptr %x.addr, metadata !13, metadata !DIExpression()), !dbg !23
-  store i32 2, ptr %x.addr, align 4, !dbg !23
-  call void @escape(ptr %x.addr), !dbg !24
-  ret void, !dbg !25
-}
-
-; ASM-LABEL: f: # @f
-; ASM: #DEBUG_VALUE: f:x <- [DW_OP_plus_uconst [[OFF_X:[0-9]+]]] [$rsp+0]
-; ASM: movl    %ecx, [[OFF_X]](%rsp)
-; ASM: callq   escape
-; ASM: #DEBUG_VALUE: f:x <- 1
-; ASM: movl    $1, global(%rip)
-; ASM: #DEBUG_VALUE: f:x <- [DW_OP_plus_uconst [[OFF_X]]] [$rsp+0]
-; ASM: movl    $2, [[OFF_X]](%rsp)
-; ASM: callq   escape
-; ASM: retq
-
-; DWARF:      DW_TAG_formal_parameter
-; DWARF-NEXT:   DW_AT_location        (0x00000000
-; DWARF-NEXT:      {{[^:]*}}: DW_OP_breg7 RSP+{{[0-9]+}}
-; DWARF-NEXT:      {{[^:]*}}: DW_OP_consts +1, DW_OP_stack_value
-; DWARF-NEXT:      {{[^:]*}}: DW_OP_breg7 RSP+{{[0-9]+}})
-; DWARF-NEXT:   DW_AT_name    ("x")
-
-attributes #0 = { nounwind uwtable }
-attributes #2 = { nounwind readnone speculatable }
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!3, !4, !5, !6}
-!llvm.ident = !{!7}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
-!1 = !DIFile(filename: "dse.c", directory: "C:\5Csrc\5Cllvm-project\5Cbuild")
-!2 = !{}
-!3 = !{i32 2, !"Dwarf Version", i32 4}
-!4 = !{i32 2, !"Debug Info Version", i32 3}
-!5 = !{i32 1, !"wchar_size", i32 2}
-!6 = !{i32 7, !"PIC Level", i32 2}
-!7 = !{!"clang version 6.0.0 "}
-!8 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 3, type: !9, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !12)
-!9 = !DISubroutineType(types: !10)
-!10 = !{null, !11}
-!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!12 = !{!13}
-!13 = !DILocalVariable(name: "x", arg: 1, scope: !8, file: !1, line: 3, type: !11)
-!14 = !{!15, !15, i64 0}
-!15 = !{!"int", !16, i64 0}
-!16 = !{!"omnipotent char", !17, i64 0}
-!17 = !{!"Simple C/C++ TBAA"}
-!18 = !DILocation(line: 3, column: 12, scope: !8)
-!19 = !DILocation(line: 4, column: 3, scope: !8)
-!20 = !DILocation(line: 5, column: 5, scope: !8)
-!21 = !DILocation(line: 6, column: 12, scope: !8)
-!22 = !DILocation(line: 6, column: 10, scope: !8)
-!23 = !DILocation(line: 7, column: 5, scope: !8)
-!24 = !DILocation(line: 8, column: 3, scope: !8)
-!25 = !DILocation(line: 9, column: 1, scope: !8)

llvm/test/DebugInfo/X86/dbg-addr.ll

-;; Run twice -- once with DBG_VALUEs, once with instruction referencing.
-; RUN: llc %s -o %t.s -experimental-debug-variable-locations=false
-; RUN: llvm-mc -triple x86_64--linux %t.s -filetype=obj -o %t.o
-; RUN: FileCheck -input-file=%t.s %s
-; RUN: llvm-dwarfdump %t.o | FileCheck %s --check-prefix=DWARF
-; RUN: llc %s -o %t.s -experimental-debug-variable-locations=true
-; RUN: llvm-mc -triple x86_64--linux %t.s -filetype=obj -o %t.o
-; RUN: FileCheck -input-file=%t.s %s
-; RUN: llvm-dwarfdump %t.o | FileCheck %s --check-prefix=DWARF
-
-
-; Unlike dbg.declare, dbg.addr should be lowered to DBG_VALUE instructions. It
-; is control-dependent.
-
-; CHECK-LABEL: use_dbg_addr:
-; CHECK: #DEBUG_VALUE: use_dbg_addr:o <- [$rsp+0]
-; CHECK-NOT: #DEBUG_VALUE:
-
-; DWARF: DW_TAG_variable
-; DWARF-NEXT:              DW_AT_location (DW_OP_fbreg +0)
-; DWARF-NEXT:              DW_AT_name ("o")
-
-; Make sure that in the second case, we properly get a validity range in the
-; dwarf for the value. This ensures that we can use this technique to invalidate
-; variables.
-
-; CHECK-LABEL: test_dbg_addr_and_dbg_val_undef
-; CHECK: #DEBUG_VALUE: test_dbg_addr_and_dbg_val_undef:second_o <- [$rsp+0]
-; CHECK: #DEBUG_VALUE: test_dbg_addr_and_dbg_val_undef:second_o <- undef
-; CHECK-NOT: #DEBUG_VALUE:
-
-; DWARF: DW_TAG_variable
-; DWARF-NEXT:              DW_AT_location (0x{{[0-9a-z][0-9a-z]*}}:
-; DWARF-NEXT:                 [0x{{[0-9a-z][0-9a-z]*}}, 0x{{[0-9a-z][0-9a-z]*}}): DW_OP_breg7 RSP+0)
-; DWARF-NEXT:              DW_AT_name ("second_o")
-
-; ModuleID = 't.c'
-source_filename = "t.c"
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64--linux"
-
-%struct.Foo = type { i32 }
-
-; Function Attrs: noinline nounwind uwtable
-define void @use_dbg_addr() #0 !dbg !7 {
-entry:
-  %o = alloca %struct.Foo, align 8
-  call void @llvm.dbg.addr(metadata ptr %o, metadata !10, metadata !15), !dbg !16
-  call void @escape_foo(ptr %o), !dbg !17
-  ret void, !dbg !18
-}
-
-define void @test_dbg_addr_and_dbg_val_undef() #0 !dbg !117 {
-entry:
-  %o = alloca %struct.Foo, align 8
-  call void @llvm.dbg.addr(metadata ptr %o, metadata !1110, metadata !1115), !dbg !1116
-  call void @escape_foo(ptr %o), !dbg !1117
-  call void @llvm.dbg.value(metadata ptr undef, metadata !1110, metadata !1115), !dbg !1116
-  ret void, !dbg !1118
-}
-
-; Function Attrs: nounwind readnone speculatable
-declare void @llvm.dbg.addr(metadata, metadata, metadata) #1
-declare void @llvm.dbg.value(metadata, metadata, metadata) #1
-
-declare void @escape_foo(ptr)
-
-attributes #0 = { noinline nounwind uwtable }
-attributes #1 = { nounwind readnone speculatable }
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!3, !4, !5}
-!llvm.ident = !{!6}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
-!1 = !DIFile(filename: "t.c", directory: "C:\5Csrc\5Cllvm-project\5Cbuild")
-!2 = !{}
-!3 = !{i32 2, !"Dwarf Version", i32 4}
-!4 = !{i32 2, !"Debug Info Version", i32 3}
-!5 = !{i32 1, !"wchar_size", i32 4}
-!6 = !{!"clang version 6.0.0 "}
-!7 = distinct !DISubprogram(name: "use_dbg_addr", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
-!8 = !DISubroutineType(types: !9)
-!9 = !{null}
-!10 = !DILocalVariable(name: "o", scope: !7, file: !1, line: 4, type: !11)
-!11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo", file: !1, line: 1, size: 32, elements: !12)
-!12 = !{!13}
-!13 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !11, file: !1, line: 1, baseType: !14, size: 32)
-!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!15 = !DIExpression()
-!16 = !DILocation(line: 4, column: 14, scope: !7)
-!17 = !DILocation(line: 5, column: 3, scope: !7)
-!18 = !DILocation(line: 6, column: 1, scope: !7)
-
-!117 = distinct !DISubprogram(name: "test_dbg_addr_and_dbg_val_undef", scope: !1, file: !1, line: 3, type: !118, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
-!118 = !DISubroutineType(types: !119)
-!119 = !{null}
-!1110 = !DILocalVariable(name: "second_o", scope: !117, file: !1, line: 4, type: !1111)
-!1111 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo", file: !1, line: 1, size: 32, elements: !1112)
-!1112 = !{!1113}
-!1113 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !1111, file: !1, line: 1, baseType: !1114, size: 32)
-!1114 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!1115 = !DIExpression()
-!1116 = !DILocation(line: 4, column: 14, scope: !117)
-!1117 = !DILocation(line: 5, column: 3, scope: !117)
-!1118 = !DILocation(line: 6, column: 1, scope: !117)

llvm/test/DebugInfo/X86/merge-equivalent-ranges.ll

 ; XFAIL: target=aarch64-pc-windows-{{.*}}
 ; RUN: %llc_dwarf -filetype=obj -o %t.o < %s
 ; RUN: llvm-dwarfdump %t.o | FileCheck %s
 
 ;; Checks that when we have a set of debug values that are all different but
 ;; result in identical DW_AT_location entries, we are able to merge them into a
 ;; single DW_AT_location instead of producing a loclist with identical locations
 ;; for each.
 
-;; Checks that we can merge an indirect debug value with an equivalent direct
-;; debug value that uses DW_OP_deref.
-; CHECK: DW_AT_location
-; CHECK-SAME: (DW_OP_fbreg +{{[0-9]+}})
-; CHECK-NEXT: DW_AT_name    ("Var1")
-
 ;; Checks that we can merge a non-variadic debug value with an equivalent
 ;; variadic debug value.
 ; CHECK: DW_AT_location
 ; CHECK-SAME: (DW_OP_fbreg +{{[0-9]+}}, DW_OP_deref, DW_OP_stack_value)
 ; CHECK-NEXT: DW_AT_name    ("Var2")
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 %"struct.std::_Rb_tree_node_base" = type { i32, ptr, ptr, ptr }
 
 define i32 @_ZN4llvm9MCContext12GetDwarfFileENS_9StringRefES1_jj(ptr %this, ptr %A, i64 %B, ptr %C, i64 %D, i32 %E, i32 %CUID, i1 %cmp.i.i.i.i.i) !dbg !10 {
 entry:
   %CUID.addr = alloca i32, align 4
   store i32 %CUID, ptr %CUID.addr, align 4
-  call void @llvm.dbg.value(metadata ptr %CUID.addr, metadata !16, metadata !DIExpression(DW_OP_deref)), !dbg !17
   call void @llvm.dbg.value(metadata ptr %CUID.addr, metadata !26, metadata !DIExpression(DW_OP_deref, DW_OP_stack_value)), !dbg !17
   %0 = load ptr, ptr null, align 8, !dbg !18
   call void @llvm.dbg.value(metadata !DIArgList(ptr %CUID.addr), metadata !26, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_deref, DW_OP_stack_value)), !dbg !18
   br label %while.body.i.i.i.i
 
 while.body.i.i.i.i:                               ; preds = %while.body.i.i.i.i, %entry
   %__x.addr.011.i.i.i.i = phi ptr [ %__x.addr.1.in.i.i.i.i, %while.body.i.i.i.i ], [ %0, %entry ]
   %_M_right.i.i.i.i.i = getelementptr inbounds %"struct.std::_Rb_tree_node_base", ptr %__x.addr.011.i.i.i.i, i64 0, i32 3, !dbg !20
-  call void @llvm.dbg.addr(metadata ptr %CUID.addr, metadata !16, metadata !DIExpression()), !dbg !20
   %__x.addr.1.in.i.i.i.i = select i1 %cmp.i.i.i.i.i, ptr %__x.addr.011.i.i.i.i, ptr null, !dbg !20
   br label %while.body.i.i.i.i
 }
 
 declare void @llvm.dbg.value(metadata, metadata, metadata)
-declare void @llvm.dbg.addr(metadata, metadata, metadata)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
 !llvm.ident = !{!9}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 16.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
 !1 = !DIFile(filename: "test.cpp", directory: "/")
 !2 = !{i32 7, !"Dwarf Version", i32 5}