Move DwarfCompileUnit::getOrCreateGlobalVariableDIE to DwarfUnit.

[friss]

Summary

I'm not sure this doesn't break some intended layering, but the end
goal is to call it from constructInportedEntityDIE which is in
DwarfUnit (along with the others getOrCreate* helpers) so I suppose
it's OK to move it there. (Another option is to move everything down
into DwarfCompileUnit, but if you want to create some other types of
Units, you might need this functionality in the base class).

BTW, although I think I can imaging where you are going to with the
code shuffling you are doing around the Dwarf(Compile)Unit, could you
explain a bit what the expected end result hierarchy looks like, and
what the responsabilities of each level will be? And maybe update the
file header comments of these 2 classes to reflect the difference.

Thanks!

[dblaikie]

Looks like the right answer for your end goal is instead to sink constructImportedEntityDIE into DwarfCompileUnit - it's only needed for compile units, not type units.

(at some point, we should supported imported entities at class scope - but those are a special case, they can't refer to global variables, only to functions declared in base classes - so we still wouldn't need the ability to emit global variable definitions or declarations in a type unit)

Sorry for the rather terse commit messages on some of my recent refactorings. I'd mentioned in some detail the direction in some of the commit messages, but it seemed verbose to repeat it on every step. The basic idea is that we need to emit -gmlt-like data into the skeleton type unit to support in-process symbolication (including inlining) in tools like ASan even when using fission/-gsplit-dwarf. This means I need much of the subprogram emission functionality to be reusable for both the full and skeleton type unit - currently all that state exists in DwarfDebug, so it can't be reused to do both (eg: the AbstractSPDies map - from MDNode* to DIE*, well if we produce abstract subprograms in both the full and skeleton unit, tehre will be two such DIEs for every MDNode, so the map can't live here) - by sinking these variables down into DwarfFile, there will be one for each file and we'll be able to produce subprograms and inlining in both the full and skeleton DIEs.

Unfortunately, variables are rather inseparable from function emission, so this is sinking too, even though -gmlt doesn't need/have any variables. It seems like better layering anyway.

Sinking operations into DwarfCompileUnit (in theory I could've just sunk all the functions and their variables only as far as DwarfFile) just seems better for layering - they're compile unit-y kind of operations. It also makes for smaller files - DwarfDebug is pretty big, so it's nice to be able to pull out a bunch of its logic into the one place that should really be handling that.

Hence the suggestion to pull constructImportedEntityDIE down into DwarfCompileUnit - we would never emit a global variable into a type unit (type units can't reference addresses, so it wouldn't make sense to do that) so it's nice to have that enforced/supported by the API. Indeed when I made this layering change I found and fixed at least one bug, if I recall correctly... though it is a bit hazy now.

[friss]

Changing this into "Sink DwarfUnit::constructImportedEntityDIE into DwarfCompileUnit."

The revision title/description isn't accurate anymore, but the commit log will be.

[dblaikie]

Looks good (would've been totally fine for you to commit this without an extra review round-trip, sorry - I could've been more clear about that when I suggested the idea)

[friss]

Closed by commit rL220594 (authored by @friss).

lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

public:
 
   /// \brief Construct a DIE for this subprogram scope.
   void constructSubprogramScopeDIE(LexicalScope *Scope);
 
   DIE *createAndAddScopeChildren(LexicalScope *Scope, DIE &ScopeDIE);
 
   DIE &constructAbstractSubprogramScopeDIE(LexicalScope *Scope);
 
+  /// \brief Construct import_module DIE.
+  std::unique_ptr<DIE>
+  constructImportedEntityDIE(const DIImportedEntity &Module);
+
   void finishSubprogramDefinition(DISubprogram SP);
 };
 
 } // end llvm namespace
 
 #endif

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

DwarfCompileUnit::constructAbstractSubprogramScopeDIE(LexicalScope *Scope) {
 
   if (getCUNode().getEmissionKind() != DIBuilder::LineTablesOnly)
     addUInt(AbsDef, dwarf::DW_AT_inline, None, dwarf::DW_INL_inlined);
   if (DIE *ObjectPointer = createAndAddScopeChildren(Scope, AbsDef))
     addDIEEntry(AbsDef, dwarf::DW_AT_object_pointer, *ObjectPointer);
   return AbsDef;
 }
 
+std::unique_ptr<DIE>
+DwarfCompileUnit::constructImportedEntityDIE(const DIImportedEntity &Module) {
+  assert(Module.Verify() &&
+         "Use one of the MDNode * overloads to handle invalid metadata");
+  std::unique_ptr<DIE> IMDie = make_unique<DIE>((dwarf::Tag)Module.getTag());
+  insertDIE(Module, IMDie.get());
+  DIE *EntityDie;
+  DIDescriptor Entity = resolve(Module.getEntity());
+  if (Entity.isNameSpace())
+    EntityDie = getOrCreateNameSpace(DINameSpace(Entity));
+  else if (Entity.isSubprogram())
+    EntityDie = getOrCreateSubprogramDIE(DISubprogram(Entity));
+  else if (Entity.isType())
+    EntityDie = getOrCreateTypeDIE(DIType(Entity));
+  else
+    EntityDie = getDIE(Entity);
+  assert(EntityDie);
+  addSourceLine(*IMDie, Module.getLineNumber(),
+                Module.getContext().getFilename(),
+                Module.getContext().getDirectory());
+  addDIEEntry(*IMDie, dwarf::DW_AT_import, *EntityDie);
+  StringRef Name = Module.getName();
+  if (!Name.empty())
+    addString(*IMDie, dwarf::DW_AT_name, Name);
+
+  return IMDie;
+}
+
 void DwarfCompileUnit::finishSubprogramDefinition(DISubprogram SP) {
   DIE *D = getDIE(SP);
   if (DIE *AbsSPDIE = DD->getAbstractSPDies().lookup(SP)) {
     if (D)
       // If this subprogram has an abstract definition, reference that
       addDIEEntry(*D, dwarf::DW_AT_abstract_origin, *AbsSPDIE);
   } else {
     if (!D && getCUNode().getEmissionKind() != DIBuilder::LineTablesOnly)

lib/CodeGen/AsmPrinter/DwarfUnit.h

public:
 
   /// constructContainingTypeDIEs - Construct DIEs for types that contain
   /// vtables.
   void constructContainingTypeDIEs();
 
   /// constructSubprogramArguments - Construct function argument DIEs.
   void constructSubprogramArguments(DIE &Buffer, DITypeArray Args);
 
-  /// \brief Construct import_module DIE.
-  std::unique_ptr<DIE>
-  constructImportedEntityDIE(const DIImportedEntity &Module);
-
   /// Create a DIE with the given Tag, add the DIE to its parent, and
   /// call insertDIE if MD is not null.
   DIE &createAndAddDIE(unsigned Tag, DIE &Parent,
                        DIDescriptor N = DIDescriptor());
 
   /// Compute the size of a header for this unit, not including the initial
   /// length field.
   virtual unsigned getHeaderSize() const {

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

if (!Ty) {
       DIE &Arg = createAndAddDIE(dwarf::DW_TAG_formal_parameter, Buffer);
       addType(Arg, DIType(Ty));
       if (DIType(Ty).isArtificial())
         addFlag(Arg, dwarf::DW_AT_artificial);
     }
   }
 }
 
-std::unique_ptr<DIE>
-DwarfUnit::constructImportedEntityDIE(const DIImportedEntity &Module) {
-  assert(Module.Verify() &&
-         "Use one of the MDNode * overloads to handle invalid metadata");
-  std::unique_ptr<DIE> IMDie = make_unique<DIE>((dwarf::Tag)Module.getTag());
-  insertDIE(Module, IMDie.get());
-  DIE *EntityDie;
-  DIDescriptor Entity = resolve(Module.getEntity());
-  if (Entity.isNameSpace())
-    EntityDie = getOrCreateNameSpace(DINameSpace(Entity));
-  else if (Entity.isSubprogram())
-    EntityDie = getOrCreateSubprogramDIE(DISubprogram(Entity));
-  else if (Entity.isType())
-    EntityDie = getOrCreateTypeDIE(DIType(Entity));
-  else
-    EntityDie = getDIE(Entity);
-  assert(EntityDie);
-  addSourceLine(*IMDie, Module.getLineNumber(),
-                Module.getContext().getFilename(),
-                Module.getContext().getDirectory());
-  addDIEEntry(*IMDie, dwarf::DW_AT_import, *EntityDie);
-  StringRef Name = Module.getName();
-  if (!Name.empty())
-    addString(*IMDie, dwarf::DW_AT_name, Name);
-
-  return IMDie;
-}
-
 /// constructTypeDIE - Construct type DIE from DICompositeType.
 void DwarfUnit::constructTypeDIE(DIE &Buffer, DICompositeType CTy) {
   // Add name if not anonymous or intermediate type.
   StringRef Name = CTy.getName();
 
   uint64_t Size = CTy.getSizeInBits() >> 3;
   uint16_t Tag = Buffer.getTag();