[HeterogeneousDwarf] Support DBG_INSTR_REF salvaging of REG_SEQUENCE (llvm#1886)

Just like COPY, REG_SEQUENCE isn't a good instruction to attach a
debug-instr-number to. Dig back through the registers it's composing to
find a set of defining instructions, and combine them with a
DIOpComposite.

Author: epilk

llvm/include/llvm/CodeGen/MachineFunction.h

@@ -614,6 +614,8 @@ class LLVM_ABI MachineFunction {
   void substituteDebugValuesForInst(const MachineInstr &Old, MachineInstr &New,
                                     unsigned MaxOperand = UINT_MAX);
 
+  using SalvageCopySSAResult = std::pair<DebugInstrOperandPair, MachineInstr *>;
+
   /// Find the underlying  defining instruction / operand for a COPY instruction
   /// while in SSA form. Copies do not actually define values -- they move them
   /// between registers. Labelling a COPY-like instruction with an instruction
@@ -625,11 +627,11 @@ class LLVM_ABI MachineFunction {
   /// \p MI The copy-like instruction to salvage.
   /// \p DbgPHICache A container to cache already-solved COPYs.
   /// \returns An instruction/operand pair identifying the defining value.
-  DebugInstrOperandPair
+  SalvageCopySSAResult
   salvageCopySSA(MachineInstr &MI,
-                 DenseMap<Register, DebugInstrOperandPair> &DbgPHICache);
+                 DenseMap<Register, SalvageCopySSAResult> &DbgPHICache);
 
-  DebugInstrOperandPair salvageCopySSAImpl(MachineInstr &MI);
+  SalvageCopySSAResult salvageCopySSAImpl(MachineInstr &MI);
 
   /// Finalise any partially emitted debug instructions. These are DBG_INSTR_REF
   /// instructions where we only knew the vreg of the value they use, not the

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

@@ -1188,7 +1188,27 @@ std::optional<NewOpResult> DwarfExpression::traverse(DIOp::Select Select,
 
 std::optional<NewOpResult> DwarfExpression::traverse(DIOp::Composite Composite,
                                                      ChildrenT Children) {
-  return std::nullopt;
+  if (IsFragment)
+    emitOp(dwarf::DW_OP_lit0);
+
+  for (auto &Child : Children) {
+    auto R = traverse(Child.get(), std::nullopt);
+    if (!R)
+      return std::nullopt;
+    TypeSize Size = R->Ty->getPrimitiveSizeInBits();
+    if (!Size.isFixed() || Size.getFixedValue() % 8 != 0)
+      return std::nullopt;
+    emitOp(dwarf::DW_OP_piece);
+    emitUnsigned(Size.getFixedValue() / 8);
+  }
+  emitUserOp(dwarf::DW_OP_LLVM_piece_end);
+
+  if (IsFragment) {
+    emitOp(dwarf::DW_OP_swap);
+    emitOp(dwarf::DW_OP_drop);
+  }
+
+  return NewOpResult{Composite.getResultType(), ValueKind::LocationDesc};
 }
 
 std::optional<NewOpResult> DwarfExpression::traverseMathOp(uint8_t DwarfOp,

llvm/lib/CodeGen/MachineFunction.cpp

@@ -45,6 +45,7 @@
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/EHPersonalities.h"
 #include "llvm/IR/Function.h"
@@ -1052,8 +1053,8 @@ void MachineFunction::substituteDebugValuesForInst(const MachineInstr &Old,
 }
 
 auto MachineFunction::salvageCopySSA(
-    MachineInstr &MI, DenseMap<Register, DebugInstrOperandPair> &DbgPHICache)
-    -> DebugInstrOperandPair {
+    MachineInstr &MI, DenseMap<Register, SalvageCopySSAResult> &DbgPHICache)
+    -> SalvageCopySSAResult {
   const TargetInstrInfo &TII = *getSubtarget().getInstrInfo();
 
   // Check whether this copy-like instruction has already been salvaged into
@@ -1077,7 +1078,7 @@ auto MachineFunction::salvageCopySSA(
 }
 
 auto MachineFunction::salvageCopySSAImpl(MachineInstr &MI)
-    -> DebugInstrOperandPair {
+    -> SalvageCopySSAResult {
   MachineRegisterInfo &MRI = getRegInfo();
   const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo();
   const TargetInstrInfo &TII = *getSubtarget().getInstrInfo();
@@ -1175,7 +1176,8 @@ auto MachineFunction::salvageCopySSAImpl(MachineInstr &MI)
     for (auto &MO : Inst->all_defs()) {
       if (MO.getReg() != State.first)
         continue;
-      return ApplySubregisters({Inst->getDebugInstrNum(), MO.getOperandNo()});
+      return {ApplySubregisters({Inst->getDebugInstrNum(), MO.getOperandNo()}),
+              Inst};
     }
 
     llvm_unreachable("Vreg def with no corresponding operand?");
@@ -1195,8 +1197,9 @@ auto MachineFunction::salvageCopySSAImpl(MachineInstr &MI)
       if (!TRI.regsOverlap(RegToSeek, MO.getReg()))
         continue;
 
-      return ApplySubregisters(
-          {ToExamine.getDebugInstrNum(), MO.getOperandNo()});
+      return {
+          ApplySubregisters({ToExamine.getDebugInstrNum(), MO.getOperandNo()}),
+          &ToExamine};
     }
   }
 
@@ -1217,7 +1220,131 @@ auto MachineFunction::salvageCopySSAImpl(MachineInstr &MI)
   Builder.addReg(State.first);
   unsigned NewNum = getNewDebugInstrNum();
   Builder.addImm(NewNum);
-  return ApplySubregisters({NewNum, 0u});
+  return {ApplySubregisters({NewNum, 0u}), nullptr};
+}
+
+/// The Op operand to the DBG_INSTR_REF instruction DbgInstr is a virtual
+/// register defined by the REG_SEQUENCE instruction RegSeq. In order to
+/// finalize DbgInstr to use instruction references, find the defining
+/// instruction for each register in the sequence and compose them with a
+/// DIOpComposite.
+static bool finalizeInstrRefRegSequenceNew(
+    MachineInstr &DbgInstr, MachineOperand &Op, MachineInstr &RegSeq,
+    DenseMap<Register, MachineFunction::SalvageCopySSAResult> &DbgPHICache) {
+
+  const DIExpression *Expr = DbgInstr.getDebugExpression();
+  if (Expr->holdsOldElements())
+    return false;
+
+  auto &MF = *DbgInstr.getParent()->getParent();
+  auto &Ctx = Expr->getContext();
+  auto &TRI = *MF.getSubtarget().getRegisterInfo();
+  auto &TII = *MF.getSubtarget().getInstrInfo();
+  auto &DL = MF.getDataLayout();
+
+  struct Part {
+    MachineFunction::DebugInstrOperandPair DbgInstrNum;
+    unsigned Size;
+    unsigned Offset;
+  };
+  SmallVector<Part> Parts;
+
+  // Walk through the reg sequence, collecting debug-instr-numbers and
+  // subregister piece sizes and offsets into Parts.
+  for (unsigned I = 1; I < RegSeq.getNumOperands(); I += 2) {
+    Register RegOp = RegSeq.getOperand(I).getReg();
+    if (!RegOp.isVirtual())
+      return false;
+
+    unsigned SubReg = RegSeq.getOperand(I + 1).getImm();
+    unsigned SubSize = TRI.getSubRegIdxSize(SubReg);
+    unsigned SubOffset = TRI.getSubRegIdxOffset(SubReg);
+    MachineInstr &DefMI = *MF.getRegInfo().def_instr_begin(RegOp);
+
+    if (DefMI.isCopyLike() || TII.isCopyInstr(DefMI)) {
+      auto P = MF.salvageCopySSA(DefMI, DbgPHICache);
+      Parts.push_back({P.first, SubSize, SubOffset});
+      continue;
+    }
+
+    // Otherwise, identify the operand number that the VReg refers to.
+    unsigned OperandIdx = 0;
+    for (const auto &DefMO : DefMI.operands()) {
+      if (DefMO.isReg() && DefMO.isDef() && DefMO.getReg() == RegOp)
+        break;
+      ++OperandIdx;
+    }
+    assert(OperandIdx < DefMI.getNumOperands());
+
+    // Morph this instr ref to point at the given instruction and operand.
+    unsigned ID = DefMI.getDebugInstrNum();
+    MachineFunction::DebugInstrOperandPair P{ID, OperandIdx};
+    Parts.push_back({P, SubSize, SubOffset});
+  }
+
+  // Line up the Parts and make sure there aren't any gaps, DIOpComposite can't
+  // handle that easily.
+  std::sort(Parts.begin(), Parts.end(),
+            [](auto &LHS, auto &RHS) { return LHS.Offset < RHS.Offset; });
+  for (unsigned I = 1, E = Parts.size(); I < E; ++I)
+    if (Parts[I - 1].Offset + Parts[I - 1].Size != Parts[I].Offset)
+      return false;
+  if (Parts.empty() || Parts[0].Offset)
+    return false;
+
+  unsigned ArgNoToReplace = 0;
+  unsigned NumArgs = DbgInstr.getNumDebugOperands();
+  assert(NumArgs == Expr->getNewNumLocationOperands());
+  for (; ArgNoToReplace != NumArgs; ++ArgNoToReplace)
+    if (&DbgInstr.getDebugOperand(ArgNoToReplace) == &Op)
+      break;
+  if (ArgNoToReplace == NumArgs)
+    return false;
+
+  auto Elems = Expr->getNewElementsRef();
+  auto NewSize = TypeSize::getFixed(Parts.back().Offset + Parts.back().Size);
+  for (DIOp::Variant Elem : *Elems) {
+    // Only replace the argument with a composite if it has the same size as the
+    // parts.
+    if (auto *Arg = std::get_if<DIOp::Arg>(&Elem))
+      if (Arg->getIndex() == ArgNoToReplace &&
+          DL.getTypeSizeInBits(Arg->getResultType()) != NewSize)
+        return false;
+  }
+
+  Op.ChangeToDbgInstrRef(Parts[0].DbgInstrNum.first,
+                         Parts[0].DbgInstrNum.second);
+  if (Parts.size() == 1)
+    return true;
+
+  // Split up the DIOpArg using a DIOpComposite.
+  DIExprBuilder B{Ctx};
+  for (DIOp::Variant Elem : *Elems) {
+    auto *Arg = std::get_if<DIOp::Arg>(&Elem);
+    if (!Arg || Arg->getIndex() != ArgNoToReplace) {
+      B.append(Elem);
+      continue;
+    }
+    bool FirstPart = true;
+    for (const Part &P : Parts) {
+      // Since these arguments have to line up with the order of the operands on
+      // the DBG_INSTR_REF, recycle Arg's index first, it lines up with the Op
+      // that was ChangeToDbgInstrRef'd above.
+      unsigned ArgNo = FirstPart ? Arg->getIndex() : NumArgs++;
+      FirstPart = false;
+      B.append<DIOp::Arg>(ArgNo, IntegerType::get(Ctx, P.Size));
+    }
+    B.append<DIOp::Composite>(Parts.size(), Arg->getResultType());
+  }
+
+  auto *NewExpr = B.intoExpression();
+  for (const Part &P : drop_begin(Parts, 1))
+    DbgInstr.addOperand(MachineOperand::CreateDbgInstrRef(
+        P.DbgInstrNum.first, P.DbgInstrNum.second));
+  DbgInstr.getDebugExpressionOp() = MachineOperand::CreateMetadata(NewExpr);
+  assert(NewExpr->getNewNumLocationOperands() ==
+         DbgInstr.getNumDebugOperands());
+  return true;
 }
 
 void MachineFunction::finalizeDebugInstrRefs() {
@@ -1229,15 +1356,16 @@ void MachineFunction::finalizeDebugInstrRefs() {
     MI.setDebugValueUndef();
   };
 
-  DenseMap<Register, DebugInstrOperandPair> ArgDbgPHIs;
+  DenseMap<Register, SalvageCopySSAResult> ArgDbgPHIs;
   for (auto &MBB : *this) {
     for (auto &MI : MBB) {
       if (!MI.isDebugRef())
         continue;
 
       bool IsValidRef = true;
 
-      for (MachineOperand &MO : MI.debug_operands()) {
+      for (unsigned I = 0; I < MI.getNumDebugOperands(); ++I) {
+        MachineOperand &MO = MI.getDebugOperand(I);
         if (!MO.isReg())
           continue;
 
@@ -1259,7 +1387,12 @@ void MachineFunction::finalizeDebugInstrRefs() {
         // for why this is important.
         if (DefMI.isCopyLike() || TII->isCopyInstr(DefMI)) {
           auto Result = salvageCopySSA(DefMI, ArgDbgPHIs);
-          MO.ChangeToDbgInstrRef(Result.first, Result.second);
+          if (!Result.second || !Result.second->isRegSequence() ||
+              !finalizeInstrRefRegSequenceNew(MI, MO, *Result.second,
+                                              ArgDbgPHIs))
+            MO.ChangeToDbgInstrRef(Result.first.first, Result.first.second);
+        } else if (DefMI.isRegSequence() &&
+                   finalizeInstrRefRegSequenceNew(MI, MO, DefMI, ArgDbgPHIs)) {
         } else {
           // Otherwise, identify the operand number that the VReg refers to.
           unsigned OperandIdx = 0;

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

@@ -882,7 +882,8 @@ InstrEmitter::EmitDbgInstrRef(SDDbgValue *SD,
     // Avoid copy like instructions: they don't define values, only move them.
     // Leave a virtual-register reference until it can be fixed up later, to
     // find the underlying value definition.
-    if (DefMI->isCopyLike() || TII->isCopyInstr(*DefMI)) {
+    if (DefMI->isCopyLike() || TII->isCopyInstr(*DefMI) ||
+        (Expr->holdsNewElements() && DefMI->isRegSequence())) {
       AddVRegOp(VReg);
       continue;
     }

llvm/test/DebugInfo/AMDGPU/heterogeneous-dwarf-diop-diexpression-subregs.mir

@@ -40,6 +40,7 @@
   !24 = !DILocalVariable(name: "sgpr", scope: !9, file: !1, line: 1, type: !14)
   !25 = !DILocalVariable(name: "vgpr", scope: !9, file: !1, line: 1, type: !14)
   !26 = !DILocalVariable(name: "vgpr_frags", scope: !9, file: !1, line: 1, type: !15)
+  !27 = !DILocalVariable(name: "composite", scope: !9, file: !1, line: 1, type: !15)
 
 ...
 ---
@@ -93,6 +94,11 @@ body: |
     DBG_VALUE renamable $vgpr42_vgpr43, $noreg, !26, !DIExpression(DIOpArg(0, i64), DIOpFragment(0, 32)), debug-location !16
     DBG_VALUE renamable $vgpr44_vgpr45, $noreg, !26, !DIExpression(DIOpArg(0, i64), DIOpFragment(32, 32)), debug-location !16
 
+    ; CHECK: DW_TAG_variable
+    ; CHECK-NEXT: DW_AT_location (DW_OP_regx SGPR10, DW_OP_piece 0x4, DW_OP_regx SGPR11, DW_OP_piece 0x4, DW_OP_LLVM_user DW_OP_LLVM_piece_end)
+    ; CHECK-NEXT: DW_AT_name ("composite")
+    DBG_VALUE_LIST !27, !DIExpression(DIOpArg(0, i32), DIOpArg(1, i32), DIOpComposite(2, i64)), $sgpr10, $sgpr11, debug-location !16
+
     S_ENDPGM 0, debug-location !16
 
 ...

llvm/test/DebugInfo/AMDGPU/reg-sequence-salvage.ll

@@ -0,0 +1,51 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1030 -start-before=amdgpu-isel -stop-after=amdgpu-isel %s -o - | FileCheck %s
+
+define i64 @test(ptr addrspace(1) %p) !dbg !11 {
+  ; CHECK-LABEL: name: test
+  ; CHECK: bb.0 (%ir-block.0):
+  ; CHECK-NEXT:   liveins: $vgpr0, $vgpr1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr1
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+  ; CHECK-NEXT:   [[REG_SEQUENCE:%[0-9]+]]:sreg_64 = REG_SEQUENCE [[COPY1]], %subreg.sub0, [[COPY]], %subreg.sub1
+  ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:vreg_64 = COPY [[REG_SEQUENCE]]
+  ; CHECK-NEXT:   [[GLOBAL_LOAD_DWORD:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD killed [[COPY2]], 0, 0, implicit $exec, debug-instr-number 1 :: (load (s32) from %ir.p, addrspace 1)
+  ; CHECK-NEXT:   [[V_ASHRREV_I32_e64_:%[0-9]+]]:vgpr_32 = V_ASHRREV_I32_e64 31, [[GLOBAL_LOAD_DWORD]], implicit $exec, debug-instr-number 2
+  ; CHECK-NEXT:   [[COPY3:%[0-9]+]]:vgpr_32 = COPY [[V_ASHRREV_I32_e64_]]
+  ; CHECK-NEXT:   [[REG_SEQUENCE1:%[0-9]+]]:vreg_64 = REG_SEQUENCE [[GLOBAL_LOAD_DWORD]], %subreg.sub0, killed [[COPY3]], %subreg.sub1
+  ; CHECK-NEXT:   DBG_INSTR_REF !17, !DIExpression(DIOpArg(0, i32), DIOpArg(1, i32), DIOpComposite(2, i64)), dbg-instr-ref(1, 0), dbg-instr-ref(2, 0), debug-location !18
+  ; CHECK-NEXT:   [[COPY4:%[0-9]+]]:vgpr_32 = COPY [[REG_SEQUENCE1]].sub1
+  ; CHECK-NEXT:   $vgpr0 = COPY [[GLOBAL_LOAD_DWORD]]
+  ; CHECK-NEXT:   $vgpr1 = COPY [[COPY4]]
+  ; CHECK-NEXT:   SI_RETURN implicit $vgpr0, implicit $vgpr1
+  %load = load i32, ptr addrspace(1) %p, align 4
+  %conv = sext i32 %load to i64
+    #dbg_value(i64 %conv, !17, !DIExpression(DIOpArg(0, i64)), !18)
+  ret i64 %conv
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7}
+!opencl.ocl.version = !{!8}
+!llvm.ident = !{!9, !10}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 21.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
+!1 = !DIFile(filename: "t.cpp", directory: "/")
+!2 = !{i32 1, !"amdhsa_code_object_version", i32 600}
+!3 = !{i32 1, !"amdgpu_printf_kind", !"hostcall"}
+!4 = !{i32 7, !"Dwarf Version", i32 5}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = !{i32 1, !"wchar_size", i32 4}
+!7 = !{i32 8, !"PIC Level", i32 2}
+!8 = !{i32 2, i32 0}
+!9 = !{!"clang version 21.0.0"}
+!10 = !{!"clang version 18.0.0"}
+!11 = distinct !DISubprogram(name: "test", linkageName: "test", scope: !1, file: !1, line: 6, type: !12, scopeLine: 7, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !16)
+!12 = !DISubroutineType(types: !13)
+!13 = !{!15, !14}
+!14 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !15, size: 64)
+!15 = !DIBasicType(name: "long", size: 64, encoding: DW_ATE_signed)
+!16 = !{!17}
+!17 = !DILocalVariable(name: "var", scope: !11, file: !1, line: 8, type: !15)
+!18 = !DILocation(line: 0, scope: !11)