[llvm] ab47563 - DebugInfo: Don't put types in type units if they reference internal linkage types
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 23 14:17:46 PST 2022
Author: David Blaikie
Date: 2022-01-23T14:07:31-08:00
New Revision: ab4756338c5b2216d52d9152b2f7e65f233c4dac
URL: https://github.com/llvm/llvm-project/commit/ab4756338c5b2216d52d9152b2f7e65f233c4dac
DIFF: https://github.com/llvm/llvm-project/commit/ab4756338c5b2216d52d9152b2f7e65f233c4dac.diff
LOG: DebugInfo: Don't put types in type units if they reference internal linkage types
Doing this causes a declaration of the internal linkage (anonymous
namespace) type to be emitted in the type unit, which would then be
ambiguous as to which internal linkage definition it refers to (since
the name is only valid internally).
It's possible these internal linkage types could be resolved relative to
the unit the TU is referred to from - but that doesn't seem ideal, and
there's no reason to put the type in a type unit since it can only be
defined in one CU anyway (since otherwise it'd be an ODR violation) & so
avoiding the type unit should be a smaller DWARF encoding anyway.
This also addresses an issue with Simplified Template Names where the
template parameter could not be rebuilt from the declaration emitted
into the TU (specifically for an enum non-type template parameter, where
looking up the enumerators is necessary to rebuild the full template
name)
Added:
Modified:
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/test/DebugInfo/X86/tu-to-non-tu.ll
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 609b568f28beb..680b9586228f7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -3367,7 +3367,8 @@ void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU,
// Fast path if we're building some type units and one has already used the
// address pool we know we're going to throw away all this work anyway, so
// don't bother building dependent types.
- if (!TypeUnitsUnderConstruction.empty() && AddrPool.hasBeenUsed())
+ if (!TypeUnitsUnderConstruction.empty() &&
+ (AddrPool.hasBeenUsed() || SeenLocalType))
return;
auto Ins = TypeSignatures.insert(std::make_pair(CTy, 0));
@@ -3378,6 +3379,7 @@ void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU,
bool TopLevelType = TypeUnitsUnderConstruction.empty();
AddrPool.resetUsedFlag();
+ SeenLocalType = false;
auto OwnedUnit = std::make_unique<DwarfTypeUnit>(CU, Asm, this, &InfoHolder,
getDwoLineTable(CU));
@@ -3421,7 +3423,7 @@ void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU,
// Types referencing entries in the address table cannot be placed in type
// units.
- if (AddrPool.hasBeenUsed()) {
+ if (AddrPool.hasBeenUsed() || SeenLocalType) {
// Remove all the types built while building this type.
// This is pessimistic as some of these types might not be dependent on
@@ -3449,14 +3451,18 @@ void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU,
DwarfDebug::NonTypeUnitContext::NonTypeUnitContext(DwarfDebug *DD)
: DD(DD),
- TypeUnitsUnderConstruction(std::move(DD->TypeUnitsUnderConstruction)), AddrPoolUsed(DD->AddrPool.hasBeenUsed()) {
+ TypeUnitsUnderConstruction(std::move(DD->TypeUnitsUnderConstruction)),
+ AddrPoolUsed(DD->AddrPool.hasBeenUsed()),
+ SeenLocalType(DD->SeenLocalType) {
DD->TypeUnitsUnderConstruction.clear();
DD->AddrPool.resetUsedFlag();
+ DD->SeenLocalType = false;
}
DwarfDebug::NonTypeUnitContext::~NonTypeUnitContext() {
DD->TypeUnitsUnderConstruction = std::move(TypeUnitsUnderConstruction);
DD->AddrPool.resetUsedFlag(AddrPoolUsed);
+ DD->SeenLocalType = SeenLocalType;
}
DwarfDebug::NonTypeUnitContext DwarfDebug::enterNonTypeUnitContext() {
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
index 4e1a1b1e068df..0043000652e89 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
@@ -433,6 +433,7 @@ class DwarfDebug : public DebugHandlerBase {
DenseMap<const DIStringType *, unsigned> StringTypeLocMap;
AddressPool AddrPool;
+ bool SeenLocalType = false;
/// Accelerator tables.
AccelTable<DWARF5AccelTableData> AccelDebugNames;
@@ -671,6 +672,7 @@ class DwarfDebug : public DebugHandlerBase {
DwarfDebug *DD;
decltype(DwarfDebug::TypeUnitsUnderConstruction) TypeUnitsUnderConstruction;
bool AddrPoolUsed;
+ bool SeenLocalType;
friend class DwarfDebug;
NonTypeUnitContext(DwarfDebug *DD);
public:
@@ -679,6 +681,7 @@ class DwarfDebug : public DebugHandlerBase {
};
NonTypeUnitContext enterNonTypeUnitContext();
+ void seenLocalType() { SeenLocalType = true; }
/// Add a label so that arange data can be generated for it.
void addArangeLabel(SymbolCU SCU) { ArangeLabels.push_back(SCU); }
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index 6b2eb8f2bf1d9..956ed33b9eaa1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -597,10 +597,8 @@ DIE *DwarfUnit::createTypeDIE(const DIScope *Context, DIE &ContextDIE,
// Skip updating the accelerator tables since this is not the full type.
if (MDString *TypeId = CTy->getRawIdentifier())
DD->addDwarfTypeUnitType(getCU(), TypeId->getString(), TyDIE, CTy);
- else {
- auto X = DD->enterNonTypeUnitContext();
+ else
finishNonUnitTypeDIE(TyDIE, CTy);
- }
return &TyDIE;
}
constructTypeDIE(TyDIE, CTy);
@@ -1842,5 +1840,23 @@ void DwarfTypeUnit::finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) {
StringRef Name = CTy->getName();
if (!Name.empty())
addString(D, dwarf::DW_AT_name, Name);
+ // If the type is in an anonymous namespace, we can't reference it from a TU
+ // (since the type would be CU local and the TU doesn't specify which TU has
+ // the appropriate type definition) - so flag this emission as such and skip
+ // the rest of the emission now since we're going to throw out all this work
+ // and put the outer/referencing type in the CU instead.
+ // FIXME: Probably good to generalize this to a DICompositeType flag populated
+ // by the frontend, then we could use that to have types that can have
+ // decl+def merged by LTO but where the definition still doesn't go in a type
+ // unit because the type has only one definition.
+ for (DIScope *S = CTy->getScope(); S; S = S->getScope()) {
+ if (auto *NS = dyn_cast<DINamespace>(S)) {
+ if (NS->getName().empty()) {
+ DD->seenLocalType();
+ break;
+ }
+ }
+ }
+ auto X = DD->enterNonTypeUnitContext();
getCU().createTypeDIE(CTy);
}
diff --git a/llvm/test/DebugInfo/X86/tu-to-non-tu.ll b/llvm/test/DebugInfo/X86/tu-to-non-tu.ll
index cdd4f52fc7252..6b9e0d9e60999 100644
--- a/llvm/test/DebugInfo/X86/tu-to-non-tu.ll
+++ b/llvm/test/DebugInfo/X86/tu-to-non-tu.ll
@@ -1,28 +1,24 @@
; RUN: llc -filetype=obj -O0 -generate-type-units -mtriple=x86_64-unknown-linux-gnu < %s \
; RUN: | llvm-dwarfdump -debug-info -debug-types - | FileCheck %s
-; Test that a type unit referencing a non-type unit (in this case, it's
-; bordering on an ODR violation - a type with linkage references a type without
-; linkage, so there's no way for the first type to be defined in more than one
-; translation unit, so there's no need for it to be in a type unit - but this
-; is quirky/rare and an easy way to test a broader issue). The type unit should
-; not end up with a whole definition of the referenced type - instead it should
-; have a declaration of the type, while the definition remains in the primary
-; CU.
-; (again, arguably in this instance - since the type is only referenced once, it
-; could go in the TU only - but that requires tracking usage & then deciding
-; where to put types, which isn't worthwhile right now)
+; Test that a type unit referencing a non-type unit produces a declaration of
+; the referent in the referee.
+
+; Also check that an attempt to reference an internal linkage (defined in an anonymous
+; namespace) type from a type unit (could happen with a pimpl idiom, for instance -
+; it does mean the linkage-having type can only be defined in one translation
+; unit anyway) forces the referent to not be placed in a type unit (because the
+; declaration of the internal linkage type would be ambiguous/wouldn't allow a
+; consumer to find the definition with certainty)
; CHECK: Type Unit:
; CHECK: DW_TAG_structure_type
-; CHECK-NEXT: DW_AT_name {{.*}}"bar"
+; CHECK-NEXT: DW_AT_name {{.*}}"t1"
-; CHECK: DW_TAG_namespace
-; CHECK-NOT: {{DW_AT_name|DW_TAG}}
; CHECK: DW_TAG_structure_type
; CHECK-NEXT: DW_AT_declaration
-; CHECK-NEXT: DW_AT_name {{.*}}"foo"
+; CHECK-NEXT: DW_AT_name {{.*}}"t2"
; CHECK: Compile Unit:
@@ -30,32 +26,36 @@
; CHECK-NEXT: DW_AT_declaration
; CHECK-NEXT: DW_AT_signature
-; CHECK: DW_TAG_namespace
-; CHECK-NOT: {{DW_AT_name|DW_TAG}}
; CHECK: DW_TAG_structure_type
-; CHECK-NEXT: DW_AT_name {{.*}}"foo"
+; CHECK-NEXT: DW_AT_name {{.*}}"t2"
; CHECK-NEXT: DW_AT_byte_size
-%struct.bar = type { %"struct.(anonymous namespace)::foo" }
-%"struct.(anonymous namespace)::foo" = type { i8 }
+; CHECK: DW_TAG_structure_type
+; CHECK-NEXT: DW_AT_name {{.*}}"t3"
- at b = global %struct.bar zeroinitializer, align 1, !dbg !0
+; CHECK: DW_TAG_namespace
+; CHECK-NOT: {{DW_TAG|DW_AT}}
+
+; CHECK: DW_TAG_structure_type
+; CHECK-NEXT: DW_AT_name {{.*}}"t4"
!llvm.dbg.cu = !{!2}
!llvm.module.flags = !{!11, !13}
!llvm.ident = !{!12}
-!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
-!1 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 8, type: !6, isLocal: false, isDefinition: true)
-!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 5.0.0 (trunk 294954) (llvm/trunk 294959)", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "tu-to-non-tu.dwo", emissionKind: FullDebug, enums: !4, globals: !5)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 5.0.0 (trunk 294954) (llvm/trunk 294959)", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "tu-to-non-tu.dwo", emissionKind: FullDebug, enums: !4, retainedTypes: !14)
!3 = !DIFile(filename: "tu.cpp", directory: "/usr/local/google/home/blaikie/dev/scratch")
!4 = !{}
-!5 = !{!0}
-!6 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "bar", file: !3, line: 5, size: 8, elements: !7, identifier: "_ZTS3bar")
+!6 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t1", file: !3, line: 5, size: 8, elements: !7, identifier: "_ZTS2t1")
!7 = !{!8}
!8 = !DIDerivedType(tag: DW_TAG_member, name: "f", scope: !6, file: !3, line: 6, baseType: !9, size: 8)
-!9 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo", scope: !10, file: !3, line: 2, size: 8, elements: !4)
+!9 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t2", file: !3, line: 2, size: 8, elements: !4)
!10 = !DINamespace(scope: null)
!11 = !{i32 2, !"Debug Info Version", i32 3}
!12 = !{!"clang version 5.0.0 (trunk 294954) (llvm/trunk 294959)"}
!13 = !{i32 2, !"Dwarf Version", i32 5}
+!14 = !{!6, !15}
+!15 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t3", file: !3, line: 5, size: 8, elements: !16, identifier: "_ZTS2t3")
+!16 = !{!17}
+!17 = !DIDerivedType(tag: DW_TAG_member, name: "f", scope: !15, file: !3, line: 6, baseType: !18, size: 8)
+!18 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t4", scope: !10, file: !3, line: 2, size: 8, elements: !4)
More information about the llvm-commits
mailing list