ThinLTO: Don't import imported entities (not even local ones)

[dblaikie]

Summary

This was my mistake/bad advice when this feature was being implemented -
currently local imported entities are only attached to function
definitions - and definitions are created (even though there's no code
for them) in the DWARF to attach the imported entity to if no definition
exists otherwise.

So importing the imported entities when the only thing that's going to
be produced is inline definitions is unnecessary & also leads to
creationg of extra CUs to house the synthesized definition which is
otherwise unused.

At some point the imported entity work could be improved on, to (as the
old comment mentions) move local imported entities to be attached to the
function/subprogram so they would eb dropped/imported naturally - but
also to ensure the imported entities go on the appropriate entities
(maybe even on the inlined subroutine, or on the abstract origin(s)
(there may be multiple abstract origins when doing Fission(especially
DWP)+LTO because cross-CU references aren't possible there))

[tejohnson]

LGTM

[dblaikie]

Actually realized there's a better/different direction for this (more debug focussed & able to use this info better rather than skipping/dropping/omitting it). Thanks for looking though!

lib/Linker/IRMover.cpp

void IRLinker::prepareCompileUnitsForImport() {
   if (!SrcCompileUnits)
     return;
   // When importing for ThinLTO, prevent importing of types listed on
   // the DICompileUnit that we don't need a copy of in the importing
   // module. They will be emitted by the originating module.
   for (unsigned I = 0, E = SrcCompileUnits->getNumOperands(); I != E; ++I) {
     auto *CU = cast<DICompileUnit>(SrcCompileUnits->getOperand(I));
     assert(CU && "Expected valid compile unit");
-    // Enums, macros, and retained types don't need to be listed on the
-    // imported DICompileUnit. This means they will only be imported
-    // if reached from the mapped IR. Do this by setting their value map
-    // entries to nullptr, which will automatically prevent their importing
+    // Enums, macros, retained types, and imported entities don't need to be
+    // listed on the imported DICompileUnit. This means they will only be
+    // imported if reached from the mapped IR. Do this by setting their value
+    // map entries to nullptr, which will automatically prevent their importing
     // when reached from the DICompileUnit during metadata mapping.
     ValueMap.MD()[CU->getRawEnumTypes()].reset(nullptr);
     ValueMap.MD()[CU->getRawMacros()].reset(nullptr);
     ValueMap.MD()[CU->getRawRetainedTypes()].reset(nullptr);
+    ValueMap.MD()[CU->getRawImportedEntities()].reset(nullptr);
     // If we ever start importing global variable defs, we'll need to
     // add their DIGlobalVariable to the globals list on the imported
     // DICompileUnit. Confirm none are imported, and then we can
     // map the list of global variables to nullptr.
     assert(none_of(
                ValuesToLink,
                [](const GlobalValue *GV) { return isa<GlobalVariable>(GV); }) &&
            "Unexpected importing of a GlobalVariable definition");
     ValueMap.MD()[CU->getRawGlobalVariables()].reset(nullptr);
-
-    // Imported entities only need to be mapped in if they have local
-    // scope, as those might correspond to an imported entity inside a
-    // function being imported (any locally scoped imported entities that
-    // don't end up referenced by an imported function will not be emitted
-    // into the object). Imported entities not in a local scope
-    // (e.g. on the namespace) only need to be emitted by the originating
-    // module. Create a list of the locally scoped imported entities, and
-    // replace the source CUs imported entity list with the new list, so
-    // only those are mapped in.
-    // FIXME: Locally-scoped imported entities could be moved to the
-    // functions they are local to instead of listing them on the CU, and
-    // we would naturally only link in those needed by function importing.
-    SmallVector<TrackingMDNodeRef, 4> AllImportedModules;
-    bool ReplaceImportedEntities = false;
-    for (auto *IE : CU->getImportedEntities()) {
-      DIScope *Scope = IE->getScope();
-      assert(Scope && "Invalid Scope encoding!");
-      if (isa<DILocalScope>(Scope))
-        AllImportedModules.emplace_back(IE);
-      else
-        ReplaceImportedEntities = true;
-    }
-    if (ReplaceImportedEntities) {
-      if (!AllImportedModules.empty())
-        CU->replaceImportedEntities(MDTuple::get(
-            CU->getContext(),
-            SmallVector<Metadata *, 16>(AllImportedModules.begin(),
-                                        AllImportedModules.end())));
-      else
-        // If there were no local scope imported entities, we can map
-        // the whole list to nullptr.
-        ValueMap.MD()[CU->getRawImportedEntities()].reset(nullptr);
-    }
   }
 }
 
 /// Insert all of the named MDNodes in Src into the Dest module.
 void IRLinker::linkNamedMDNodes() {
   const NamedMDNode *SrcModFlags = SrcM->getModuleFlagsMetadata();
   for (const NamedMDNode &NMD : SrcM->named_metadata()) {
     // Don't link module flags here. Do them separately.

test/ThinLTO/X86/debuginfo-cu-import.ll

 ; Test to ensure only the necessary DICompileUnit fields are imported
 ; for ThinLTO
 
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/debuginfo-cu-import.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc
 
 ; Don't import enums, macros, retainedTypes or globals lists.
 ; Only import local scope imported entities.
 ; RUN: llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t.index.bc -o - | llvm-dis -o - | FileCheck %s
-; CHECK-NOT: DICompileUnit{{.*}} enums:
-; CHECK-NOT: DICompileUnit{{.*}} macros:
-; CHECK-NOT: DICompileUnit{{.*}} retainedTypes:
-; CHECK-NOT: DICompileUnit{{.*}} globals:
-; CHECK: DICompileUnit{{.*}} imports: ![[IMP:[0-9]+]]
-; CHECK: ![[IMP]] = !{!{{[0-9]+}}}
+; CHECK: DICompileUnit(
+; CHECK-NOT: enums:
+; CHECK-NOT: macros:
+; CHECK-NOT: retainedTypes:
+; CHECK-NOT: globals:
+; CHECK-NOT: imports:
+; CHECK-SAME: FullDebug){{$}}
 
 ; ModuleID = 'debuginfo-cu-import.c'
 source_filename = "debuginfo-cu-import.c"
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 define void @foo() !dbg !28 {
 entry: