Revert "DebugInfo: Don't put types in type units if they reference internal linkage types"

This reverts commit ab4756338c.

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

@@ -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() {

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); }

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);
 }

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"