[ThinLTO] Link in only necessary DICompileUnit operands

[tejohnson]

Summary

As discussed on IRC, for function importing we don't need to link in
the type, enums and many other metadata hanging off the DICompileUnit
for the source module. With this patch, we will only map in the needed
DISubprograms and the DIFile (which is required) and a few simple
non-metadata members.

Do we also need to link in the DIImportedEntity though? We are marking any
DISubprograms reached from them as needed (findNeededSubprograms()).

[dblaikie]

Should the tests be testing that, when you import the DISubprogram, that you get any types needed by that DISubprogram (& only those types)?

test/Transforms/FunctionImport/funcimport_debug.ll

17

Does this test the right thing? (if you revert your change and run this test, does it fail?)

I would've expected the following DICompileUnit to match, and there to be no DICompileUnit match between one and the next. Whereas I would've guessed this was trying to check that there were no enums in the DICompileUNit checked on the next line?

[tejohnson]

In D16440#333561, @dblaikie wrote:

Should the tests be testing that, when you import the DISubprogram, that you get any types needed by that DISubprogram (& only those types)?

Good idea, I will add a new test that has some types.

test/Transforms/FunctionImport/funcimport_debug.ll

17

Good catch - it doesn't work. I modified the test to ensure that there is no enums here by specifying the expected preceeding operand (emissionKind), here and in the other test. Confirmed that backing out my change causes this to fail.

Is there a better way to do this?

[tejohnson]

• Fix test to correctly ensure that enums does not exist in the imported DICompileUnit.

[aprantl]

lib/Linker/IRMover.cpp

1566

We also need to preserve the splitDebugName. When module debugging is enabled there will be several additional skeleton compile units without any children in the module that point to the (clang) modules that contain shared debug type information which may be referenced by types that hang off the imported function.

I also don't see a reason to drop the string fields like flags and producer.

[aprantl]

lib/Linker/IRMover.cpp

1564

Why not use a DIBuilder for this?

[tejohnson]

lib/Linker/IRMover.cpp

1564

I looked at DIBuilder last night and it wasn't clear to me how to use that in this context where I need to call MapMetadata to map in the transitively referenced MD from source to dest.

1566

Ok, will map those MDString fields in as well.

[tejohnson]

• Map in MDStrings (producer, flags, splitDebugFilename)

[dblaikie]

test/Transforms/FunctionImport/funcimport_debug.ll

16–17

Nothing great I can think of, unfortunately.

[mehdi_amini]

I still hit an assert with this patch:

Assertion failed: (I != Map.end() && "Missing identifier in type map"), function resolve, file include/llvm/IR/DebugInfoMetadata.h, line 89.

Investigating...

[mehdi_amini]

So I debugged the assertion I see and the issue seems that we don't import anymore a retained type.

DwarfDebug::beginModule() calls TypeIdentifierMap = generateDITypeIdentifierMap(CU_Nodes); to build (from the retained types list) a map from type name to matching DICompositeType.

When emitting a DISubprogram, getOrCreateSubprogramDIE will call resolve(SP->getScope()) which query the map.
It seems that any scope for a DISubprogram needs to be in the retained types list, is it correct?

[tejohnson]

In D16440#334379, @joker.eph wrote:

So I debugged the assertion I see and the issue seems that we don't import anymore a retained type.

DwarfDebug::beginModule() calls TypeIdentifierMap = generateDITypeIdentifierMap(CU_Nodes); to build (from the retained types list) a map from type name to matching DICompositeType.

When emitting a DISubprogram, getOrCreateSubprogramDIE will call resolve(SP->getScope()) which query the map.
It seems that any scope for a DISubprogram needs to be in the retained types list, is it correct?

Is it the case that you can reach the SP and composite types from the retained type, but not vice versa? It sounds like it if it is building a map from retained type. In that case my change would not map those in (since the retained type list on the compile unit is no longer mapped, only things we can reach from the imported functions). I'll have to change the patch to build up the same type of map and then map in any retained types that map to something that was mapped in.

Anyone know offhand if there is anything else hanging off the compile unit that is like this?

[tejohnson]

In D16440#334416, @tejohnson wrote:

In D16440#334379, @joker.eph wrote:

So I debugged the assertion I see and the issue seems that we don't import anymore a retained type.

DwarfDebug::beginModule() calls TypeIdentifierMap = generateDITypeIdentifierMap(CU_Nodes); to build (from the retained types list) a map from type name to matching DICompositeType.

When emitting a DISubprogram, getOrCreateSubprogramDIE will call resolve(SP->getScope()) which query the map.
It seems that any scope for a DISubprogram needs to be in the retained types list, is it correct?

Is it the case that you can reach the SP and composite types from the retained type, but not vice versa? It sounds like it if it is building a map from retained type. In that case my change would not map those in (since the retained type list on the compile unit is no longer mapped, only things we can reach from the imported functions). I'll have to change the patch to build up the same type of map and then map in any retained types that map to something that was mapped in.

Anyone know offhand if there is anything else hanging off the compile unit that is like this?

I reproduced this issue with a build of xalancbmk from SPEC cpu2006. The issue happens when composite types (on the retained types list) are used as the scope on a DISubprogram, because the association is done by MDString identifier. E.g.:

!78 = !DICompositeType(tag: DW_TAG_class_type, name: "XMLPlatformUtils", scope: !17, file: !16, line: 104, size: 8, align: 8, elements: !79, identifier: "_ZTSN11xercesc_2_516XMLPlatformUtilsE")
...
!93 = !DISubprogram(name: "Initialize", linkageName: "_ZN11xercesc_2_516XMLPlatformUtils10InitializeEPKcS2_PNS_12PanicHandlerEPNS_13MemoryManagerE", scope: !"_ZTSN11xercesc_2_516XMLPlatformUtilsE", file: !16, line: 204, type: !94, isLocal: false, isDefinition: false, scopeLine: 204, flags: DIFlagPublic | DIFlagPrototyped, isOptimized: true)

Why aren't these correlated by the metadata ID instead? I.e. !93 instead of !"_ZTSN11xercesc_2_516XMLPlatformUtilsE" in the DISubprogram scope? Then no TypeIdentifierMap would be needed.

In any case, I added some handling for this, so that we detect and map in retained types onto the new DICompileUnit if they correspond to a DISubprogram that is being mapped in. However, I just hit another similar issue. In this case, the retained type DICompositeType identifier is the base type of a DIDerivedType needed by a DISubprogram that was mapped in. E.g.:

!3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "PanicReasons", scope: !"_ZTSN11xercesc_2_512PanicHandlerE", file: !4, line: 116, size: 32, align: 32, elements: !5, identifier: "_ZTSN11xercesc_2_512PanicHandler12PanicReasonsE")
...
!107 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !"_ZTSN11xercesc_2_512PanicHandler12PanicReasonsE")

where the DIDerivedType is the DISubroutineType for a DISubprogram that was mapped in. I'll need to extend my handling a bit to get this case. Same question here though, why doesn't it just use "baseType: !3"?

[tejohnson]

!78 = !DICompositeType(tag: DW_TAG_class_type, name: "XMLPlatformUtils", scope: !17, file: !16, line: 104, size: 8, align: 8, elements: !79, identifier: "_ZTSN11xercesc_2_516XMLPlatformUtilsE")
...
!93 = !DISubprogram(name: "Initialize", linkageName: "_ZN11xercesc_2_516XMLPlatformUtils10InitializeEPKcS2_PNS_12PanicHandlerEPNS_13MemoryManagerE", scope: !"_ZTSN11xercesc_2_516XMLPlatformUtilsE", file: !16, line: 204, type: !94, isLocal: false, isDefinition: false, scopeLine: 204, flags: DIFlagPublic | DIFlagPrototyped, isOptimized: true)

Correction, the DISubprogram that references the composite type (and was pulled in due to an import) is:
!103 = distinct !DISubprogram(name: "alignPointerForNewBlockAllocation", linkageName: "_ZN11xercesc_2_516XMLPlatformUtils33alignPointerForNewBlockAllocationEm", scope: !"_ZTSN11xercesc_2_516XMLPlatformUtilsE", file: !104, line: 851, type: !105, isLocal: false, isDefinition: true, scopeLine: 852, flags: DIFlagPrototyped, isOptimized: true, declaration: !107, variables: !108)

Why aren't these correlated by the metadata ID instead? I.e. !93 instead of !"_ZTSN11xercesc_2_516XMLPlatformUtilsE" in the DISubprogram scope? Then no TypeIdentifierMap would be needed.

In any case, I added some handling for this, so that we detect and map in retained types onto the new DICompileUnit if they correspond to a DISubprogram that is being mapped in. However, I just hit another similar issue. In this case, the retained type DICompositeType identifier is the base type of a DIDerivedType needed by a DISubprogram that was mapped in. E.g.:

!3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "PanicReasons", scope: !"_ZTSN11xercesc_2_512PanicHandlerE", file: !4, line: 116, size: 32, align: 32, elements: !5, identifier: "_ZTSN11xercesc_2_512PanicHandler12PanicReasonsE")
...
!107 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !"_ZTSN11xercesc_2_512PanicHandler12PanicReasonsE")

where the DIDerivedType is the DISubroutineType for a DISubprogram that was mapped in. I'll need to extend my handling a bit to get this case. Same question here though, why doesn't it just use "baseType: !3"?

It turns out the second problem was caused by the fix for the first. The subprogram that referenced the DIDerivedType !107 was only mapped in because it is in the list of elements for the DICompositeType !78 that was the scope for the function needed due to importing. So the set of what debug metadata needs to be imported keeps exploding as the problem is addressed.

Is there any way to avoid this? Does the complete DICompositeType !78 need to be recreated in the importing module? It isn't clear to me which debug type metadata needs to be fully recreated in the importing module, I know the original premise behind this patch was that it was sufficient for it to exist in the original module. But getOrCreateSubprogramDIE wants to get the ContextDIE for the subprogram.

[mehdi_amini]

It seems that

Why aren't these correlated by the metadata ID instead? I.e. !93 instead of !"_ZTSN11xercesc_2_516XMLPlatformUtilsE" in the DISubprogram scope? Then no TypeIdentifierMap would be needed.

I think it is to break cycles. String are a convenient way to break dependencies :)

I discussed with Duncan yesterday, the ultimate plan he had last year seemed to be to reverse all the links: Subprograms would point to the CompileUnit instead of the opposite for instance. And ultimately there wouldn't be a need for the named metadata dbg.cu.

[tejohnson]

Ensure that all needed retained types are mapped in, and as declarations.

The retained composite types needed in the dest module include not only
those reached by mapped subprograms, but also those in the scope-chain
of other needed composite types. Because the full composite type will
be emitted in the original module, only the type declaration is needed
in the dest module. Therefore, a new composite type is constructed for
the dest module, and only fields required for a declaration are
populated, and the FlagFwdDecl flag is set.

Different support is needed to detect the set of retained types
referenced by imported functions for normal metadata linking vs
postpass metadata linking. The former can simply look at which
retained types were added to the value map, whereas the latter needs to
recursively locate the retained types reached from needed subprograms
(which have not yet been mapped). This is similar to how we recursively
identify needed subprograms in the post-pass linking case.

The test cases were augmented to contain composite types referenced
in various ways, and check that the expected ones are imported as
type declarations.

Also tested by building 483.xalancbmk from SPEC with and without
postpass metadata linking. In both cases the size of the binary
was reduced around 28% with this patch.

[dblaikie]

It's a little unclear from your description - are you importing /any/ types as definitions? Or are all imported types being imported as declarations? (I think the latter would be good/correct/minimal/simpler)

test/Transforms/FunctionImport/funcimport_debug.ll

18

It might be clearer to match the retained types list (assuming all the types go in there, which I think they do - at least the ones using mangled names, etc) and then match all the types referenced from there, then you wouldn't need the CHECK-NOTs which are a bit brittle here anyway (if types are emitted in a different order, the CHECK-NOTs wouldn't catch them - they'd only catch the case where the type is emitted between the two surrounding CHECK'd types)

Just a thought. (& could check that things like vtableHolder, etc, aren't emitted in the declarations you're interested in)

[tejohnson]

In D16440#338695, @dblaikie wrote:

It's a little unclear from your description - are you importing /any/ types as definitions? Or are all imported types being imported as declarations? (I think the latter would be good/correct/minimal/simpler)

I think so. At least for composite types. Are there any cases, other than from the retainedTypes list on the compile unit, where a DICompositeType node is referenced by its address rather than by UUID? I haven't seen any, which means that we would not import any while mapping in metadata on imported functions/instructions. In that case they are only imported as declarations with this patch, since they are only imported by the new linkImportedCompileUnit routine.

Note that there are DIDerivedTypes that are referenced from imported functions (e.g. via the subprogram type array) and therefore mapped in when the functions and instructions are imported and mapped. Is there a distinction between definition and declaration for these types?

test/Transforms/FunctionImport/funcimport_debug.ll

18

Good idea. I already found and fixed some issues where my existing CHECK-NOTs were not behaving as expected. Will do that.

[tejohnson]

Explicitely cross-check retainedTypes list against DICompositeTypes
instead of CHECK-NOTs. Also, check full DICompositeType to ensure only
expected fields are present.

[tejohnson]

I think so. At least for composite types. Are there any cases, other than from the retainedTypes list on the compile unit, where a DICompositeType node is referenced by its address rather than by UUID?

Try types without a mangled name - I suppose one example might be something like:

a.cpp:
void b();
int main() {

b();

}

b.cpp:
namespace {
struct anon {
};
void bimpl(anon) {
}
}
void b() {

bimpl(anon());

}

If bimpl is imported into a (presumably after b is imported into a) then you'll see a type without a mangled name... what you should do with this is perhaps a difficult question. A debugger's going to have a hard time identifying these types (or even the two 'bimpl' functions) as the same when they're in two different object files & flagged as being "local" entities...

Confirmed that in this case, the composite type for anon is in the subroutine type list for bimpl, and that when we import bimpl into a.cpp we therefore import the full composite type definition.

It isn't clear to me from your last sentence what we should do here. Does having the full type definition imported make it easier or harder for the debugger?

Thanks,
Teresa

• Dave

[mehdi_amini]

I just checked on one of our projects and my ThinLTO think time improves from 3m31 (211s) to 3m8 (188s), i.e. 10% improvement with the current version of the patch.
(note this is the full link time, including optimization and codegen!)

[tejohnson]

Ping. Any other comments on this patch or can it go in?

[ahatanak]

lib/Linker/IRMover.cpp

1388

While I was building a project using thin-LTO, I found out that the call to MapMetadata here causes an assert if CU-getSubprograms().get() returns null (the assert is triggered in MapMetadataImpl when isa<MDString> is called on a null ptr). Should we check whether CU->getSubprograms() is empty before calling the function? Mehdi also suggested that we can do the check inside MapMetadaImpl and have it return null if the first argument is null. That might be better if there are other call sites that passes a null pointer as the first argument.

[tejohnson]

lib/Linker/IRMover.cpp

1388

Yes, either of those sound like they would fix this. I am traveling all week and won't be able to make any changes until early next week. Will see if I can make the change in MapMetadataImpl then, otherwise here.

[tejohnson]

Handle a null MD passed to MapMetadata to address problem reported by
ahatanak.

[tejohnson]

Ensure retained types are imported exactly once, as declaration, and
used both in the retained types list and in any referencing scope chains
for other imported retained types. Expand test to cover this case.

[tejohnson]

Add a new option to control full importing of types. By default import
only type decls instead of full defs.

[tejohnson]

In D16440#360836, @tejohnson wrote:

Add a new option to control full importing of types. By default import
only type decls instead of full defs.

Let me know if this is too big of a hammer. For now it will essentially turn off this entire patch, meaning not only will we get the full type defs, but we also import the full dbg.cu (including enums, etc).

[tejohnson]

Ping.

Mehdi - any more issues in your internal testing? Does this look good now?
Thanks

[mehdi_amini]

Sorry, I haven't been able to conduct more testing on this one since your last update :(

[mehdi_amini]

With the flag that allow to disable this optimization is something goes wrong (or the platform debugger isn't happy after that), I wouldn't oppose to this patch.
I'd rather have someone familiar with debug info to approve this revision, but I'm also OK to iterate on it post-commit if we find other issues.
Any of Adrian, David, or Duncan can express an opinion here?

[dblaikie]

lib/Linker/IRMover.cpp

1379

I'm not sure this is true. Here's a type you might have to import that is not in the retained types list:

namespace {
struct foo { };
void f(foo) { }
}
struct bar { };
void f(bar) { }
void f() {
  f(foo());
  f(bar());
}

The 'foo' type is not in the retained types list, but it may need to be imported if 'f(foo)' is imported, yes?

1391

Pretty sure you don't need the base type, size, alignment, and offset in a declaration - check equivalent declarations that are generated by clang? (check that the declarations this code creates look like natural declarations in the original source language, etc)

1404

This name seems a bit misleading. If I'm reading the code correctly, this is used for any type's scope context - which might not be a type at all. (it could be a namespace, for example) - and I assume the non-type cases come out in the "map the type metadata normally" part?

1420

Name seems a bit off - this links in potentially multiple compile units, not just a single one.

1437

What's postpass metadata linking? (what's the alternative?)

1438

I'm not really following why we need to iterate the retained types list - wouldn't we import everything starting from the subprograms we needed to import, not the retained types list?

1527

What if nothing was imported from this CU?

It seems to me we'd want to start at the imported functions, follow their DISubprograms, and import that way, rather than walking all the CUs and subprograms - maybe?

[tejohnson]

• Address review comments from dblaikie.

[llvm-commits]

Hey Teresa,

How will the changes we were discussing with Adrian yesterday on IRC will affect this patch?

[mehdi_amini]

I wonder if your approach here has still any gain with the new DebugInfo representation that Adrian and Duncan worked on?

[tejohnson]

In D16440#410434, @joker.eph wrote:

I wonder if your approach here has still any gain with the new DebugInfo representation that Adrian and Duncan worked on?

At the very least it is stale with Duncan's change in r267296 to eliminate remove MDString-based type references and Adrian's change in r266446 to remove the DISubprogramArray from the DICompileUnit.

For now the retainedTypes list still exists on the DICompileUnit, however, in Duncan's r267296 from this weekend he notes that the retainedTypes list is now unnecessary, so I assume he is removing that imminently?

Once the retainedTypes list is gone, most of this is unnecessary. However, this patch was also nulling out several other fields on the imported DICompileUnit, namely EnumTypes, GlobalVariables, ImportedEntities and Macros. What is the plan for those? Gping forward, should the DICompileUnit still be modified in some way when it is imported?

include/llvm/Linker/IRMover.h

public:
   IRMover(Module &M);
 
   typedef std::function<void(GlobalValue &)> ValueAdder;
   /// Move in the provide values. The source is destroyed.
   /// Returns true on error.
   bool move(Module &Src, ArrayRef<GlobalValue *> ValuesToLink,
             std::function<void(GlobalValue &GV, ValueAdder Add)> AddLazyFor,
             DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr,
-            bool IsMetadataLinkingPostpass = false);
+            bool IsMetadataLinkingPostpass = false,
+            bool IsFunctionImport = false);
   Module &getModule() { return Composite; }
 
 private:
   Module &Composite;
   IdentifiedStructTypeSet IdentifiedStructTypes;
 };
 
 } // End llvm namespace
 
 #endif

lib/Linker/IRMover.cpp

class IRLinker {
   bool DoneLinkingBodies = false;
 
   bool HasError = false;
 
   /// Flag indicating that we are just linking metadata (after function
   /// importing).
   bool IsMetadataLinkingPostpass;
 
+  /// Flag indicating that this is a module with a function being
+  /// imported for ThinLTO. Used to control the amount of metadata
+  /// linked in.
+  bool IsFunctionImport;
+
   /// Flags to pass to value mapper invocations.
   RemapFlags ValueMapperFlags = RF_MoveDistinctMDs;
 
   /// Association between metadata values created during bitcode parsing and
   /// the value id. Used to correlate temporary metadata created during
   /// function importing with the final metadata parsed during the subsequent
   /// metadata linking postpass.
   DenseMap<const Metadata *, unsigned> MetadataToIDs;
class IRLinker {
   /// Functions that take care of cloning a specific global value type
   /// into the destination module.
   GlobalVariable *copyGlobalVariableProto(const GlobalVariable *SGVar);
   Function *copyFunctionProto(const Function *SF);
   GlobalValue *copyGlobalAliasProto(const GlobalAlias *SGA);
 
   void linkNamedMDNodes();
 
+  /// For ThinLTO function importing, specialize the import of the
+  /// compile unit metadata to avoid importing unneeded type definitions.
+  void linkImportedCompileUnit();
+
+  /// Helper to create a new composite type declaration when importing
+  /// only the needed metadata (e.g. for ThinLTO).
+  Metadata *importCompositeTypeAsDeclaration(
+      DICompositeType *Composite,
+      DenseMap<DICompositeType *, DICompositeType *> &RetainMap,
+      DICompileUnit *CU);
+
+  /// Helper to import a type for ThinLTO, importing as a type declaration
+  /// if appropriate.
+  Metadata *
+  importType(Metadata *Ty,
+             DenseMap<DICompositeType *, DICompositeType *> &RetainMap,
+             DICompileUnit *CU);
+
   /// Populate the UnneededSubprograms set with the DISubprogram metadata
   /// from the source module that we don't need to link into the dest module,
   /// because the functions were not imported directly or via an inlined body
   /// in an imported function.
   void findNeededSubprograms();
 
   /// Recursive helper for findNeededSubprograms to locate any DISubprogram
   /// reached from the given Node, marking any found as needed.
   void findReachedSubprograms(const MDNode *Node,
                               SmallPtrSet<const MDNode *, 16> &Visited);
 
   /// The value mapper leaves nulls in the list of subprograms for any
   /// in the UnneededSubprograms map. Strip those out of the mapped
   /// compile unit.
   void stripNullSubprograms(DICompileUnit *CU);
 
 public:
   IRLinker(Module &DstM, IRMover::IdentifiedStructTypeSet &Set, Module &SrcM,
            ArrayRef<GlobalValue *> ValuesToLink,
            std::function<void(GlobalValue &, IRMover::ValueAdder)> AddLazyFor,
            DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr,
-           bool IsMetadataLinkingPostpass = false)
+           bool IsMetadataLinkingPostpass = false,
+           bool IsFunctionImport = false)
       : DstM(DstM), SrcM(SrcM), AddLazyFor(AddLazyFor), TypeMap(Set),
         GValMaterializer(this), LValMaterializer(this),
         IsMetadataLinkingPostpass(IsMetadataLinkingPostpass),
-        ValIDToTempMDMap(ValIDToTempMDMap) {
+        IsFunctionImport(IsFunctionImport), ValIDToTempMDMap(ValIDToTempMDMap) {
     for (GlobalValue *GV : ValuesToLink)
       maybeAdd(GV);
 
     // If appropriate, tell the value mapper that it can expect to see
     // temporary metadata.
     if (!shouldLinkMetadata())
       ValueMapperFlags = ValueMapperFlags | RF_HaveUnmaterializedMetadata;
   }
if (!SP) {
       continue;
     }
     NewSPs.push_back(SP);
   }
   if (FoundNull)
     CU->replaceSubprograms(MDTuple::get(CU->getContext(), NewSPs));
 }
 
+/// Helper to check if the given DIType is a composite type, and if
+/// so, whether it already exists in the dest module. Otherwise, return
+/// it as a composite type. Assumes that the provided type is needed
+/// in the dest module due to an import.
+static DICompositeType *
+getCompositeTypeToImport(DIType *Ty, const Module &DstM,
+                         DITypeIdentifierMap &DestTypeIdentifierMap) {
+  if (!Ty || !isa<DICompositeType>(Ty))
+    return nullptr;
+  // At this point we have an identifier string for a composite
+  // type that is needed by something mapped from the source module
+  // into the dest module.
+  auto *Composite = dyn_cast<DICompositeType>(Ty);
+  // This either returns an existing MDString matching the
+  // identifier StringRef, or creates a new one (which is
+  // fine given that we know we need this in the dest module).
+  auto *NewMDString =
+      MDString::get(DstM.getContext(), Composite->getIdentifier());
+  // If it already exists in the dest type map, no need to
+  // do anything.
+  if (NewMDString &&
+      (DestTypeIdentifierMap.find(NewMDString) != DestTypeIdentifierMap.end()))
+    return nullptr;
+  return Composite;
+}
+
+/// Recursively walks through the given MDNode's operands to locate
+/// the set of reached composite types that do not yet exist in the
+/// dest module. Any found are added to the \p RetainWorklist.
+static void
+findReachedCompositeTypes(const MDNode *Node, const Module &DstM,
+                          DITypeIdentifierMap &SrcTypeIdentifierMap,
+                          DITypeIdentifierMap &DestTypeIdentifierMap,
+                          std::vector<DICompositeType *> &RetainWorklist,
+                          SmallPtrSet<const MDNode *, 16> &Visited) {
+  if (!Visited.insert(Node).second)
+    return;
+
+  for (auto &Op : Node->operands()) {
+    if (auto *MDS = dyn_cast_or_null<MDString>(Op.get())) {
+      // First check if this maps to a composite type in the source.
+      auto I = SrcTypeIdentifierMap.find(MDS);
+      if (I == SrcTypeIdentifierMap.end())
+        continue;
+      auto *Ty = cast<DIType>(I->second);
+      // Invoke getCompositeTypeToImport to see if this is a composite
+      // type that is not yet in the dest module.
+      auto *Composite =
+          getCompositeTypeToImport(Ty, DstM, DestTypeIdentifierMap);
+      if (Composite)
+        RetainWorklist.push_back(Composite);
+    } else if (auto *OpN = dyn_cast_or_null<MDNode>(Op.get()))
+      // Recursively check the MDNode operands
+      findReachedCompositeTypes(OpN, DstM, SrcTypeIdentifierMap,
+                                DestTypeIdentifierMap, RetainWorklist, Visited);
+  }
+}
+
+Metadata *IRLinker::importCompositeTypeAsDeclaration(
+    DICompositeType *Composite,
+    DenseMap<DICompositeType *, DICompositeType *> &RetainMap,
+    DICompileUnit *CU) {
+  auto RMI = RetainMap.find(Composite);
+  // An entry should have been made in the map for any composite type
+  // that is needed, and we should only invoke this for needed composite types.
+  assert(RMI != RetainMap.end());
+  // If we already created a new composite type declaration, use it.
+  if (auto *NewComposite = RMI->second)
+    return NewComposite;
+
+  // This is the first time we invoked this routine for this composite
+  // type. Create a declaration for the imported copy.
+  auto *NewScope = importType(Composite->getScope(), RetainMap, CU);
+  auto *NewBaseType = importType(Composite->getBaseType(), RetainMap, CU);
+  auto *NewComposite = DICompositeType::get(
+      CU->getContext(), Composite->getTag(), Composite->getRawName(),
+      Composite->getFile(), Composite->getLine(), NewScope, NewBaseType,
+      Composite->getSizeInBits(), Composite->getAlignInBits(),
+      Composite->getOffsetInBits(), Composite->getFlags() | DINode::FlagFwdDecl,
+      nullptr /* Elements */, Composite->getRuntimeLang(),
+      nullptr /* VTableHolder */, nullptr /* TemplateParams */,
+      Composite->getRawIdentifier());
+  // Save this new declaration so it is used elsewhere (either in the
+  // imported retained types map, or as the scope in other imported
+  // composite types).
+  RetainMap[Composite] = NewComposite;
+  return NewComposite;
+}
+
+Metadata *
+IRLinker::importType(Metadata *Ty,
+                     DenseMap<DICompositeType *, DICompositeType *> &RetainMap,
+                     DICompileUnit *CU) {
+  if (!Ty)
+    return nullptr;
+  // If this is a composite type, we want to import it as a declaration.
+  // Any existing imported copy in the \p RetainMap is used.
+  if (isa<DICompositeType>(Ty))
+    return importCompositeTypeAsDeclaration(dyn_cast<DICompositeType>(Ty),
+                                            RetainMap, CU);
+  // Otherwise, map the type metadata normally.
+  return MapMetadata(Ty, ValueMap,
+                     ValueMapperFlags | RF_NullMapMissingGlobalValues, &TypeMap,
+                     &GValMaterializer);
+}
+
+void IRLinker::linkImportedCompileUnit() {
+  NamedMDNode *SrcCompileUnits = SrcM.getNamedMetadata("llvm.dbg.cu");
+  NamedMDNode *DestCompileUnits = DstM.getNamedMetadata("llvm.dbg.cu");
+  auto SrcTypeIdentifierMap = generateDITypeIdentifierMap(SrcCompileUnits);
+  auto DestTypeIdentifierMap = generateDITypeIdentifierMap(DestCompileUnits);
+  for (unsigned I = 0, E = SrcCompileUnits->getNumOperands(); I != E; ++I) {
+    auto *CU = cast<DICompileUnit>(SrcCompileUnits->getOperand(I));
+    assert(CU && "Expected valid compile unit");
+    auto *NewFile =
+        MapMetadata(CU->getFile(), ValueMap,
+                    ValueMapperFlags | RF_NullMapMissingGlobalValues, &TypeMap,
+                    &GValMaterializer);
+    auto *NewSubprograms =
+        MapMetadata(CU->getSubprograms().get(), ValueMap,
+                    ValueMapperFlags | RF_NullMapMissingGlobalValues, &TypeMap,
+                    &GValMaterializer);
+    std::vector<DICompositeType *> RetainWorklist;
+    if (!IsMetadataLinkingPostpass) {
+      for (DIType *Ty : CU->getRetainedTypes()) {
+        // For non-postpass metadata linking, any metadata referenced
+        // by imported functions should be mapped. If not, then we didn't
+        // need this composite type (unless it serves as the scope for
+        // another imported type, which is handled while iterating over
+        // the worklist.
+        auto *Composite = dyn_cast<DICompositeType>(Ty);
+        if (!Composite)
+          continue;
+        Metadata *NewMD =
+            ValueMap.MD().lookup(Composite->getRawIdentifier()).get();
+        if (!NewMD)
+          continue;
+
+        // Although we know Op is a composite, this will return null
+        // if it is already mapped into the dest.
+        Composite = getCompositeTypeToImport(Ty, DstM, DestTypeIdentifierMap);
+        if (!Composite)
+          continue;
+
+        RetainWorklist.push_back(Composite);
+      }
+    } else {
+      // For postpass metadata linking, we have not yet mapped in the
+      // subprograms, so we need to recursively visit any needed subprograms
+      // to locate all referenced composite types not yet in the dest module.
+      SmallPtrSet<const MDNode *, 16> Visited;
+      for (auto *Op : CU->getSubprograms()) {
+        if (UnneededSubprograms.count(Op))
+          continue;
+        findReachedCompositeTypes(Op, DstM, SrcTypeIdentifierMap,
+                                  DestTypeIdentifierMap, RetainWorklist,
+                                  Visited);
+      }
+    }
+    // Next we need to walk up through the scope chain of any reached
+    // composite types and bring in any composite types that are the outer
+    // scope for our retained types. Insert into the map with a null value,
+    // which will eventually be replaced with the imported type declaration.
+    DenseMap<DICompositeType *, DICompositeType *> RetainMap;
+    for (auto *Composite : RetainWorklist) {
+      // Add each visited retained type to the map.
+      // We can abort the scope walk once we hit an already identified and
+      // therefore visited retained type.
+      while (RetainMap.insert(std::make_pair(Composite, nullptr)).second) {
+        // Get the scope of this retained type and check if it is a
+        // composite that is not yet in the dest module.
+        DIType *Ty = dyn_cast_or_null<DIType>(
+            Composite->getScope().resolve(SrcTypeIdentifierMap));
+        Composite = getCompositeTypeToImport(Ty, DstM, DestTypeIdentifierMap);
+        // If not, we can abort the scope walk.
+        if (!Composite)
+          break;
+      }
+    }
+    // Finally, walk through the source modules retained types list and
+    // add any identified as needed to the list that will be mapped into
+    // the new compile unit in the dest module. This ensures that the
+    // retained types list ordering is stable.
+    SmallVector<Metadata *, 16> RetainValues;
+    for (auto *Ty : CU->getRetainedTypes()) {
+      auto *Composite = dyn_cast<DICompositeType>(Ty);
+      if (!Composite)
+        continue;
+      // First check if this was identified as needed for an imported function,
+      // possibly indirectly via a scope chain of another needed type.
+      auto RMI = RetainMap.find(Composite);
+      if (RMI == RetainMap.end())
+        continue;
+      // Import it as a type declaration. The \p RetainMap is used to
+      // ensure we import as a declaration exactly once. The composite
+      // type might be encountered both in this retained types list and
+      // again in the scope chain of another imported retained type.
+      auto *NewComposite =
+          importCompositeTypeAsDeclaration(Composite, RetainMap, CU);
+      RetainValues.push_back(NewComposite);
+    }
+    // Create the simple DICompileUnit to hold the necessary
+    // mapped operands (DISubprogram and DIFile) and other simple
+    // non-metadata operands.
+    auto *NewCU = DICompileUnit::getDistinct(
+        CU->getContext(), CU->getSourceLanguage(), NewFile,
+        CU->getRawProducer(), CU->isOptimized(), CU->getRawFlags(),
+        CU->getRuntimeVersion(), CU->getRawSplitDebugFilename(),
+        CU->getEmissionKind(), nullptr /* EnumTypes */,
+        MDTuple::get(CU->getContext(), RetainValues), NewSubprograms,
+        nullptr /* GlobalVariables */, nullptr /* ImportedEntities */,
+        nullptr /* Macros */, CU->getDWOId());
+    stripNullSubprograms(NewCU);
+    DestCompileUnits->addOperand(NewCU);
+  }
+}
+
 /// Insert all of the named MDNodes in Src into the Dest module.
 void IRLinker::linkNamedMDNodes() {
   findNeededSubprograms();
   const NamedMDNode *SrcModFlags = SrcM.getModuleFlagsMetadata();
+  NamedMDNode *CompileUnits = SrcM.getNamedMetadata("llvm.dbg.cu");
   for (const NamedMDNode &NMD : SrcM.named_metadata()) {
     // Don't link module flags here. Do them separately.
     if (&NMD == SrcModFlags)
       continue;
     NamedMDNode *DestNMD = DstM.getOrInsertNamedMetadata(NMD.getName());
+
+    // Check if we are importing a function from the source module for
+    // ThinLTO. In that case we only need to map in metadata for imported
+    // functions and any required type declarations, and don't need to link in
+    // the enums, type definitions and other metadata hanging off the
+    // DICompileUnit. Create a simple DICompileUnit with just the needed
+    // metadata and other operands populated.
+    if (&NMD == CompileUnits && IsFunctionImport) {
+      linkImportedCompileUnit();
+      continue;
+    }
+
     // Add Src elements into Dest node.
     for (const MDNode *op : NMD.operands()) {
       MDNode *DestMD = MapMetadata(
           op, ValueMap, ValueMapperFlags | RF_NullMapMissingGlobalValues,
           &TypeMap, &GValMaterializer);
       // For each newly mapped compile unit remove any null subprograms,
       // which occur when findNeededSubprograms identified any as unneeded
       // in the dest module.
       if (auto *CU = dyn_cast<DICompileUnit>(DestMD))
         stripNullSubprograms(CU);
       DestNMD->addOperand(DestMD);
     }
   }
 }
 
 /// Merge the linker flags in Src into the Dest module.
 bool IRLinker::linkModuleFlagsMetadata() {
   // If the source module has no module flags, we are done.
   const NamedMDNode *SrcModFlags = SrcM.getModuleFlagsMetadata();
   if (!SrcModFlags)
     return false;
 
else
       IdentifiedStructTypes.addNonOpaque(Ty);
   }
 }
 
 bool IRMover::move(
     Module &Src, ArrayRef<GlobalValue *> ValuesToLink,
     std::function<void(GlobalValue &, ValueAdder Add)> AddLazyFor,
     DenseMap<unsigned, MDNode *> *ValIDToTempMDMap,
-    bool IsMetadataLinkingPostpass) {
+    bool IsMetadataLinkingPostpass, bool IsFunctionImport) {
   IRLinker TheIRLinker(Composite, IdentifiedStructTypes, Src, ValuesToLink,
-                       AddLazyFor, ValIDToTempMDMap, IsMetadataLinkingPostpass);
+                       AddLazyFor, ValIDToTempMDMap, IsMetadataLinkingPostpass,
+                       IsFunctionImport);
   bool RetCode = TheIRLinker.run();
   Composite.dropTriviallyDeadConstantArrays();
   return RetCode;
 }

lib/Linker/LinkModules.cpp

if (shouldInternalizeLinkedSymbols()) {
     for (GlobalValue *GV : ValuesToLink)
       Internalize.insert(GV->getName());
   }
 
   if (Mover.move(SrcM, ValuesToLink.getArrayRef(),
                  [this](GlobalValue &GV, IRMover::ValueAdder Add) {
                    addLazyFor(GV, Add);
                  },
-                 ValIDToTempMDMap, false))
+                 ValIDToTempMDMap, false, isPerformingImport()))
     return true;
   Module &DstM = Mover.getModule();
   for (auto &P : Internalize) {
     GlobalValue *GV = DstM.getNamedValue(P.first());
     GV->setLinkage(GlobalValue::InternalLinkage);
   }
 
   return false;

 }
 
 bool Linker::linkInMetadata(Module &Src,
                             DenseMap<unsigned, MDNode *> *ValIDToTempMDMap) {
   SetVector<GlobalValue *> ValuesToLink;
   if (Mover.move(
           Src, ValuesToLink.getArrayRef(),
           [this](GlobalValue &GV, IRMover::ValueAdder Add) { assert(false); },
-          ValIDToTempMDMap, true))
+          ValIDToTempMDMap, true, true))
     return true;
   return false;
 }
 
 //===----------------------------------------------------------------------===//
 // LinkModules entrypoint.
 //===----------------------------------------------------------------------===//
 

lib/Transforms/Utils/ValueMapper.cpp

static Metadata *MapMetadataImpl(const Metadata *MD,
 
   return mapUniquedNode(Node, DistinctWorklist, VM, Flags, TypeMapper,
                         Materializer);
 }
 
 Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,
                             RemapFlags Flags, ValueMapTypeRemapper *TypeMapper,
                             ValueMaterializer *Materializer) {
+  if (!MD)
+    return nullptr;
+
   SmallVector<MDNode *, 8> DistinctWorklist;
   Metadata *NewMD = MapMetadataImpl(MD, DistinctWorklist, VM, Flags, TypeMapper,
                                     Materializer);
 
   // When there are no module-level changes, it's possible that the metadata
   // graph has temporaries.  Skip the logic to resolve cycles, since it's
   // unnecessary (and invalid) in that case.
   if (Flags & RF_NoModuleLevelChanges)

test/Linker/thinlto_funcimport_debug.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: llvm-as -function-summary %s -o %t.bc
 ; RUN: llvm-as -function-summary %p/Inputs/thinlto_funcimport_debug.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
 
 ; If we import func1 and not func2 we should only link DISubprogram for func1
 ; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc -import=func1:%t.bc -S | FileCheck %s
 
 ; CHECK: declare i32 @func2
 ; CHECK: define available_externally i32 @func1
 
 ; Extract out the list of subprograms from each compile unit and ensure
-; that neither contains null.
+; that neither contains null (by checking that each contains exactly the
+; expected number of subprograms).
 ; CHECK: !{{[0-9]+}} = distinct !DICompileUnit({{.*}} subprograms: ![[SPs1:[0-9]+]]
-; CHECK-NOT: ![[SPs1]] = !{{{.*}}null{{.*}}}
-; CHECK: !{{[0-9]+}} = distinct !DICompileUnit({{.*}} subprograms: ![[SPs2:[0-9]+]]
-; CHECK-NOT: ![[SPs2]] = !{{{.*}}null{{.*}}}
+; CHECK: ![[SPs1]] = !{!{{[0-9]+}}}
+; Confirm that the enums didn't get pulled in to the imported compile unit
+; as it isn't needed here, but ensuring that retainedTypes immediately follows
+; emissionKind.
+; CHECK: !{{[0-9]+}} = distinct !DICompileUnit({{.*}} emissionKind: {{[0-9]+}}, retainedTypes: ![[RTs:[0-9]+]], subprograms: ![[SPs2:[0-9]+]]
+
+; CHECK: ![[RTs]] = !{![[RT1:[0-9]+]], ![[RT2:[0-9]+]], ![[RT3:[0-9]+]]}
+; CompositeID1 is scope of imported func1, should be mapped in as declaration.
+; CHECK: ![[RT1]] = !DICompositeType(tag: DW_TAG_class_type, name: "Composite1", scope: !"CompositeID2", file: !{{[0-9]+}}, line: 100, size: 64, align: 64, flags: DIFlagFwdDecl, identifier: "CompositeID1")
+; CompositeID2 is scope of CompositeID1, and should be mapped as declaration.
+; CHECK: ![[RT2]] = !DICompositeType(tag: DW_TAG_class_type, name: "Composite2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 100, size: 64, align: 64, flags: DIFlagFwdDecl, identifier: "CompositeID2")
+; CompositeID3 is scope of non-imported func2, should not be mapped in.
+; CompositeID4 is vtableHolder of imported composite type (which won't have
+; a vtableHolder on its imported declaration), as well as base
+; type of non-imported derived type, neither should cause it to be mapped in.
+; CompositeID5 is baseType of imported derived type, should be mapped in as
+; declaration.
+; CHECK: ![[RT3]] = !DICompositeType(tag: DW_TAG_class_type, name: "Composite5", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 100, size: 64, align: 64, flags: DIFlagFwdDecl, identifier: "CompositeID5")
 
+; CHECK: ![[SPs2]] = !{!{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}}
 ; CHECK: distinct !DISubprogram(name: "func1"
+; This derived type is in the subroutine type list of imported func1, and should
+; be mapped in.
+; CHECK: DIDerivedType(tag: DW_TAG_pointer_type, baseType: !"CompositeID5"
 ; CHECK-NOT: distinct !DISubprogram(name: "func2"
 ; CHECK: distinct !DISubprogram(name: "func3"
 ; CHECK: distinct !DISubprogram(name: "func4"
 
 
 ; ModuleID = 'dbg.o'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: nounwind readnone uwtable
-define i32 @func1(i32 %n) #0 !dbg !4 {
+define i32 @func1(i32 %n) #0 !dbg !12 {
 entry:
-  tail call void @llvm.dbg.value(metadata i32 %n, i64 0, metadata !9, metadata !17), !dbg !18
-  tail call void @llvm.dbg.value(metadata i32 5, i64 0, metadata !10, metadata !17), !dbg !19
-  %cmp = icmp sgt i32 %n, 10, !dbg !20
-  %. = select i1 %cmp, i32 10, i32 5, !dbg !22
-  tail call void @llvm.dbg.value(metadata i32 %., i64 0, metadata !10, metadata !17), !dbg !19
-  ret i32 %., !dbg !23
+  tail call void @llvm.dbg.value(metadata i32 %n, i64 0, metadata !18, metadata !30), !dbg !31
+  tail call void @llvm.dbg.value(metadata i32 5, i64 0, metadata !19, metadata !30), !dbg !32
+  %cmp = icmp sgt i32 %n, 10, !dbg !33
+  %. = select i1 %cmp, i32 10, i32 5, !dbg !36
+  tail call void @llvm.dbg.value(metadata i32 %., i64 0, metadata !19, metadata !30), !dbg !32
+  ret i32 %., !dbg !37
 }
 
 ; Function Attrs: nounwind readnone uwtable
-define i32 @func2(i32 %n) #0 !dbg !11 {
+define i32 @func2(i32 %n) #0 !dbg !20 {
 entry:
-  tail call void @llvm.dbg.value(metadata i32 %n, i64 0, metadata !13, metadata !17), !dbg !24
-  ret i32 %n, !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %n, i64 0, metadata !22, metadata !30), !dbg !38
+  ret i32 %n, !dbg !39
 }
 
 ; Function Attrs: nounwind readnone
 declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #1
 
 attributes #0 = { nounwind readnone uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #1 = { nounwind readnone }
 
 !llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!14, !15}
-!llvm.ident = !{!16}
+!llvm.module.flags = !{!27, !28}
+!llvm.ident = !{!29}
 
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)", isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3)
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)", isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !2, retainedTypes: !3, subprograms: !11)
 !1 = !DIFile(filename: "dbg.c", directory: ".")
 !2 = !{}
-!3 = !{!4, !11, !27, !30}
-!4 = distinct !DISubprogram(name: "func1", scope: !1, file: !1, line: 1, type: !5, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, variables: !8)
-!5 = !DISubroutineType(types: !6)
-!6 = !{!7, !7}
-!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
-!8 = !{!9, !10}
-!9 = !DILocalVariable(name: "n", arg: 1, scope: !4, file: !1, line: 1, type: !7)
-!10 = !DILocalVariable(name: "x", scope: !4, file: !1, line: 2, type: !7)
-!11 = distinct !DISubprogram(name: "func2", scope: !1, file: !1, line: 8, type: !5, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, variables: !12)
-!12 = !{!13}
-!13 = !DILocalVariable(name: "n", arg: 1, scope: !11, file: !1, line: 8, type: !7)
-!14 = !{i32 2, !"Dwarf Version", i32 4}
-!15 = !{i32 2, !"Debug Info Version", i32 3}
-!16 = !{!"clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)"}
-!17 = !DIExpression()
-!18 = !DILocation(line: 1, column: 15, scope: !4)
-!19 = !DILocation(line: 2, column: 7, scope: !4)
-!20 = !DILocation(line: 3, column: 9, scope: !21, inlinedAt: !26)
-!21 = distinct !DILexicalBlock(scope: !27, file: !1, line: 3, column: 7)
-!22 = !DILocation(line: 3, column: 7, scope: !4)
-!23 = !DILocation(line: 5, column: 3, scope: !4)
-!24 = !DILocation(line: 8, column: 15, scope: !11)
-!25 = !DILocation(line: 9, column: 3, scope: !11)
-!26 = !DILocation(line: 9, column: 3, scope: !4)
-!27 = distinct !DISubprogram(name: "func3", scope: !1, file: !1, line: 8, type: !5, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, variables: !28)
-!28 = !{!29}
-!29 = !DILocalVariable(name: "n", arg: 1, scope: !30, file: !1, line: 8, type: !7)
-!30 = distinct !DISubprogram(name: "func4", scope: !1, file: !1, line: 8, type: !5, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, variables: !31)
-!31 = !{!32}
-!32 = !DILocalVariable(name: "n", arg: 1, scope: !30, file: !1, line: 8, type: !7)
-
+!3 = !{!4, !7, !8, !9, !10}
+!4 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite1", scope: !"CompositeID2", file: !1, line: 100, size: 64, align: 64, elements: !5, vtableHolder: !"CompositeID4", templateParams: !2, identifier: "CompositeID1")
+!5 = !{!6}
+!6 = !DIDerivedType(tag: DW_TAG_inheritance, scope: !"CompositeID1", baseType: !"CompositeID4", flags: DIFlagPublic)
+!7 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite2", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, vtableHolder: !"CompositeID4", templateParams: !2, identifier: "CompositeID2")
+!8 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite3", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, vtableHolder: !"CompositeID4", templateParams: !2, identifier: "CompositeID3")
+!9 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite4", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, templateParams: !2, identifier: "CompositeID4")
+!10 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite5", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, templateParams: !2, identifier: "CompositeID5")
+!11 = !{!12, !20, !23, !26}
+!12 = distinct !DISubprogram(name: "func1", scope: !"CompositeID1", file: !1, line: 1, type: !13, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, variables: !17)
+!13 = !DISubroutineType(types: !14)
+!14 = !{!15, !15, !16}
+!15 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !"CompositeID5", flags: DIFlagPublic)
+!17 = !{!18, !19}
+!18 = !DILocalVariable(name: "n", arg: 1, scope: !12, file: !1, line: 1, type: !15)
+!19 = !DILocalVariable(name: "x", scope: !12, file: !1, line: 2, type: !15)
+!20 = distinct !DISubprogram(name: "func2", scope: !"CompositeID3", file: !1, line: 8, type: !13, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, variables: !21)
+!21 = !{!22}
+!22 = !DILocalVariable(name: "n", arg: 1, scope: !20, file: !1, line: 8, type: !15)
+!23 = distinct !DISubprogram(name: "func3", scope: !1, file: !1, line: 8, type: !13, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, variables: !24)
+!24 = !{!25}
+!25 = !DILocalVariable(name: "n", arg: 1, scope: !26, file: !1, line: 8, type: !15)
+!26 = distinct !DISubprogram(name: "func4", scope: !1, file: !1, line: 8, type: !13, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, variables: !24)
+!27 = !{i32 2, !"Dwarf Version", i32 4}
+!28 = !{i32 2, !"Debug Info Version", i32 3}
+!29 = !{!"clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)"}
+!30 = !DIExpression()
+!31 = !DILocation(line: 1, column: 15, scope: !12)
+!32 = !DILocation(line: 2, column: 7, scope: !12)
+!33 = !DILocation(line: 3, column: 9, scope: !34, inlinedAt: !35)
+!34 = distinct !DILexicalBlock(scope: !23, file: !1, line: 3, column: 7)
+!35 = !DILocation(line: 9, column: 3, scope: !12)
+!36 = !DILocation(line: 3, column: 7, scope: !12)
+!37 = !DILocation(line: 5, column: 3, scope: !12)
+!38 = !DILocation(line: 8, column: 15, scope: !20)
+!39 = !DILocation(line: 9, column: 3, scope: !20)

test/Transforms/FunctionImport/Inputs/funcimport_debug.ll

 ; ModuleID = 'funcimport_debug.o'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: nounwind uwtable
-define void @func() #0 !dbg !4 {
+define void @func() #0 !dbg !12 {
 entry:
-    ret void, !dbg !10
+  ret void, !dbg !19, !tbaa !27
 }
 
 attributes #0 = { nounwind uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" }
 
 !llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!7, !8}
-!llvm.ident = !{!9}
+!llvm.module.flags = !{!16, !17}
+!llvm.ident = !{!18}
 
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 255685) (llvm/trunk 255682)", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3)
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 255685) (llvm/trunk 255682)", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, retainedTypes: !3, subprograms: !11)
 !1 = !DIFile(filename: "funcimport_debug.c", directory: ".")
 !2 = !{}
-!3 = !{!4}
-!4 = distinct !DISubprogram(name: "func", scope: !1, file: !1, line: 1, type: !5, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: false, variables: !2)
-!5 = !DISubroutineType(types: !6)
-!6 = !{null}
-!7 = !{i32 2, !"Dwarf Version", i32 4}
-!8 = !{i32 2, !"Debug Info Version", i32 3}
-!9 = !{!"clang version 3.8.0 (trunk 255685) (llvm/trunk 255682)"}
-!10 = !DILocation(line: 2, column: 1, scope: !4)
+!3 = !{!4, !7, !8, !9, !10, !20, !21, !28}
+!4 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite1", scope: !"CompositeID2", file: !1, line: 100, size: 64, align: 64, elements: !5, vtableHolder: !"CompositeID4", templateParams: !2, identifier: "CompositeID1")
+!5 = !{!6}
+!6 = !DIDerivedType(tag: DW_TAG_inheritance, scope: !"CompositeID1", baseType: !"CompositeID4", flags: DIFlagPublic)
+!7 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite2", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, vtableHolder: !"CompositeID4", templateParams: !2, identifier: "CompositeID2")
+!8 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite3", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, vtableHolder: !"CompositeID4", templateParams: !2, identifier: "CompositeID3")
+!9 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite4", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, templateParams: !2, identifier: "CompositeID4")
+!10 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite5", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, templateParams: !2, identifier: "CompositeID5")
+!11 = !{!12}
+!12 = distinct !DISubprogram(name: "func", scope: !"CompositeID1", file: !1, line: 1, type: !13, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: false, variables: !2)
+!13 = !DISubroutineType(types: !14)
+!14 = !{!15}
+!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !"CompositeID5", flags: DIFlagPublic)
+!16 = !{i32 2, !"Dwarf Version", i32 4}
+!17 = !{i32 2, !"Debug Info Version", i32 3}
+!18 = !{!"clang version 3.8.0 (trunk 255685) (llvm/trunk 255682)"}
+!19 = !DILocation(line: 2, column: 1, scope: !12)
+!20 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite6", scope: !21, file: !1, line: 100, size: 64, align: 64, elements: !5, vtableHolder: !"CompositeID4", templateParams: !2, identifier: "CompositeID6")
+!21 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite7", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !22, vtableHolder: !"CompositeID4", templateParams: !2, identifier: "CompositeID7")
+!22 = !{!23}
+!23 = !DISubprogram(name: "func2", scope: !1, file: !1, line: 1158, type: !24, isLocal: false, isDefinition: false, scopeLine: 1158, flags: DIFlagPrototyped, isOptimized: true)
+!24 = !DISubroutineType(types: !25)
+!25 = !{!26}
+!26 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !"CompositeID8", size: 64, align: 64)
+!27 = !{!"CompositeID6"}
+!28 = !DICompositeType(tag: DW_TAG_class_type, name: "Composite8", scope: !1, file: !1, line: 100, size: 64, align: 64, elements: !2, templateParams: !2, identifier: "CompositeID8")

test/Transforms/FunctionImport/funcimport_debug.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: llvm-as -function-summary %s -o %t.bc
 ; RUN: llvm-as -function-summary %p/Inputs/funcimport_debug.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
 
 ; Do the import now and confirm that metadata is linked for imported function.
 ; RUN: opt -function-import -summary-file %t3.thinlto.bc %s -S | FileCheck %s
 
 ; CHECK: define available_externally void @func()
 
 ; Check that we have exactly two subprograms (that func's subprogram wasn't
 ; linked more than once for example), and that they are connected to
 ; the subprogram list on a compute unit.
 ; CHECK: !{{[0-9]+}} = distinct !DICompileUnit({{.*}} subprograms: ![[SPs1:[0-9]+]]
 ; CHECK: ![[SPs1]] = !{![[MAINSP:[0-9]+]]}
 ; CHECK: ![[MAINSP]] = distinct !DISubprogram(name: "main"
-; CHECK: !{{[0-9]+}} = distinct !DICompileUnit({{.*}} subprograms: ![[SPs2:[0-9]+]]
-; CHECK-NOT: ![[SPs2]] = !{{{.*}}null{{.*}}}
+
+; Confirm that the enums didn't get pulled in to the imported compile unit
+; as it isn't needed here, but ensuring that retainedTypes immediately follows
+; emissionKind.
+; CHECK: !{{[0-9]+}} = distinct !DICompileUnit({{.*}} emissionKind: {{[0-9]+}}, retainedTypes: ![[RTs:[0-9]+]], subprograms: ![[SPs2:[0-9]+]]
+
+; CHECK: ![[RTs]] = !{![[RT1:[0-9]+]], ![[RT2:[0-9]+]], ![[RT3:[0-9]+]], ![[RT4:[0-9]+]], ![[RT5:[0-9]+]]}
+; CompositeID1 is scope of imported func, should be mapped in as declaration.
+; CHECK: ![[RT1]] = !DICompositeType(tag: DW_TAG_class_type, name: "Composite1", scope: !"CompositeID2", file: !{{[0-9]+}}, line: 100, size: 64, align: 64, flags: DIFlagFwdDecl, identifier: "CompositeID1")
+; CompositeID2 is scope of CompositeID1, and should be mapped as declaration.
+; CHECK: ![[RT2]] = !DICompositeType(tag: DW_TAG_class_type, name: "Composite2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 100, size: 64, align: 64, flags: DIFlagFwdDecl, identifier: "CompositeID2")
+; CompositeID3 is not referenced (in reality could be the scope of a
+; non-imported function), and should not be mapped in.
+; CompositeID4 is vtableHolder of imported composite type (which won't have
+; a vtableHolder on its imported declaration), as well as base
+; type of non-imported derived type, neither should cause it to be mapped in.
+; CompositeID5 is baseType of imported derived type, should be mapped in as
+; declaration.
+; CHECK: ![[RT3]] = !DICompositeType(tag: DW_TAG_class_type, name: "Composite5", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 100, size: 64, align: 64, flags: DIFlagFwdDecl, identifier: "CompositeID5")
+; CompositeID6 is referenced via a tbaa instruction attachment in the imported
+; function and should be mapped in as a declaration.
+; CHECK: ![[RT4]] = !DICompositeType(tag: DW_TAG_class_type, name: "Composite6", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 100, size: 64, align: 64, flags: DIFlagFwdDecl, identifier: "CompositeID6")
+; CompositeID7 is the scope of needed CompositeID6, and should be mapped in as
+; a declaration.
+; CHECK: ![[RT5]] = !DICompositeType(tag: DW_TAG_class_type, name: "Composite7", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 100, size: 64, align: 64, flags: DIFlagFwdDecl, identifier: "CompositeID7")
+; CompositeID8 is only reached via the definition of CompositeID7, and should
+; not be mapped in.
+
 ; CHECK: ![[SPs2]] = !{![[FUNCSP:[0-9]+]]}
 ; CHECK: ![[FUNCSP]] = distinct !DISubprogram(name: "func"
 ; CHECK-NOT: distinct !DISubprogram
 
 ; ModuleID = 'funcimport_debug.o'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"