[DebugInfo] Only create DIEs of concrete functions #90523
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 llvm#29985
|
@llvm/pr-subscribers-debuginfo Author: Ellis Hoag (ellishg) ChangesAt 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 Fixes #29985 Full diff: https://github.com/llvm/llvm-project/pull/90523.diff 7 Files Affected:
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)
|
|
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 |
dwblaikie
reviewed
May 6, 2024
Comment on lines
+17
to
+18
| ; CHECK: DW_TAG_subprogram | ||
| ; TODO: Place the variable in the abstract DIE rather than this concrete DIE. |
There was a problem hiding this comment.
maybe CHECK that this has an abstract_origin, to show this is the concrete instance?
| ; 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 | ||
|
|
There was a problem hiding this comment.
some instructions on how to reproduce the IR would be good - just the clang command line or whatever (which optimizations, etc)
Comment on lines
+32
to
+33
| ; CHECK: DW_TAG_inlined_subroutine | ||
| ; CHECK: DW_TAG_inlined_subroutine |
There was a problem hiding this comment.
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?)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.llcase.Fixes #29985