[DebugInfo] Only create DIEs of concrete functions

[ellishg]

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 #29985

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: Ellis Hoag (ellishg)

Changes

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 #29985

---

Full diff: https://github.com/llvm/llvm-project/pull/90523.diff

7 Files Affected:

• (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (+31-34)
• (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h (+5-7)
• (modified) llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (+4)
• (modified) llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h (+13)
• (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+16-8)
• (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h (+1)
• (added) llvm/test/DebugInfo/Generic/inlined-static-var.ll (+96)

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
+
+@_ZZ11not_removedvE1A = internal global i32 0, align 4, !dbg !0
+@_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)

[ellishg]

I see that #75385 might be somewhat related to this, but it was reverted. I'm not confident that my pr is the best solution, but I do think the test inlined-static-var.ll should pass, and I didn't want the file to get lost in reviews.llvm.org. @dzhidzhoev do you still have plans to reland this work?

[dwblaikie]

maybe CHECK that this has an abstract_origin, to show this is the concrete instance?

[dwblaikie]

some instructions on how to reproduce the IR would be good - just the clang command line or whatever (which optimizations, etc)

[dwblaikie]

could check these have the right abstract_origins too (& I guess, maybe, check they have no TAGs inside them, to demonstrate that the static member variables aren't described in the inlined subroutines?)

[ellishg]

It seems that this was fixed in #159104. Closing.