[llvm] [DebugInfo] Only create DIEs of concrete functions (PR #90523)

Ellis Hoag via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 29 14:53:18 PDT 2024


https://github.com/ellishg created https://github.com/llvm/llvm-project/pull/90523

At the begining of the module we can iterate through the functions to see which SPs should have concrete DIEs. Then when we need to reference a DIE for a SP we can decide if it's ok to create a concrete DIE or not.

This was originally https://reviews.llvm.org/D113736 and was abandoned in favor of https://reviews.llvm.org/D113741. However, the result of that work did not handle the `inlined-static-var.ll` case.

Fixes https://github.com/llvm/llvm-project/issues/29985

>From 7d09a4234035790cd60cc2c4f7d1442a7fe5b8b5 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellis.sparky.hoag at gmail.com>
Date: Mon, 29 Apr 2024 14:47:45 -0700
Subject: [PATCH] [DebugInfo] Only create DIEs of concrete functions

At the begining of the module we can iterate through the functions to see which SPs should have concrete DIEs. Then when we need to reference a DIE for a SP we can decide if it's ok to create a concrete DIE or not.

This was originally https://reviews.llvm.org/D113736 and was abandoned in favor of https://reviews.llvm.org/D113741. However, the result of that work did not handle the `inlined-static-var.ll` case.

Fixes https://github.com/llvm/llvm-project/issues/29985
---
 .../CodeGen/AsmPrinter/DwarfCompileUnit.cpp   | 65 ++++++-------
 .../lib/CodeGen/AsmPrinter/DwarfCompileUnit.h | 12 +--
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp    |  4 +
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h      | 13 +++
 llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp     | 24 +++--
 llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h       |  1 +
 .../DebugInfo/Generic/inlined-static-var.ll   | 96 +++++++++++++++++++
 7 files changed, 166 insertions(+), 49 deletions(-)
 create mode 100644 llvm/test/DebugInfo/Generic/inlined-static-var.ll

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 6022afbae574ad..aa8dfc10b63cda 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -1167,44 +1167,41 @@ DIE *DwarfCompileUnit::createAndAddScopeChildren(LexicalScope *Scope,
 void DwarfCompileUnit::constructAbstractSubprogramScopeDIE(
     LexicalScope *Scope) {
   auto *SP = cast<DISubprogram>(Scope->getScopeNode());
-  if (getAbstractScopeDIEs().count(SP))
+  DIE *&AbsDef = getAbstractScopeDIEs()[SP];
+  if (AbsDef)
     return;
 
-  DIE *ContextDIE;
-  DwarfCompileUnit *ContextCU = this;
-
-  if (includeMinimalInlineScopes())
-    ContextDIE = &getUnitDie();
-  // Some of this is duplicated from DwarfUnit::getOrCreateSubprogramDIE, with
-  // the important distinction that the debug node is not associated with the
-  // DIE (since the debug node will be associated with the concrete DIE, if
-  // any). It could be refactored to some common utility function.
-  else if (auto *SPDecl = SP->getDeclaration()) {
-    ContextDIE = &getUnitDie();
-    getOrCreateSubprogramDIE(SPDecl);
-  } else {
-    ContextDIE = getOrCreateContextDIE(SP->getScope());
-    // The scope may be shared with a subprogram that has already been
-    // constructed in another CU, in which case we need to construct this
-    // subprogram in the same CU.
-    ContextCU = DD->lookupCU(ContextDIE->getUnitDie());
-  }
-
-  // Passing null as the associated node because the abstract definition
-  // shouldn't be found by lookup.
-  DIE &AbsDef = ContextCU->createAndAddDIE(dwarf::DW_TAG_subprogram,
-                                           *ContextDIE, nullptr);
-
-  // Store the DIE before creating children.
-  ContextCU->getAbstractScopeDIEs()[SP] = &AbsDef;
-
-  ContextCU->applySubprogramAttributesToDefinition(SP, AbsDef);
-  ContextCU->addSInt(AbsDef, dwarf::DW_AT_inline,
+  DIE *ContextDIE =
+      getOrCreateSubprogramContextDIE(SP, includeMinimalInlineScopes());
+
+  if (auto *SPDecl = SP->getDeclaration())
+    if (!includeMinimalInlineScopes())
+      // Build the decl now to ensure it precedes the definition.
+      getOrCreateSubprogramDIE(SPDecl);
+
+  // The scope may be shared with a subprogram that has already been
+  // constructed in another CU, in which case we need to construct this
+  // subprogram in the same CU.
+  auto *ContextCU = (includeMinimalInlineScopes() || SP->getDeclaration())
+                        ? this
+                        : DD->lookupCU(ContextDIE->getUnitDie());
+
+  // The abstract definition can only be looked up from the associated node if
+  // the subprogram does not have a concrete function.
+  if (!ContextCU->DD->isConcrete(SP))
+    AbsDef = ContextCU->getDIE(SP);
+  if (!AbsDef)
+    AbsDef = &ContextCU->createAndAddDIE(dwarf::DW_TAG_subprogram, *ContextDIE,
+                                         nullptr);
+
+  ContextCU->applySubprogramAttributesToDefinition(SP, *AbsDef);
+  ContextCU->addSInt(*AbsDef, dwarf::DW_AT_inline,
                      DD->getDwarfVersion() <= 4 ? std::optional<dwarf::Form>()
                                                 : dwarf::DW_FORM_implicit_const,
                      dwarf::DW_INL_inlined);
-  if (DIE *ObjectPointer = ContextCU->createAndAddScopeChildren(Scope, AbsDef))
-    ContextCU->addDIEEntry(AbsDef, dwarf::DW_AT_object_pointer, *ObjectPointer);
+  if (DIE *ObjectPointer = ContextCU->createAndAddScopeChildren(Scope, *AbsDef))
+    ContextCU->addDIEEntry(*AbsDef, dwarf::DW_AT_object_pointer,
+                           *ObjectPointer);
 }
 
 bool DwarfCompileUnit::useGNUAnalogForDwarf5Feature() const {
@@ -1416,7 +1413,7 @@ DIE *DwarfCompileUnit::getOrCreateImportedEntityDIE(
 void DwarfCompileUnit::finishSubprogramDefinition(const DISubprogram *SP) {
   DIE *D = getDIE(SP);
   if (DIE *AbsSPDIE = getAbstractScopeDIEs().lookup(SP)) {
-    if (D)
+    if (D && D != AbsSPDIE)
       // If this subprogram has an abstract definition, reference that
       addDIEEntry(*D, dwarf::DW_AT_abstract_origin, *AbsSPDIE);
   } else {
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index dc772bb459c956..63cb80a58fa940 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -121,12 +121,6 @@ class DwarfCompileUnit final : public DwarfUnit {
 
   bool isDwoUnit() const override;
 
-  DenseMap<const DILocalScope *, DIE *> &getAbstractScopeDIEs() {
-    if (isDwoUnit() && !DD->shareAcrossDWOCUs())
-      return AbstractLocalScopeDIEs;
-    return DU->getAbstractScopeDIEs();
-  }
-
   DenseMap<const DINode *, std::unique_ptr<DbgEntity>> &getAbstractEntities() {
     if (isDwoUnit() && !DD->shareAcrossDWOCUs())
       return AbstractEntities;
@@ -143,7 +137,11 @@ class DwarfCompileUnit final : public DwarfUnit {
   DwarfCompileUnit(unsigned UID, const DICompileUnit *Node, AsmPrinter *A,
                    DwarfDebug *DW, DwarfFile *DWU,
                    UnitKind Kind = UnitKind::Full);
-
+  DenseMap<const DILocalScope *, DIE *> &getAbstractScopeDIEs() {
+    if (isDwoUnit() && !DD->shareAcrossDWOCUs())
+      return AbstractLocalScopeDIEs;
+    return DU->getAbstractScopeDIEs();
+  }
   bool hasRangeLists() const { return HasRangeLists; }
 
   DwarfCompileUnit *getSkeleton() const {
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index c7fa10775ada7e..9803f2ce2f0ce1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -1154,6 +1154,10 @@ void DwarfDebug::beginModule(Module *M) {
   assert(MMI->hasDebugInfo() &&
          "DebugInfoAvailabilty unexpectedly not initialized");
   SingleCU = NumDebugCUs == 1;
+  for (const auto &F : M->functions())
+    if (!F.isDeclaration())
+      if (const auto *SP = F.getSubprogram())
+        ConcreteSPs.insert(SP);
   DenseMap<DIGlobalVariable *, SmallVector<DwarfCompileUnit::GlobalExpr, 1>>
       GVMap;
   for (const GlobalVariable &Global : M->globals()) {
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
index 452485b632c45f..fd477617a3e4e0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
@@ -375,6 +375,11 @@ class DwarfDebug : public DebugHandlerBase {
   /// create DIEs.
   SmallSetVector<const DISubprogram *, 16> ProcessedSPNodes;
 
+  /// The set of subprograms that have concrete functions.
+  /// This does not include subprograms of machine outlined functions because
+  /// they are created after this set is computed.
+  SmallSetVector<const DISubprogram *, 16> ConcreteSPs;
+
   /// Map function-local imported entities to their parent local scope
   /// (either DILexicalBlock or DISubprogram) for a processed function
   /// (including inlined subprograms).
@@ -919,6 +924,14 @@ class DwarfDebug : public DebugHandlerBase {
   /// allocated in the MCContext.
   std::optional<MD5::MD5Result> getMD5AsBytes(const DIFile *File) const;
 
+  /// Returns false for subprograms that do not have concrete functions and thus
+  /// could have only abstract DIEs. Also returns false for machine outlined
+  /// subprograms, but they will never be emitted as abstract DIEs in the first
+  /// place.
+  bool isConcrete(const DISubprogram *SP) const {
+    return ConcreteSPs.count(SP);
+  }
+
   MDNodeSet &getLocalDeclsForScope(const DILocalScope *S) {
     return LocalDeclsPerLS[S];
   }
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index 56c288ee95b431..a3ba442040d650 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1168,24 +1168,32 @@ DIE *DwarfUnit::getOrCreateModule(const DIModule *M) {
   return &MDie;
 }
 
+DIE *DwarfUnit::getOrCreateSubprogramContextDIE(const DISubprogram *SP,
+                                                bool Minimal) {
+  if (Minimal || SP->getDeclaration())
+    return &getUnitDie();
+  return getOrCreateContextDIE(SP->getScope());
+}
+
 DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal) {
   // Construct the context before querying for the existence of the DIE in case
   // such construction creates the DIE (as is the case for member function
   // declarations).
-  DIE *ContextDIE =
-      Minimal ? &getUnitDie() : getOrCreateContextDIE(SP->getScope());
+  DIE *ContextDIE = getOrCreateSubprogramContextDIE(SP, Minimal);
 
   if (DIE *SPDie = getDIE(SP))
     return SPDie;
 
-  if (auto *SPDecl = SP->getDeclaration()) {
-    if (!Minimal) {
-      // Add subprogram definitions to the CU die directly.
-      ContextDIE = &getUnitDie();
+  if (auto *SPDecl = SP->getDeclaration())
+    if (!Minimal)
       // Build the decl now to ensure it precedes the definition.
       getOrCreateSubprogramDIE(SPDecl);
-    }
-  }
+
+  // Try to find the abstract origin if the subprogram is not concrete.
+  if (auto *ContextCU = DD->lookupCU(ContextDIE->getUnitDie()))
+    if (!ContextCU->DD->isConcrete(SP))
+      if (auto *SPDie = ContextCU->getAbstractScopeDIEs().lookup(SP))
+        return SPDie;
 
   // DW_TAG_inlined_subroutine may refer to this DIE.
   DIE &SPDie = createAndAddDIE(dwarf::DW_TAG_subprogram, *ContextDIE, SP);
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
index 18f50f86ec87a1..713de783a4abc7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
@@ -248,6 +248,7 @@ class DwarfUnit : public DIEUnit {
 
   DIE *getOrCreateNameSpace(const DINamespace *NS);
   DIE *getOrCreateModule(const DIModule *M);
+  DIE *getOrCreateSubprogramContextDIE(const DISubprogram *SP, bool Minimal);
   DIE *getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal = false);
 
   void applySubprogramAttributes(const DISubprogram *SP, DIE &SPDie,
diff --git a/llvm/test/DebugInfo/Generic/inlined-static-var.ll b/llvm/test/DebugInfo/Generic/inlined-static-var.ll
new file mode 100644
index 00000000000000..a07965f2a64163
--- /dev/null
+++ b/llvm/test/DebugInfo/Generic/inlined-static-var.ll
@@ -0,0 +1,96 @@
+; RUN: %llc_dwarf -O0 -filetype=obj < %s | llvm-dwarfdump -debug-info - | FileCheck --implicit-check-not "{{DW_TAG|NULL}}" %s
+
+; RUN: %llc_dwarf --try-experimental-debuginfo-iterators -O0 -filetype=obj < %s | llvm-dwarfdump -debug-info - | FileCheck --implicit-check-not "{{DW_TAG|NULL}}" %s
+
+; inline __attribute__((always_inline))
+; int removed() { static int A; return A++; }
+;
+; __attribute__((always_inline))
+; int not_removed() { static int B; return B++; }
+;
+; int foo() { return removed() + not_removed(); }
+
+; Ensure that global variables belong to the correct subprograms even if those
+; subprograms are inlined.
+
+; CHECK: DW_TAG_compile_unit
+; CHECK:   DW_TAG_subprogram
+; TODO: Place the variable in the abstract DIE rather than this concrete DIE.
+; CHECK:     DW_TAG_variable
+; CHECK:       DW_AT_name     ("B")
+; CHECK:     NULL
+; CHECK:   DW_TAG_base_type
+; CHECK:   DW_TAG_subprogram
+; CHECK:     DW_AT_name       ("removed")
+; CHECK:     DW_TAG_variable
+; CHECK:       DW_AT_name     ("A")
+; CHECK:     NULL
+; CHECK:   DW_TAG_subprogram
+; CHECK:     DW_AT_name       ("not_removed")
+; CHECK:   DW_TAG_subprogram
+; CHECK:     DW_AT_name       ("foo")
+; CHECK:     DW_TAG_inlined_subroutine
+; CHECK:     DW_TAG_inlined_subroutine
+; CHECK:     NULL
+; CHECK:   NULL
+
+$_ZZ7removedvE1A = comdat any
+
+ at _ZZ11not_removedvE1A = internal global i32 0, align 4, !dbg !0
+ at _ZZ7removedvE1A = linkonce_odr dso_local global i32 0, comdat, align 4, !dbg !10
+
+define dso_local i32 @_Z11not_removedv() !dbg !2 {
+  %1 = load i32, ptr @_ZZ11not_removedvE1A, align 4, !dbg !24
+  %2 = add nsw i32 %1, 1, !dbg !24
+  store i32 %2, ptr @_ZZ11not_removedvE1A, align 4, !dbg !24
+  ret i32 %1, !dbg !25
+}
+
+define dso_local i32 @_Z3foov() !dbg !26 {
+  %1 = load i32, ptr @_ZZ7removedvE1A, align 4, !dbg !27
+  %2 = add nsw i32 %1, 1, !dbg !27
+  store i32 %2, ptr @_ZZ7removedvE1A, align 4, !dbg !27
+  %3 = load i32, ptr @_ZZ11not_removedvE1A, align 4, !dbg !29
+  %4 = add nsw i32 %3, 1, !dbg !29
+  store i32 %4, ptr @_ZZ11not_removedvE1A, align 4, !dbg !29
+  %5 = add nsw i32 %1, %3, !dbg !31
+  ret i32 %5, !dbg !32
+}
+
+!llvm.dbg.cu = !{!7}
+!llvm.module.flags = !{!14, !15, !16, !17, !18, !19, !20, !21, !22}
+!llvm.ident = !{!23}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "B", scope: !2, file: !3, line: 5, type: !6, isLocal: true, isDefinition: true)
+!2 = distinct !DISubprogram(name: "not_removed", linkageName: "_Z11not_removedv", scope: !3, file: !3, line: 5, type: !4, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !7, retainedNodes: !13)
+!3 = !DIFile(filename: "example.cpp", directory: "")
+!4 = !DISubroutineType(types: !5)
+!5 = !{!6}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !8, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !9, splitDebugInlining: false, nameTableKind: None)
+!8 = !DIFile(filename: "example.cpp", directory: "")
+!9 = !{!0, !10}
+!10 = !DIGlobalVariableExpression(var: !11, expr: !DIExpression())
+!11 = distinct !DIGlobalVariable(name: "A", scope: !12, file: !3, line: 2, type: !6, isLocal: false, isDefinition: true)
+!12 = distinct !DISubprogram(name: "removed", linkageName: "_Z7removedv", scope: !3, file: !3, line: 2, type: !4, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !7, retainedNodes: !13)
+!13 = !{}
+!14 = !{i32 7, !"Dwarf Version", i32 4}
+!15 = !{i32 2, !"Debug Info Version", i32 3}
+!16 = !{i32 1, !"wchar_size", i32 4}
+!17 = !{i32 1, !"branch-target-enforcement", i32 0}
+!18 = !{i32 1, !"sign-return-address", i32 0}
+!19 = !{i32 1, !"sign-return-address-all", i32 0}
+!20 = !{i32 1, !"sign-return-address-with-bkey", i32 0}
+!21 = !{i32 7, !"uwtable", i32 1}
+!22 = !{i32 7, !"frame-pointer", i32 1}
+!23 = !{!"clang version 14.0.0"}
+!24 = !DILocation(line: 5, column: 43, scope: !2)
+!25 = !DILocation(line: 5, column: 35, scope: !2)
+!26 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !3, file: !3, line: 7, type: !4, scopeLine: 7, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !7, retainedNodes: !13)
+!27 = !DILocation(line: 2, column: 39, scope: !12, inlinedAt: !28)
+!28 = distinct !DILocation(line: 7, column: 20, scope: !26)
+!29 = !DILocation(line: 5, column: 43, scope: !2, inlinedAt: !30)
+!30 = distinct !DILocation(line: 7, column: 32, scope: !26)
+!31 = !DILocation(line: 7, column: 30, scope: !26)
+!32 = !DILocation(line: 7, column: 13, scope: !26)



More information about the llvm-commits mailing list