| From f69f23396d32c95dacf3765bc63af02b23ccff3e Mon Sep 17 00:00:00 2001 |
| From: David Blaikie <dblaikie@gmail.com> |
| Date: Mon, 31 Jan 2022 18:27:39 -0800 |
| Subject: [PATCH] Revert "DebugInfo: Don't put types in type units if they |
| reference internal linkage types" |
| |
| This reverts commit ab4756338c5b2216d52d9152b2f7e65f233c4dac. |
| |
| Breaks some cases, including this: |
| |
| namespace { |
| template <typename> struct a {}; |
| } // namespace |
| class c { |
| c(); |
| }; |
| class b { |
| b(); |
| a<c> ax; |
| }; |
| b::b() {} |
| c::c() {} |
| |
| By producing a reference to a type unit for "c" but not producing the type unit. |
| --- |
| llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 12 +++--------- |
| llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h | 3 --- |
| llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | 22 +++------------------- |
| llvm/test/DebugInfo/X86/tu-to-non-tu.ll | 22 +++++++++++++--------- |
| 4 files changed, 19 insertions(+), 40 deletions(-) |
| |
| diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp |
| index 680b9586228f..609b568f28be 100644 |
| --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp |
| +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp |
| @@ -3367,8 +3367,7 @@ 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() || SeenLocalType)) |
| + if (!TypeUnitsUnderConstruction.empty() && AddrPool.hasBeenUsed()) |
| return; |
| |
| auto Ins = TypeSignatures.insert(std::make_pair(CTy, 0)); |
| @@ -3379,7 +3378,6 @@ 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)); |
| @@ -3423,7 +3421,7 @@ void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU, |
| |
| // Types referencing entries in the address table cannot be placed in type |
| // units. |
| - if (AddrPool.hasBeenUsed() || SeenLocalType) { |
| + if (AddrPool.hasBeenUsed()) { |
| |
| // Remove all the types built while building this type. |
| // This is pessimistic as some of these types might not be dependent on |
| @@ -3451,18 +3449,14 @@ void DwarfDebug::addDwarfTypeUnitType(DwarfCompileUnit &CU, |
| |
| DwarfDebug::NonTypeUnitContext::NonTypeUnitContext(DwarfDebug *DD) |
| : DD(DD), |
| - TypeUnitsUnderConstruction(std::move(DD->TypeUnitsUnderConstruction)), |
| - AddrPoolUsed(DD->AddrPool.hasBeenUsed()), |
| - SeenLocalType(DD->SeenLocalType) { |
| + TypeUnitsUnderConstruction(std::move(DD->TypeUnitsUnderConstruction)), AddrPoolUsed(DD->AddrPool.hasBeenUsed()) { |
| 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 0043000652e8..4e1a1b1e068d 100644 |
| --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h |
| +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h |
| @@ -433,7 +433,6 @@ private: |
| DenseMap<const DIStringType *, unsigned> StringTypeLocMap; |
| |
| AddressPool AddrPool; |
| - bool SeenLocalType = false; |
| |
| /// Accelerator tables. |
| AccelTable<DWARF5AccelTableData> AccelDebugNames; |
| @@ -672,7 +671,6 @@ public: |
| DwarfDebug *DD; |
| decltype(DwarfDebug::TypeUnitsUnderConstruction) TypeUnitsUnderConstruction; |
| bool AddrPoolUsed; |
| - bool SeenLocalType; |
| friend class DwarfDebug; |
| NonTypeUnitContext(DwarfDebug *DD); |
| public: |
| @@ -681,7 +679,6 @@ public: |
| }; |
| |
| 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 132579aedbc9..5a2bd479f277 100644 |
| --- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp |
| +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp |
| @@ -596,8 +596,10 @@ 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 |
| + else { |
| + auto X = DD->enterNonTypeUnitContext(); |
| finishNonUnitTypeDIE(TyDIE, CTy); |
| + } |
| return &TyDIE; |
| } |
| constructTypeDIE(TyDIE, CTy); |
| @@ -1851,23 +1853,5 @@ void DwarfTypeUnit::finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) { |
| addString(D, dwarf::DW_AT_name, Name); |
| if (Name.startswith("_STN") || !Name.contains('<')) |
| addTemplateParams(D, CTy->getTemplateParams()); |
| - // 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 517872d2b92e..4b8ea04bedc5 100644 |
| --- a/llvm/test/DebugInfo/X86/tu-to-non-tu.ll |
| +++ b/llvm/test/DebugInfo/X86/tu-to-non-tu.ll |
| @@ -1,15 +1,17 @@ |
| ; 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 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) |
| +; 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) |
| |
| ; Built from the following source, compiled with this command: |
| ; $ clang++-tot decl.cpp -g -fdebug-types-section -c |
| @@ -118,6 +120,8 @@ |
| ; CHECK-NEXT: DW_AT_declaration (true) |
| ; CHECK-NEXT: DW_AT_signature (0xb1cde890d320f5c2) |
| |
| +; CHECK: DW_TAG_namespace |
| +; CHECK-NOT: {{DW_AT_name|DW_TAG}} |
| ; CHECK: DW_TAG_structure_type |
| ; CHECK-NOT: DW_TAG |
| ; CHECK: DW_AT_name {{.*}}"non_tu" |
| -- |
| 2.35.1.723.g4982287a31-goog |
| |