[llvm] [NFC][AsmPrinter] Refactor constructVariableDIE (PR #66435)

Scott Linder via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 20 13:32:00 PDT 2023


https://github.com/slinder1 updated https://github.com/llvm/llvm-project/pull/66435

>From e6e39c9c269a80d5892a96a254af6a59f8e5cbe9 Mon Sep 17 00:00:00 2001
From: Scott Linder <Scott.Linder at amd.com>
Date: Wed, 13 Sep 2023 17:34:35 +0000
Subject: [PATCH 1/2] [NFC][AsmPrinter] Refactor constructVariableDIE

Fold constructVariableDIEImpl into constructVariableDIE,
simplify it and group related functions.

Pull out the previously inline lambdas for visiting the active
variant of the DbgVariable to add location and related attributes
as an overload set for a private method
applyConcreteDbgVariableAttributes.

Rename applyVariableAttribute to reflect what kinds of attributes it
applies, and to contrast it with the new
applyConcreteDbgVariableAttributes.

Move constructLabelDIE down in the implementation file, so all of
the constructVariableDIE-related function impls are adjacent.
---
 .../CodeGen/AsmPrinter/DwarfCompileUnit.cpp   | 427 +++++++++---------
 .../lib/CodeGen/AsmPrinter/DwarfCompileUnit.h |  41 +-
 2 files changed, 246 insertions(+), 222 deletions(-)

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index e37dd14c96f76b3..57531e3660c039c 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -740,232 +740,213 @@ DIE *DwarfCompileUnit::constructLexicalScopeDIE(LexicalScope *Scope) {
   return ScopeDIE;
 }
 
-/// constructVariableDIE - Construct a DIE for the given DbgVariable.
 DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV, bool Abstract) {
-  auto D = constructVariableDIEImpl(DV, Abstract);
-  DV.setDIE(*D);
-  return D;
+  auto *VariableDie = DIE::get(DIEValueAllocator, DV.getTag());
+  insertDIE(DV.getVariable(), VariableDie);
+  DV.setDIE(*VariableDie);
+  // Abstract variables don't get common attributes later, so apply them now.
+  if (Abstract) {
+    applyCommonDbgVariableAttributes(DV, *VariableDie);
+  } else {
+    std::visit(
+        [&](const auto &V) {
+          applyConcreteDbgVariableAttributes(V, DV, VariableDie);
+        },
+        DV.asVariant());
+  }
+  return VariableDie;
 }
 
-DIE *DwarfCompileUnit::constructLabelDIE(DbgLabel &DL,
-                                         const LexicalScope &Scope) {
-  auto LabelDie = DIE::get(DIEValueAllocator, DL.getTag());
-  insertDIE(DL.getLabel(), LabelDie);
-  DL.setDIE(*LabelDie);
+void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
+    const Loc::Single &Single, const DbgVariable &DV, DIE *VariableDie) {
+  const DbgValueLoc *DVal = &Single.getValueLoc();
+  if (!DVal->isVariadic()) {
+    const DbgValueLocEntry *Entry = DVal->getLocEntries().begin();
+    if (Entry->isLocation()) {
+      addVariableAddress(DV, *VariableDie, Entry->getLoc());
+    } else if (Entry->isInt()) {
+      auto *Expr = Single.getExpr();
+      if (Expr && Expr->getNumElements()) {
+        DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+        DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+        // If there is an expression, emit raw unsigned bytes.
+        DwarfExpr.addFragmentOffset(Expr);
+        DwarfExpr.addUnsignedConstant(Entry->getInt());
+        DwarfExpr.addExpression(Expr);
+        addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+        if (DwarfExpr.TagOffset)
+          addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
+                  dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
+      } else
+        addConstantValue(*VariableDie, Entry->getInt(), DV.getType());
+    } else if (Entry->isConstantFP()) {
+      addConstantFPValue(*VariableDie, Entry->getConstantFP());
+    } else if (Entry->isConstantInt()) {
+      addConstantValue(*VariableDie, Entry->getConstantInt(), DV.getType());
+    } else if (Entry->isTargetIndexLocation()) {
+      DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+      DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+      const DIBasicType *BT = dyn_cast<DIBasicType>(
+          static_cast<const Metadata *>(DV.getVariable()->getType()));
+      DwarfDebug::emitDebugLocValue(*Asm, BT, *DVal, DwarfExpr);
+      addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+    }
+    return;
+  }
+  // If any of the location entries are registers with the value 0,
+  // then the location is undefined.
+  if (any_of(DVal->getLocEntries(), [](const DbgValueLocEntry &Entry) {
+        return Entry.isLocation() && !Entry.getLoc().getReg();
+      }))
+    return;
+  const DIExpression *Expr = Single.getExpr();
+  assert(Expr && "Variadic Debug Value must have an Expression.");
+  DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+  DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+  DwarfExpr.addFragmentOffset(Expr);
+  DIExpressionCursor Cursor(Expr);
+  const TargetRegisterInfo &TRI = *Asm->MF->getSubtarget().getRegisterInfo();
 
-  if (Scope.isAbstractScope())
-    applyLabelAttributes(DL, *LabelDie);
+  auto AddEntry = [&](const DbgValueLocEntry &Entry,
+                      DIExpressionCursor &Cursor) {
+    if (Entry.isLocation()) {
+      if (!DwarfExpr.addMachineRegExpression(TRI, Cursor,
+                                             Entry.getLoc().getReg()))
+        return false;
+    } else if (Entry.isInt()) {
+      // If there is an expression, emit raw unsigned bytes.
+      DwarfExpr.addUnsignedConstant(Entry.getInt());
+    } else if (Entry.isConstantFP()) {
+      // DwarfExpression does not support arguments wider than 64 bits
+      // (see PR52584).
+      // TODO: Consider chunking expressions containing overly wide
+      // arguments into separate pointer-sized fragment expressions.
+      APInt RawBytes = Entry.getConstantFP()->getValueAPF().bitcastToAPInt();
+      if (RawBytes.getBitWidth() > 64)
+        return false;
+      DwarfExpr.addUnsignedConstant(RawBytes.getZExtValue());
+    } else if (Entry.isConstantInt()) {
+      APInt RawBytes = Entry.getConstantInt()->getValue();
+      if (RawBytes.getBitWidth() > 64)
+        return false;
+      DwarfExpr.addUnsignedConstant(RawBytes.getZExtValue());
+    } else if (Entry.isTargetIndexLocation()) {
+      TargetIndexLocation Loc = Entry.getTargetIndexLocation();
+      // TODO TargetIndexLocation is a target-independent. Currently
+      // only the WebAssembly-specific encoding is supported.
+      assert(Asm->TM.getTargetTriple().isWasm());
+      DwarfExpr.addWasmLocation(Loc.Index, static_cast<uint64_t>(Loc.Offset));
+    } else {
+      llvm_unreachable("Unsupported Entry type.");
+    }
+    return true;
+  };
 
-  return LabelDie;
+  if (!DwarfExpr.addExpression(
+          std::move(Cursor),
+          [&](unsigned Idx, DIExpressionCursor &Cursor) -> bool {
+            return AddEntry(DVal->getLocEntries()[Idx], Cursor);
+          }))
+    return;
+
+  // Now attach the location information to the DIE.
+  addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+  if (DwarfExpr.TagOffset)
+    addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
+            *DwarfExpr.TagOffset);
 }
 
-DIE *DwarfCompileUnit::constructVariableDIEImpl(const DbgVariable &DV,
-                                                bool Abstract) {
-  // Define variable debug information entry.
-  auto VariableDie = DIE::get(DIEValueAllocator, DV.getTag());
-  insertDIE(DV.getVariable(), VariableDie);
+void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
+    const Loc::Multi &Multi, const DbgVariable &DV, DIE *VariableDie) {
+  addLocationList(*VariableDie, dwarf::DW_AT_location,
+                  Multi.getDebugLocListIndex());
+  auto TagOffset = Multi.getDebugLocListTagOffset();
+  if (TagOffset)
+    addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
+            *TagOffset);
+}
 
-  if (Abstract) {
-    applyVariableAttributes(DV, *VariableDie);
-    return VariableDie;
-  }
+void DwarfCompileUnit::applyConcreteDbgVariableAttributes(const Loc::MMI &MMI,
+                                                          const DbgVariable &DV,
+                                                          DIE *VariableDie) {
+  std::optional<unsigned> NVPTXAddressSpace;
+  DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+  DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+  for (const auto &Fragment : MMI.getFrameIndexExprs()) {
+    Register FrameReg;
+    const DIExpression *Expr = Fragment.Expr;
+    const TargetFrameLowering *TFI = Asm->MF->getSubtarget().getFrameLowering();
+    StackOffset Offset =
+        TFI->getFrameIndexReference(*Asm->MF, Fragment.FI, FrameReg);
+    DwarfExpr.addFragmentOffset(Expr);
 
-  // Add variable address.
-  std::visit(
-      makeVisitor(
-          [=, &DV](const Loc::Single &Single) {
-            const DbgValueLoc *DVal = &Single.getValueLoc();
-            if (!DVal->isVariadic()) {
-              const DbgValueLocEntry *Entry = DVal->getLocEntries().begin();
-              if (Entry->isLocation()) {
-                addVariableAddress(DV, *VariableDie, Entry->getLoc());
-              } else if (Entry->isInt()) {
-                auto *Expr = Single.getExpr();
-                if (Expr && Expr->getNumElements()) {
-                  DIELoc *Loc = new (DIEValueAllocator) DIELoc;
-                  DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
-                  // If there is an expression, emit raw unsigned bytes.
-                  DwarfExpr.addFragmentOffset(Expr);
-                  DwarfExpr.addUnsignedConstant(Entry->getInt());
-                  DwarfExpr.addExpression(Expr);
-                  addBlock(*VariableDie, dwarf::DW_AT_location,
-                           DwarfExpr.finalize());
-                  if (DwarfExpr.TagOffset)
-                    addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
-                            dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
-                } else
-                  addConstantValue(*VariableDie, Entry->getInt(), DV.getType());
-              } else if (Entry->isConstantFP()) {
-                addConstantFPValue(*VariableDie, Entry->getConstantFP());
-              } else if (Entry->isConstantInt()) {
-                addConstantValue(*VariableDie, Entry->getConstantInt(),
-                                 DV.getType());
-              } else if (Entry->isTargetIndexLocation()) {
-                DIELoc *Loc = new (DIEValueAllocator) DIELoc;
-                DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
-                const DIBasicType *BT = dyn_cast<DIBasicType>(
-                    static_cast<const Metadata *>(DV.getVariable()->getType()));
-                DwarfDebug::emitDebugLocValue(*Asm, BT, *DVal, DwarfExpr);
-                addBlock(*VariableDie, dwarf::DW_AT_location,
-                         DwarfExpr.finalize());
-              }
-              return;
-            }
-            // If any of the location entries are registers with the value 0,
-            // then the location is undefined.
-            if (any_of(DVal->getLocEntries(),
-                       [](const DbgValueLocEntry &Entry) {
-                         return Entry.isLocation() && !Entry.getLoc().getReg();
-                       }))
-              return;
-            const DIExpression *Expr = Single.getExpr();
-            assert(Expr && "Variadic Debug Value must have an Expression.");
-            DIELoc *Loc = new (DIEValueAllocator) DIELoc;
-            DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
-            DwarfExpr.addFragmentOffset(Expr);
-            DIExpressionCursor Cursor(Expr);
-            const TargetRegisterInfo &TRI =
-                *Asm->MF->getSubtarget().getRegisterInfo();
-
-            auto AddEntry = [&](const DbgValueLocEntry &Entry,
-                                DIExpressionCursor &Cursor) {
-              if (Entry.isLocation()) {
-                if (!DwarfExpr.addMachineRegExpression(TRI, Cursor,
-                                                       Entry.getLoc().getReg()))
-                  return false;
-              } else if (Entry.isInt()) {
-                // If there is an expression, emit raw unsigned bytes.
-                DwarfExpr.addUnsignedConstant(Entry.getInt());
-              } else if (Entry.isConstantFP()) {
-                // DwarfExpression does not support arguments wider than 64 bits
-                // (see PR52584).
-                // TODO: Consider chunking expressions containing overly wide
-                // arguments into separate pointer-sized fragment expressions.
-                APInt RawBytes =
-                    Entry.getConstantFP()->getValueAPF().bitcastToAPInt();
-                if (RawBytes.getBitWidth() > 64)
-                  return false;
-                DwarfExpr.addUnsignedConstant(RawBytes.getZExtValue());
-              } else if (Entry.isConstantInt()) {
-                APInt RawBytes = Entry.getConstantInt()->getValue();
-                if (RawBytes.getBitWidth() > 64)
-                  return false;
-                DwarfExpr.addUnsignedConstant(RawBytes.getZExtValue());
-              } else if (Entry.isTargetIndexLocation()) {
-                TargetIndexLocation Loc = Entry.getTargetIndexLocation();
-                // TODO TargetIndexLocation is a target-independent. Currently
-                // only the WebAssembly-specific encoding is supported.
-                assert(Asm->TM.getTargetTriple().isWasm());
-                DwarfExpr.addWasmLocation(Loc.Index,
-                                          static_cast<uint64_t>(Loc.Offset));
-              } else {
-                llvm_unreachable("Unsupported Entry type.");
-              }
-              return true;
-            };
-
-            if (!DwarfExpr.addExpression(
-                    std::move(Cursor),
-                    [&](unsigned Idx, DIExpressionCursor &Cursor) -> bool {
-                      return AddEntry(DVal->getLocEntries()[Idx], Cursor);
-                    }))
-              return;
-
-            // Now attach the location information to the DIE.
-            addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
-            if (DwarfExpr.TagOffset)
-              addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
-                      dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
-          },
-          [=](const Loc::Multi &Multi) {
-            addLocationList(*VariableDie, dwarf::DW_AT_location,
-                            Multi.getDebugLocListIndex());
-            auto TagOffset = Multi.getDebugLocListTagOffset();
-            if (TagOffset)
-              addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
-                      dwarf::DW_FORM_data1, *TagOffset);
-          },
-          [=](const Loc::MMI &MMI) {
-            std::optional<unsigned> NVPTXAddressSpace;
-            DIELoc *Loc = new (DIEValueAllocator) DIELoc;
-            DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
-            for (const auto &Fragment : MMI.getFrameIndexExprs()) {
-              Register FrameReg;
-              const DIExpression *Expr = Fragment.Expr;
-              const TargetFrameLowering *TFI =
-                  Asm->MF->getSubtarget().getFrameLowering();
-              StackOffset Offset =
-                  TFI->getFrameIndexReference(*Asm->MF, Fragment.FI, FrameReg);
-              DwarfExpr.addFragmentOffset(Expr);
-
-              auto *TRI = Asm->MF->getSubtarget().getRegisterInfo();
-              SmallVector<uint64_t, 8> Ops;
-              TRI->getOffsetOpcodes(Offset, Ops);
-
-              // According to
-              // https://docs.nvidia.com/cuda/archive/10.0/ptx-writers-guide-to-interoperability/index.html#cuda-specific-dwarf
-              // cuda-gdb requires DW_AT_address_class for all variables to be
-              // able to correctly interpret address space of the variable
-              // address. Decode DW_OP_constu <DWARF Address Space> DW_OP_swap
-              // DW_OP_xderef sequence for the NVPTX + gdb target.
-              unsigned LocalNVPTXAddressSpace;
-              if (Asm->TM.getTargetTriple().isNVPTX() && DD->tuneForGDB()) {
-                const DIExpression *NewExpr = DIExpression::extractAddressClass(
-                    Expr, LocalNVPTXAddressSpace);
-                if (NewExpr != Expr) {
-                  Expr = NewExpr;
-                  NVPTXAddressSpace = LocalNVPTXAddressSpace;
-                }
-              }
-              if (Expr)
-                Ops.append(Expr->elements_begin(), Expr->elements_end());
-              DIExpressionCursor Cursor(Ops);
-              DwarfExpr.setMemoryLocationKind();
-              if (const MCSymbol *FrameSymbol = Asm->getFunctionFrameSymbol())
-                addOpAddress(*Loc, FrameSymbol);
-              else
-                DwarfExpr.addMachineRegExpression(
-                    *Asm->MF->getSubtarget().getRegisterInfo(), Cursor,
-                    FrameReg);
-              DwarfExpr.addExpression(std::move(Cursor));
-            }
-            if (Asm->TM.getTargetTriple().isNVPTX() && DD->tuneForGDB()) {
-              // According to
-              // https://docs.nvidia.com/cuda/archive/10.0/ptx-writers-guide-to-interoperability/index.html#cuda-specific-dwarf
-              // cuda-gdb requires DW_AT_address_class for all variables to be
-              // able to correctly interpret address space of the variable
-              // address.
-              const unsigned NVPTX_ADDR_local_space = 6;
-              addUInt(*VariableDie, dwarf::DW_AT_address_class,
-                      dwarf::DW_FORM_data1,
-                      NVPTXAddressSpace.value_or(NVPTX_ADDR_local_space));
-            }
-            addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
-            if (DwarfExpr.TagOffset)
-              addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
-                      dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
-          },
-          [=](const Loc::EntryValue &EntryValue) {
-            DIELoc *Loc = new (DIEValueAllocator) DIELoc;
-            DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
-            // Emit each expression as: EntryValue(Register) <other ops>
-            // <Fragment>.
-            for (auto [Register, Expr] : EntryValue.EntryValues) {
-              DwarfExpr.addFragmentOffset(&Expr);
-              DIExpressionCursor Cursor(Expr.getElements());
-              DwarfExpr.beginEntryValueExpression(Cursor);
-              DwarfExpr.addMachineRegExpression(
-                  *Asm->MF->getSubtarget().getRegisterInfo(), Cursor, Register);
-              DwarfExpr.addExpression(std::move(Cursor));
-            }
-            addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
-          },
-          [](const std::monostate &) {}),
-      DV.asVariant());
+    auto *TRI = Asm->MF->getSubtarget().getRegisterInfo();
+    SmallVector<uint64_t, 8> Ops;
+    TRI->getOffsetOpcodes(Offset, Ops);
 
-  return VariableDie;
+    // According to
+    // https://docs.nvidia.com/cuda/archive/10.0/ptx-writers-guide-to-interoperability/index.html#cuda-specific-dwarf
+    // cuda-gdb requires DW_AT_address_class for all variables to be
+    // able to correctly interpret address space of the variable
+    // address. Decode DW_OP_constu <DWARF Address Space> DW_OP_swap
+    // DW_OP_xderef sequence for the NVPTX + gdb target.
+    unsigned LocalNVPTXAddressSpace;
+    if (Asm->TM.getTargetTriple().isNVPTX() && DD->tuneForGDB()) {
+      const DIExpression *NewExpr =
+          DIExpression::extractAddressClass(Expr, LocalNVPTXAddressSpace);
+      if (NewExpr != Expr) {
+        Expr = NewExpr;
+        NVPTXAddressSpace = LocalNVPTXAddressSpace;
+      }
+    }
+    if (Expr)
+      Ops.append(Expr->elements_begin(), Expr->elements_end());
+    DIExpressionCursor Cursor(Ops);
+    DwarfExpr.setMemoryLocationKind();
+    if (const MCSymbol *FrameSymbol = Asm->getFunctionFrameSymbol())
+      addOpAddress(*Loc, FrameSymbol);
+    else
+      DwarfExpr.addMachineRegExpression(
+          *Asm->MF->getSubtarget().getRegisterInfo(), Cursor, FrameReg);
+    DwarfExpr.addExpression(std::move(Cursor));
+  }
+  if (Asm->TM.getTargetTriple().isNVPTX() && DD->tuneForGDB()) {
+    // According to
+    // https://docs.nvidia.com/cuda/archive/10.0/ptx-writers-guide-to-interoperability/index.html#cuda-specific-dwarf
+    // cuda-gdb requires DW_AT_address_class for all variables to be
+    // able to correctly interpret address space of the variable
+    // address.
+    const unsigned NVPTX_ADDR_local_space = 6;
+    addUInt(*VariableDie, dwarf::DW_AT_address_class, dwarf::DW_FORM_data1,
+            NVPTXAddressSpace.value_or(NVPTX_ADDR_local_space));
+  }
+  addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+  if (DwarfExpr.TagOffset)
+    addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
+            *DwarfExpr.TagOffset);
+}
+
+void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
+    const Loc::EntryValue &EntryValue, const DbgVariable &DV,
+    DIE *VariableDie) {
+  DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+  DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+  // Emit each expression as: EntryValue(Register) <other ops>
+  // <Fragment>.
+  for (auto [Register, Expr] : EntryValue.EntryValues) {
+    DwarfExpr.addFragmentOffset(&Expr);
+    DIExpressionCursor Cursor(Expr.getElements());
+    DwarfExpr.beginEntryValueExpression(Cursor);
+    DwarfExpr.addMachineRegExpression(
+        *Asm->MF->getSubtarget().getRegisterInfo(), Cursor, Register);
+    DwarfExpr.addExpression(std::move(Cursor));
+  }
+  addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
 }
 
+void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
+    const std::monostate &, const DbgVariable &DV, DIE *VariableDie) {}
+
 DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV,
                                             const LexicalScope &Scope,
                                             DIE *&ObjectPointer) {
@@ -975,6 +956,18 @@ DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV,
   return Var;
 }
 
+DIE *DwarfCompileUnit::constructLabelDIE(DbgLabel &DL,
+                                         const LexicalScope &Scope) {
+  auto LabelDie = DIE::get(DIEValueAllocator, DL.getTag());
+  insertDIE(DL.getLabel(), LabelDie);
+  DL.setDIE(*LabelDie);
+
+  if (Scope.isAbstractScope())
+    applyLabelAttributes(DL, *LabelDie);
+
+  return LabelDie;
+}
+
 /// Return all DIVariables that appear in count: expressions.
 static SmallVector<const DIVariable *, 2> dependencies(DbgVariable *Var) {
   SmallVector<const DIVariable *, 2> Result;
@@ -1423,7 +1416,7 @@ void DwarfCompileUnit::finishEntityDefinition(const DbgEntity *Entity) {
     Label = dyn_cast<const DbgLabel>(Entity);
   } else {
     if (const DbgVariable *Var = dyn_cast<const DbgVariable>(Entity))
-      applyVariableAttributes(*Var, *Die);
+      applyCommonDbgVariableAttributes(*Var, *Die);
     else if ((Label = dyn_cast<const DbgLabel>(Entity)))
       applyLabelAttributes(*Label, *Die);
     else
@@ -1604,8 +1597,8 @@ void DwarfCompileUnit::addLocationList(DIE &Die, dwarf::Attribute Attribute,
   addAttribute(Die, Attribute, Form, DIELocList(Index));
 }
 
-void DwarfCompileUnit::applyVariableAttributes(const DbgVariable &Var,
-                                               DIE &VariableDie) {
+void DwarfCompileUnit::applyCommonDbgVariableAttributes(const DbgVariable &Var,
+                                                        DIE &VariableDie) {
   StringRef Name = Var.getName();
   if (!Name.empty())
     addString(VariableDie, dwarf::DW_AT_name, Name);
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index 948ecc21bc94c2b..f358d26853f74ae 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -96,9 +96,34 @@ class DwarfCompileUnit final : public DwarfUnit {
   const DIFile *LastFile = nullptr;
   unsigned LastFileID;
 
-  /// Construct a DIE for the given DbgVariable without initializing the
-  /// DbgVariable's DIE reference.
-  DIE *constructVariableDIEImpl(const DbgVariable &DV, bool Abstract);
+  /// \anchor applyConcreteDbgVariableAttribute
+  /// \name applyConcreteDbgVariableAttribute
+  /// Overload set which applies attributes to \c VariableDie based on
+  /// the active variant of \c DV, which is passed as the first argument.
+  ///@{
+
+  /// See \ref applyConcreteDbgVariableAttribute
+  void applyConcreteDbgVariableAttributes(const Loc::Single &Single,
+                                          const DbgVariable &DV,
+                                          DIE *VariableDie);
+  /// See \ref applyConcreteDbgVariableAttribute
+  void applyConcreteDbgVariableAttributes(const Loc::Multi &Multi,
+                                          const DbgVariable &DV,
+                                          DIE *VariableDie);
+  /// See \ref applyConcreteDbgVariableAttribute
+  void applyConcreteDbgVariableAttributes(const Loc::MMI &MMI,
+                                          const DbgVariable &DV,
+                                          DIE *VariableDie);
+  /// See \ref applyConcreteDbgVariableAttribute
+  void applyConcreteDbgVariableAttributes(const Loc::EntryValue &EntryValue,
+                                          const DbgVariable &DV,
+                                          DIE *VariableDie);
+  /// See \ref applyConcreteDbgVariableAttribute
+  void applyConcreteDbgVariableAttributes(const std::monostate &,
+                                          const DbgVariable &DV,
+                                          DIE *VariableDie);
+
+  ///@}
 
   bool isDwoUnit() const override;
 
@@ -218,9 +243,11 @@ class DwarfCompileUnit final : public DwarfUnit {
   /// and it's an error, if it hasn't.
   DIE *getLexicalBlockDIE(const DILexicalBlock *LB);
 
-  /// constructVariableDIE - Construct a DIE for the given DbgVariable.
+  /// Construct a DIE for the given DbgVariable.
   DIE *constructVariableDIE(DbgVariable &DV, bool Abstract = false);
 
+  /// Convenience overload which writes the DIE pointer into an out variable
+  /// ObjectPointer in addition to returning it.
   DIE *constructVariableDIE(DbgVariable &DV, const LexicalScope &Scope,
                             DIE *&ObjectPointer);
 
@@ -347,7 +374,11 @@ class DwarfCompileUnit final : public DwarfUnit {
 
   /// Add a Dwarf loclistptr attribute data and value.
   void addLocationList(DIE &Die, dwarf::Attribute Attribute, unsigned Index);
-  void applyVariableAttributes(const DbgVariable &Var, DIE &VariableDie);
+
+  /// Add attributes to \p Var which reflect the common attributes of \p
+  /// VariableDie, namely those which are not dependant on the active variant.
+  void applyCommonDbgVariableAttributes(const DbgVariable &Var,
+                                        DIE &VariableDie);
 
   /// Add a Dwarf expression attribute data and value.
   void addExpr(DIELoc &Die, dwarf::Form Form, const MCExpr *Expr);

>From 7ba8c353c74ec75d6e418ed67c3fbd0d20eaeea6 Mon Sep 17 00:00:00 2001
From: Scott Linder <Scott.Linder at amd.com>
Date: Wed, 20 Sep 2023 20:31:25 +0000
Subject: [PATCH 2/2] Change ptr argument to reference, and reflow comment

---
 .../CodeGen/AsmPrinter/DwarfCompileUnit.cpp   | 45 +++++++++----------
 .../lib/CodeGen/AsmPrinter/DwarfCompileUnit.h | 10 ++---
 2 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 57531e3660c039c..42bf755737506b4 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -750,7 +750,7 @@ DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV, bool Abstract) {
   } else {
     std::visit(
         [&](const auto &V) {
-          applyConcreteDbgVariableAttributes(V, DV, VariableDie);
+          applyConcreteDbgVariableAttributes(V, DV, *VariableDie);
         },
         DV.asVariant());
   }
@@ -758,12 +758,12 @@ DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV, bool Abstract) {
 }
 
 void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
-    const Loc::Single &Single, const DbgVariable &DV, DIE *VariableDie) {
+    const Loc::Single &Single, const DbgVariable &DV, DIE &VariableDie) {
   const DbgValueLoc *DVal = &Single.getValueLoc();
   if (!DVal->isVariadic()) {
     const DbgValueLocEntry *Entry = DVal->getLocEntries().begin();
     if (Entry->isLocation()) {
-      addVariableAddress(DV, *VariableDie, Entry->getLoc());
+      addVariableAddress(DV, VariableDie, Entry->getLoc());
     } else if (Entry->isInt()) {
       auto *Expr = Single.getExpr();
       if (Expr && Expr->getNumElements()) {
@@ -773,23 +773,23 @@ void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
         DwarfExpr.addFragmentOffset(Expr);
         DwarfExpr.addUnsignedConstant(Entry->getInt());
         DwarfExpr.addExpression(Expr);
-        addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+        addBlock(VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
         if (DwarfExpr.TagOffset)
-          addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
+          addUInt(VariableDie, dwarf::DW_AT_LLVM_tag_offset,
                   dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
       } else
-        addConstantValue(*VariableDie, Entry->getInt(), DV.getType());
+        addConstantValue(VariableDie, Entry->getInt(), DV.getType());
     } else if (Entry->isConstantFP()) {
-      addConstantFPValue(*VariableDie, Entry->getConstantFP());
+      addConstantFPValue(VariableDie, Entry->getConstantFP());
     } else if (Entry->isConstantInt()) {
-      addConstantValue(*VariableDie, Entry->getConstantInt(), DV.getType());
+      addConstantValue(VariableDie, Entry->getConstantInt(), DV.getType());
     } else if (Entry->isTargetIndexLocation()) {
       DIELoc *Loc = new (DIEValueAllocator) DIELoc;
       DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
       const DIBasicType *BT = dyn_cast<DIBasicType>(
           static_cast<const Metadata *>(DV.getVariable()->getType()));
       DwarfDebug::emitDebugLocValue(*Asm, BT, *DVal, DwarfExpr);
-      addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+      addBlock(VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
     }
     return;
   }
@@ -850,25 +850,25 @@ void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
     return;
 
   // Now attach the location information to the DIE.
-  addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+  addBlock(VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
   if (DwarfExpr.TagOffset)
-    addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
+    addUInt(VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
             *DwarfExpr.TagOffset);
 }
 
 void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
-    const Loc::Multi &Multi, const DbgVariable &DV, DIE *VariableDie) {
-  addLocationList(*VariableDie, dwarf::DW_AT_location,
+    const Loc::Multi &Multi, const DbgVariable &DV, DIE &VariableDie) {
+  addLocationList(VariableDie, dwarf::DW_AT_location,
                   Multi.getDebugLocListIndex());
   auto TagOffset = Multi.getDebugLocListTagOffset();
   if (TagOffset)
-    addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
+    addUInt(VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
             *TagOffset);
 }
 
 void DwarfCompileUnit::applyConcreteDbgVariableAttributes(const Loc::MMI &MMI,
                                                           const DbgVariable &DV,
-                                                          DIE *VariableDie) {
+                                                          DIE &VariableDie) {
   std::optional<unsigned> NVPTXAddressSpace;
   DIELoc *Loc = new (DIEValueAllocator) DIELoc;
   DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
@@ -917,22 +917,21 @@ void DwarfCompileUnit::applyConcreteDbgVariableAttributes(const Loc::MMI &MMI,
     // able to correctly interpret address space of the variable
     // address.
     const unsigned NVPTX_ADDR_local_space = 6;
-    addUInt(*VariableDie, dwarf::DW_AT_address_class, dwarf::DW_FORM_data1,
+    addUInt(VariableDie, dwarf::DW_AT_address_class, dwarf::DW_FORM_data1,
             NVPTXAddressSpace.value_or(NVPTX_ADDR_local_space));
   }
-  addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+  addBlock(VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
   if (DwarfExpr.TagOffset)
-    addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
+    addUInt(VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
             *DwarfExpr.TagOffset);
 }
 
 void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
     const Loc::EntryValue &EntryValue, const DbgVariable &DV,
-    DIE *VariableDie) {
+    DIE &VariableDie) {
   DIELoc *Loc = new (DIEValueAllocator) DIELoc;
   DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
-  // Emit each expression as: EntryValue(Register) <other ops>
-  // <Fragment>.
+  // Emit each expression as: EntryValue(Register) <other ops> <Fragment>.
   for (auto [Register, Expr] : EntryValue.EntryValues) {
     DwarfExpr.addFragmentOffset(&Expr);
     DIExpressionCursor Cursor(Expr.getElements());
@@ -941,11 +940,11 @@ void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
         *Asm->MF->getSubtarget().getRegisterInfo(), Cursor, Register);
     DwarfExpr.addExpression(std::move(Cursor));
   }
-  addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+  addBlock(VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
 }
 
 void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
-    const std::monostate &, const DbgVariable &DV, DIE *VariableDie) {}
+    const std::monostate &, const DbgVariable &DV, DIE &VariableDie) {}
 
 DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV,
                                             const LexicalScope &Scope,
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index f358d26853f74ae..453dcc1c90bbf49 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -105,23 +105,23 @@ class DwarfCompileUnit final : public DwarfUnit {
   /// See \ref applyConcreteDbgVariableAttribute
   void applyConcreteDbgVariableAttributes(const Loc::Single &Single,
                                           const DbgVariable &DV,
-                                          DIE *VariableDie);
+                                          DIE &VariableDie);
   /// See \ref applyConcreteDbgVariableAttribute
   void applyConcreteDbgVariableAttributes(const Loc::Multi &Multi,
                                           const DbgVariable &DV,
-                                          DIE *VariableDie);
+                                          DIE &VariableDie);
   /// See \ref applyConcreteDbgVariableAttribute
   void applyConcreteDbgVariableAttributes(const Loc::MMI &MMI,
                                           const DbgVariable &DV,
-                                          DIE *VariableDie);
+                                          DIE &VariableDie);
   /// See \ref applyConcreteDbgVariableAttribute
   void applyConcreteDbgVariableAttributes(const Loc::EntryValue &EntryValue,
                                           const DbgVariable &DV,
-                                          DIE *VariableDie);
+                                          DIE &VariableDie);
   /// See \ref applyConcreteDbgVariableAttribute
   void applyConcreteDbgVariableAttributes(const std::monostate &,
                                           const DbgVariable &DV,
-                                          DIE *VariableDie);
+                                          DIE &VariableDie);
 
   ///@}
 



More information about the llvm-commits mailing list