ThinLTO/ModuleLinker: add a flag to not always pull-in linkonce when performing importing

[mehdi_amini]

Summary

The function importer already decided what symbols need to be pulled
in. Also these magically added ones will not be in the export list
for the source module, which can confuse the internalizer for
instance.

[mehdi_amini]

test/Transforms/FunctionImport/funcimport.ll

84–85

I'm not happy about the extern_weak, but apparently the IRMover does not have any option to control that, which is annoying.
Rafael, any input on that?

[mehdi_amini]

Update ThinLTO generator to take this into account

[tejohnson]

lib/Linker/LinkModules.cpp

434

Noting down the concern I had expressed on IRC yesterday:

This is only legal if the LinkOnce is being eventually changed to something non-discardable, e.g. with the outstanding patches for libLTO (D18908) and gold (D18674, which applies when launching the backend threads through gold). However, even with those there are currently issues because neither patch handles linkonce that are involved with an alias. Any that will remain linkonce, because they aren't involved with an alias, or aren't covered by one of the two noted patches (e.g. using distributed back ends for ThinLTO), still need to be force imported or there will be failures (because the linkonce is discardable in its original exporting module).

My suggestion for fixing this is:

1. Cover aliases, either by allowing them to be resolved (via the proposed copy technique) or force import them when we collect the imports/exports lists from the index;

and

2. Indicate to the module linker when it is safe to ignore linkonce here, either by noting the new linkage in the index (which needs to be passed in here somehow), or use a flag to tell the module linker that it is safe to ignore linkonce here when importing (only for libLTO for now, when #1 above is addressed).

[mehdi_amini]

Update: resolve alias correctly now. Use a ModuleLinker flag to control the behavior.

[mehdi_amini]

lib/Linker/LinkModules.cpp

434

Yes this is only legal if the client code handles it correctly.
I'd like to remove totally any "magic" in general: it adds coupling and make the design too monolithic/complex. Here the client is passing a list of globals to import, I don't expect the "module linker" to import anything else.

I think we talked a few weeks ago about that: ThinLTO importing should be using only the IRMover, not the Linker.
You were concerned about decision that would be taken in the backend that wouldn't match what the ThinLink decided: this is such a good case. The key point is that you have two different logic for the decision (summary based and non-summary based). This is quite unsafe (and unsound)!

I updated the patch to add a ModuleLinker flag that controls if referenced linkonce are imported or not, and I also improve the handling for aliases.

[tejohnson]

Still think we have a potential issue, see comments embedded in the odr_resolution test case.

include/llvm/Linker/Linker.h

33 ↗

(On Diff #53980)

Suggest as clearer:

/// Don't force link referenced linkonce definitions.

(i.e. "linkonce definitions" instead of "definition ... linkonce")

34 ↗

(On Diff #53980)

s/DontLazyAddLinkone/DontLazyAddLinkonce/ (s/one/once)

lib/LTO/ThinLTOCodeGenerator.cpp

159

Missing space after 'if'.

160

It took me some time to convince myself that this was going to be correct in all cases. Specifically, I was concerned about the case where an imported function referenced an aliasee that remained linkonce (wasn't first def for linker), and we ended up trying to create a linkonce declaration (illegal) due to the change in addLazyFor. Then I saw the code in the IRMover that turns such declarations into ExternalWeak (as you imply in your question to Rafael in the test case, it doesn't need to be extern_weak, but the important point is that it isn't left linkonce_odr, and we will have a copy emitted in one of the modules for the linker to resolve it with).

I think it would be helpful to add a comment here about how this works. I.e. something like "If aliasee that remains linkonce is referenced by an imported function, the IRMover will convert the declaration to ExternalWeak, which will be satisfied at link time with the copy that was resolved to WeakODR."

lib/Linker/LinkModules.cpp

434

Yes this is only legal if the client code handles it correctly.
I'd like to remove totally any "magic" in general: it adds coupling and make the design too
monolithic/complex. Here the client is passing a list of globals to import, I don't expect the
"module linker" to import anything else.

I think we talked a few weeks ago about that: ThinLTO importing should be using only the
IRMover, not the Linker.
You were concerned about decision that would be taken in the backend that wouldn't match what
the ThinLink decided: this is such a good case. The key point is that you have two different logic
for the decision (summary based and non-summary based). This is quite unsafe (and unsound)!

It is unsound once the thin link step starts making optimizations based on the importing decisions, which is a good direction to go in. But first we have to get all of the ThinLTO clients to handle all linkonce cases correctly. This patch is a good start, and similar handling can be added to the gold-plugin and distributed backend cases. Eventually if the resolved linkage is updated in the index in all cases, we can add some assertion checking to ensure that we don't end up importing a reference to something that remains linkonce but doesn't have its definition in the GlobalsToLink set.

test/ThinLTO/X86/odr_resolution.ll

13

Can you remind me how we prevent the alias from being optimized this way? We have AliasSummary in DefinedGlobals, but only the aliasee are added to the GlobalInvolvedWithAlias set. I don't see where it is preventing the ODR resolution for the alias itself. If we did try to do the alias resolution, we would end up with an available_externally alias in MOD2, which is illegal.

If we do in fact prevent the ODR resolution in this case, then I think there is a different issue. If we have a reference to the alias in an imported routine, with the change to addLazyFor() we would no longer import this linkonce alias as a definition, and the IRMover will convert the imported declaration to ExternalWeak. But with all the copies of the defintion remaining linkonce, we aren't guaranteed to emit a copy anywhere, they could all be eliminated after inlining in their original modules, and we have an undef at link time.

[mehdi_amini]

test/ThinLTO/X86/odr_resolution.ll

13

Yes, it is done late in fixupODR() does not iterate over aliases in ThinLTOCodeGenerator, and yeah it is bad...
I'll write a test case with a linkonce_odr alias

[mehdi_amini]

Work in progress: start resolving alias ODR. Add a test that should provide better coverage.

Still need to update the function importer to remove the "magic" there, and think a bit more about every possible case.

[• rafael]

test/ThinLTO/X86/alias_resolution.ll

50

Is this valid? I would expect extern_weak to be only for declarations and aliases are always definitions.

[mehdi_amini]

Should be in better shape now.

[mehdi_amini]

Reintroduce the linker flag I remove inadvertently in the previous patch

[• rafael]

test/ThinLTO/X86/alias_resolution.ll

58

Are we still producing an extern_weak alias? This should be failing the verifier...

[tejohnson]

lib/LTO/ThinLTOCodeGenerator.cpp

230

Doesn't this mean we will end up with available_externally aliases, which would be illegal? I.e. alias was linkonce or weak and is not the first definition for linker, so NewLinkage is available_externally.

lib/Transforms/IPO/FunctionImport.cpp

169–178

The fix to move this check here instead of being later in importFunctions() seems independent from the rest of this patch - can/should it be committed separately?

tools/llvm-link/llvm-link.cpp

280 ↗

(On Diff #54237)

Doesn't the ODR resolution need to be done to set this flag? I don't see it being done in llvm-link.

[mehdi_amini]

lib/LTO/ThinLTOCodeGenerator.cpp

160

(This code was removed)

tools/llvm-link/llvm-link.cpp

280 ↗

(On Diff #54237)

I'm not sure what you'd see to resolve here, this is the mode where llvm-link is instructed on the command line to import a specific list of functions.

[tejohnson]

tools/llvm-link/llvm-link.cpp

280 ↗

(On Diff #54237)

Ah yes, nevermind. Just the other two comments/questions then.

[mehdi_amini]

lib/Transforms/IPO/FunctionImport.cpp

169–178

r266845

[mehdi_amini]

Update, after splitting as much as possible in D19308.

This now only adds the flag to the Linker.

[tejohnson]

Thanks for splitting this out - it made me see what I missed before about where the new flag is being set. See comments below.

Also, should this be marked as dependent on D19308?

lib/Transforms/IPO/FunctionImport.cpp

422

This should be passed down from the client, not hardwired here. E.g. this resolution is not yet done in gold. E.g. pass down to importFunctions() or to FunctionImporter constructor.

test/Linker/funcimport2.ll

6

Looks like this change should be part of D19308 instead?

test/ThinLTO/X86/Inputs/alias_resolution.ll

6

Looks like this was accidentally left in this patch (test/ThinLTO/X86/alias_resolution.ll is gone). Should this test now be part of D19308?

[mehdi_amini]

lib/Transforms/IPO/FunctionImport.cpp

422

Will update

test/Linker/funcimport2.ll

6

I don't think so: it is testing that we don't import foo (we're still seeing the original definition). So it is not about the resolution in D19308

test/ThinLTO/X86/Inputs/alias_resolution.ll

6

Yes,

[tejohnson]

test/Linker/funcimport2.ll

6

Ah, I see. Thought this was testing a linkage change, missed the fact that it is already available_externally in this file.

[mehdi_amini]

lib/Transforms/IPO/FunctionImport.cpp

422

I've been trying to add a flag to ComputeCrossModuleImport to force-import referenced discardable function, but it is very intrusive to handle.
I also tried to do it as a post-pass, but this does not work (it really can't).

Passing the flag to the FunctionImporter is wrong: this conflicts with the Import/Export list, so I don't want to go this route.

At this point, the easiest route is to force turn linkonce into weak unconditionally in clients.

[tejohnson]

lib/Transforms/IPO/FunctionImport.cpp

422

Passing the flag to the FunctionImporter is wrong: this conflicts with the Import/Export list, so I don't want to go this route.

Why? I thought the very reason for adding the flag was because some clients don't currently do the linkonce resolution (and therefore can't do optimization based on the import/export list in the thin link step).

Otherwise there was no reason to add a flag and change addLazyFor to check that instead of whether we are doing importing.

[mehdi_amini]

Thread the flag to the client of the importer, and default it to true for the functionimportpass.

(also remove the wrong alias test case from this patch)

[mehdi_amini]

lib/Transforms/IPO/FunctionImport.cpp

422

I thought the very reason for adding the flag was because some clients don't currently do the linkonce resolution (and therefore can't do optimization based on the import/export list in the thin link step).
Otherwise there was no reason to add a flag and change addLazyFor to check that instead of whether we are doing importing.

That's a good point indeed: I should not add the flag to the ModuleLinker at all... (I am less reluctant to special-case the ModuleLinker, because it is already special-cased all around).

Passing the flag to the FunctionImporter is wrong: this conflicts with the Import/Export list, so I don't want to go this route.

Why?

Here is the 3 components I try to isolate: Linker level resolution, Importing decision, and actual Importing

1. The linker layer is specific (gold plugin, ThinLTOCodeGenerator, etc.), even though at some point I think we could have a common interface that'd call back into the linker somehow.

This layer resolves weak and linkonce (TODO: this is not communicated to the next layer right now), and indicates what can be internalized and what needs to be preserved *outside of the ThinLTO partition*. libLTO (include/llvm-c/lto.h) should implement the API to pass information from this layer to the next one.

2. The ThinLTO importing decision layer, which is the *only* place were you decide what is imported/exported from a module

3. Compute linkage adjustments using information from 1 and 2.

4. The actual IR moving/update, using information from 1/2/3. I see it as a "thin" FunctionImporter that drives the IRMover using the information from the previous layer.

We're not there yet, but going in this direction at least, and I keep fighting the existing machinery to split out these layer.
The ThinLTOCodeGenerator is somehow piling work-around for these, but I hope to be able to refactor it heavily to extract pieces at some point.

This patch is moving something that happens during "4" towards the upper layers, the flag forces to keep this "feature" in the layer 4 (without good reason actually).

[tejohnson]

LGTM thanks

[mehdi_amini]

Rebase/restore the test

[tejohnson]

In D19096#406930, @joker.eph wrote:

Rebase/restore the test

I thought test/ThinLTO/X86/alias_resolution.ll was supposed to move to D19308?

[mehdi_amini]

Oh I moved the test in the wrong patch...

[mehdi_amini]

Re-upload the right patch, remove the wrong test!

[tejohnson]

In D19096#406940, @joker.eph wrote:

Re-upload the right patch, remove the wrong test!

You might have noticed this already but FYI it is still not the right version (looks like an old version)

[mehdi_amini]

Hopefully this is the right one...

[tejohnson]

In D19096#406958, @joker.eph wrote:

Hopefully this is the right one...

LGTM

[mehdi_amini]

test/Transforms/FunctionImport/funcimport.ll

85

This check is broken because of the flag now, needs to add a command line flag probably...

[tejohnson]

test/Transforms/FunctionImport/funcimport.ll

85

Or check for this behavior through one of the llvm-lto based tests in test/ThinLTO/X86, not with an opt test here.

[mehdi_amini]

Update with a command line flag to allows the test to pass.

[mehdi_amini]

Add a check that without the flag, the linkonce_odr is imported as expected

[mehdi_amini]

test/Transforms/FunctionImport/funcimport.ll

85

Already implemented the flag in the meantime, are you OK with the current patch?

[tejohnson]

test/Transforms/FunctionImport/funcimport.ll

85

Yep, thanks.

[mehdi_amini]

r266948

[mehdi_amini]

r266948

lib/LTO/ThinLTOCodeGenerator.cpp

if (llvm::any_of(
             auto Linkage = FuncInfo->summary()->linkage();
             return !GlobalValue::isAvailableExternallyLinkage(Linkage) &&
                    !GlobalValue::isWeakForLinker(Linkage);
           }))
     return false;
   return true;
 }
 
-static GlobalValue::LinkageTypes ResolveODR(const ModuleSummaryIndex &Index,
-                                            StringRef ModuleIdentifier,
-                                            GlobalValue::GUID GUID,
-                                            const GlobalValueSummary &GV) {
+static GlobalValue::LinkageTypes
+ResolveODR(const ModuleSummaryIndex &Index,
+           const FunctionImporter::ExportSetTy &ExportList,
+           StringRef ModuleIdentifier, GlobalValue::GUID GUID,
   auto HasMultipleCopies =
       [&](const GlobalValueInfoList &GVInfo) { return GVInfo.size() > 1; };
 
   auto OriginalLinkage = GV.linkage();
   switch (OriginalLinkage) {
   case GlobalValue::ExternalLinkage:
   case GlobalValue::AvailableExternallyLinkage:
   case GlobalValue::AppendingLinkage:
   case GlobalValue::InternalLinkage:
   case GlobalValue::PrivateLinkage:
   case GlobalValue::ExternalWeakLinkage:
   case GlobalValue::CommonLinkage:
   case GlobalValue::LinkOnceAnyLinkage:
   case GlobalValue::WeakAnyLinkage:
     break;
   case GlobalValue::LinkOnceODRLinkage:
   case GlobalValue::WeakODRLinkage: {
     auto &GVInfo = Index.findGlobalValueInfoList(GUID)->second;
     // We need to emit only one of these, the first module will keep
     // it, but turned into a weak while the others will drop it.
-    if (!HasMultipleCopies(GVInfo))
+    if (!HasMultipleCopies(GVInfo)) {
+      // Exported ODR needs to be promoted to not be discarded
+      if (GlobalValue::isDiscardableIfUnused(OriginalLinkage) &&
+          ExportList.count(GUID))
+        return GlobalValue::WeakODRLinkage;
       break;
+    }
     if (IsFirstDefinitionForLinker(GVInfo, Index, ModuleIdentifier))
       return GlobalValue::WeakODRLinkage;
     else
       return GlobalValue::AvailableExternallyLinkage;
     break;
   }
   }
   return OriginalLinkage;
 }
 
 /// Resolve LinkOnceODR and WeakODR.
 ///
 /// We'd like to drop these function if they are no longer referenced in the
 /// current module. However there is a chance that another module is still
 /// referencing them because of the import. We make sure we always emit at least
 /// one copy.
 static void ResolveODR(
     const ModuleSummaryIndex &Index,
+    const FunctionImporter::ExportSetTy &ExportList,
     const std::map<GlobalValue::GUID, GlobalValueSummary *> &DefinedGlobals,
     StringRef ModuleIdentifier,
     DenseMap<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR) {
   if (Index.modulePaths().size() == 1)
     // Nothing to do if we don't have multiple modules
     return;
 
   // We won't optimize the globals that are referenced by an alias for now
   // Ideally we should turn the alias into a global and duplicate the definition
   // when needed.
   DenseSet<GlobalValueSummary *> GlobalInvolvedWithAlias;
   for (auto &GA : DefinedGlobals) {
     if (auto AS = dyn_cast<AliasSummary>(GA.second))
       GlobalInvolvedWithAlias.insert(&AS->getAliasee());
   }
 
   for (auto &GV : DefinedGlobals) {
     if (GlobalInvolvedWithAlias.count(GV.second))
       continue;
-    auto NewLinkage = ResolveODR(Index, ModuleIdentifier, GV.first, *GV.second);
+    auto NewLinkage =
+        ResolveODR(Index, ExportList, ModuleIdentifier, GV.first, *GV.second);
     if (NewLinkage != GV.second->linkage()) {
       ResolvedODR[GV.first] = NewLinkage;
     }
   }
 }
 
 /// Fixup linkage, see ResolveODR() above.
 void fixupODR(
void fixupODR(
   for (auto &GV : TheModule.globals()) {
     auto NewLinkage = ResolvedODR.find(GV.getGUID());
     if (NewLinkage == ResolvedODR.end())
       continue;
     DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
                  << GV.getLinkage() << " to " << NewLinkage->second << "\n");
     GV.setLinkage(NewLinkage->second);
   }
+  for (auto &GV : TheModule.aliases()) {
+    auto NewLinkage = ResolvedODR.find(GV.getGUID());
+    if (NewLinkage == ResolvedODR.end())
+      continue;
+    DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
+                 << GV.getLinkage() << " to " << NewLinkage->second << "\n");
+    GV.setLinkage(NewLinkage->second);
+  }
 }
 
 static StringMap<MemoryBufferRef>
 generateModuleMap(const std::vector<MemoryBufferRef> &Modules) {
   StringMap<MemoryBufferRef> ModuleMap;
   for (auto &ModuleBuffer : Modules) {
     assert(ModuleMap.find(ModuleBuffer.getBufferIdentifier()) ==
                ModuleMap.end() &&
std::unique_ptr<ModuleSummaryIndex> ThinLTOCodeGenerator::linkCombinedIndex() {
   return CombinedIndex;
 }
 
 /**
  * Perform promotion and renaming of exported internal functions.
  */
 void ThinLTOCodeGenerator::promote(Module &TheModule,
                                    ModuleSummaryIndex &Index) {
+  auto ModuleCount = Index.modulePaths().size();
   auto ModuleIdentifier = TheModule.getModuleIdentifier();
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
       ModuleToDefinedGVSummaries;
   Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
+  // Generate import/export list
+  StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
+  StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
+  ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
+                           ExportLists);
+  auto &ExportList = ExportLists[ModuleIdentifier];
+
   // Resolve the LinkOnceODR, trying to turn them into "available_externally"
   // where possible.
   // This is a compile-time optimization.
   DenseMap<GlobalValue::GUID, GlobalValue::LinkageTypes> ResolvedODR;
-  ResolveODR(Index, ModuleToDefinedGVSummaries[ModuleIdentifier],
+  ResolveODR(Index, ExportList, ModuleToDefinedGVSummaries[ModuleIdentifier],
              ModuleIdentifier, ResolvedODR);
   fixupODR(TheModule, ResolvedODR);
 
   promoteModule(TheModule, Index);
 }
 
 /**
  * Perform cross-module importing for the module identified by ModuleIdentifier.
void ThinLTOCodeGenerator::run() {
   {
     ThreadPool Pool(ThreadCount);
     int count = 0;
     for (auto &ModuleBuffer : Modules) {
       Pool.async([&](int count) {
         LLVMContext Context;
         Context.setDiscardValueNames(LTODiscardValueNames);
         auto ModuleIdentifier = ModuleBuffer.getBufferIdentifier();
+        auto &ExportList = ExportLists[ModuleIdentifier];
 
         DenseMap<GlobalValue::GUID, GlobalValue::LinkageTypes> ResolvedODR;
-        ResolveODR(*Index, ModuleToDefinedGVSummaries[ModuleIdentifier],
+        ResolveODR(*Index, ExportList,
+                   ModuleToDefinedGVSummaries[ModuleIdentifier],
                    ModuleIdentifier, ResolvedODR);
 
         // Parse module now
         auto TheModule = loadModuleFromBuffer(ModuleBuffer, Context, false);
 
         // Save temps: original file.
         if (!SaveTempsDir.empty()) {
           saveTempBitcode(*TheModule, SaveTempsDir, count, ".0.original.bc");

lib/Linker/LinkModules.cpp

bool ModuleLinker::linkIfNeeded(GlobalValue &GV) {
   if (DGV && shouldLinkFromSource(LinkFromSrc, *DGV, GV))
     return true;
   if (LinkFromSrc)
     ValuesToLink.insert(&GV);
   return false;
 }
 
 void ModuleLinker::addLazyFor(GlobalValue &GV, IRMover::ValueAdder Add) {
+  if (isPerformingImport())
+    // For ThinLTO we don't import more than what was required.
+    return;
+
   // Add these to the internalize list
   if (!GV.hasLinkOnceLinkage())
     return;
 
   if (shouldInternalizeLinkedSymbols())
     Internalize.insert(GV.getName());
   Add(GV);
 

lib/Transforms/IPO/FunctionImport.cpp

for (auto &Edge : Summary.calls()) {
 
     auto *CalleeSummary = selectCallee(GUID, Threshold, Index);
     if (!CalleeSummary) {
       DEBUG(dbgs() << "ignored! No qualifying callee with summary found.\n");
       continue;
     }
     // "Resolve" the summary, traversing alias,
     const FunctionSummary *ResolvedCalleeSummary;
-    if (isa<AliasSummary>(CalleeSummary))
+    if (isa<AliasSummary>(CalleeSummary)) {
       ResolvedCalleeSummary = cast<FunctionSummary>(
           &cast<AliasSummary>(CalleeSummary)->getAliasee());
-    else
+      if (!GlobalValue::isLinkOnceODRLinkage(ResolvedCalleeSummary->linkage())) {
+        // Alias can't point to "available_externally". However when we import
+        // linkOnceODR the linkage does not change. So we import the alias
+        // and aliasee only in this case.
+        // FIXME: we should import alias as available_externally *function*, the
+        // destination module does need to know it is an alias.
+        DEBUG(dbgs() << "ignored! Aliasee is not linkonce_odr.\n");
+        continue;
+      }
+    } else
       ResolvedCalleeSummary = cast<FunctionSummary>(CalleeSummary);
 
     assert(ResolvedCalleeSummary->instCount() <= Threshold &&
            "selectCallee() didn't honor the threshold");
 
     auto ExportModulePath = ResolvedCalleeSummary->modulePath();
     auto &ProcessedThreshold = ImportsForModule[ExportModulePath][GUID];
     /// Since the traversal of the call graph is DFS, we can revisit a function
for (auto &GV : SrcModule->aliases()) {
       auto GUID = GV.getGUID();
       auto Import = ImportGUIDs.count(GUID);
       DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing alias " << GUID
                    << " " << GV.getName() << " from "
                    << SrcModule->getSourceFileName() << "\n");
       if (Import) {
         // Alias can't point to "available_externally". However when we import
         // linkOnceODR the linkage does not change. So we import the alias
-        // and aliasee only in this case.
+        // and aliasee only in this case. This has been handled by
+        // computeImportForFunction()
         GlobalObject *GO = GV.getBaseObject();
-        if (!GO->hasLinkOnceODRLinkage())
-          continue;
+        assert(GO->hasLinkOnceODRLinkage() &&
+               "Unexpected alias to a non-linkonceODR in import list");
 #ifndef NDEBUG
         if (!GlobalsToImport.count(GO))
           DEBUG(dbgs() << " alias triggers importing aliasee " << GO->getGUID()
                        << " " << GO->getName() << " from "
                        << SrcModule->getSourceFileName() << "\n");
 #endif
         GO->materialize();
         GlobalsToImport.insert(GO);
#endif
     if (PrintImports) {
       for (const auto *GV : GlobalsToImport)
         dbgs() << DestModule.getSourceFileName() << ": Import " << GV->getName()
                << " from " << SrcModule->getSourceFileName() << "\n";
     }
 
     if (TheLinker.linkInModule(std::move(SrcModule), Linker::Flags::None,
                                &GlobalsToImport))
       report_fatal_error("Function Import: link error");
 
     ImportedCount += GlobalsToImport.size();
   }
 
   NumImported += ImportedCount;
 
   DEBUG(dbgs() << "Imported " << ImportedCount << " functions for Module "
                << DestModule.getModuleIdentifier() << "\n");

test/Linker/funcimport2.ll

 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/funcimport2.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -o %t3 %t1.bc %t2.bc
 ; RUN: llvm-link -import=bar:%t2.bc %t1.bc -summary-index=%t3.thinlto.bc -S | FileCheck %s
 
-; CHECK: define linkonce_odr hidden void @foo() {
+; CHECK: define available_externally hidden void @foo() {
 define available_externally hidden void @foo() {
     ret void
 }
 
 declare void @bar()
 
 define void @caller() {
   call void @bar()
   call void @foo()
   ret void
 }

test/ThinLTO/X86/Inputs/alias_resolution.ll

+
+
+
+
+@globalfuncAlias = alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+@globalfuncWeakAlias = weak alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+@globalfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+@globalfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+@globalfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+define void @globalfunc() {
+entry:
+  ret void
+}
+
+@internalfuncAlias = alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+@internalfuncWeakAlias = weak alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+@internalfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+@internalfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+@internalfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+define internal void @internalfunc() {
+entry:
+  ret void
+}
+
+@linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+@linkonceODRfuncWeakAlias = weak alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+@linkonceODRfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+@linkonceODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+@linkonceODRfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+define linkonce_odr void @linkonceODRfunc() {
+entry:
+  ret void
+}
+
+@weakODRfuncAlias = alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+@weakODRfuncWeakAlias = weak alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+@weakODRfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+@weakODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+@weakODRfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+define weak_odr void @weakODRfunc() {
+entry:
+  ret void
+}
+
+@linkoncefuncAlias = alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+@linkoncefuncWeakAlias = weak alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+@linkoncefuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+@linkoncefuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+@linkoncefuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+define linkonce void @linkoncefunc() {
+entry:
+  ret void
+}
+
+@weakfuncAlias = alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+@weakfuncWeakAlias = weak alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+@weakfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+@weakfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+@weakfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+define weak void @weakfunc() {
+entry:
+  ret void
+}
+

test/ThinLTO/X86/alias_resolution.ll

+ ; Do setup work for all below tests: generate bitcode and combined index
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/alias_resolution.ll -o %t2.bc
+; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc
+; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc %t2.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE
+; RUN: llvm-lto -thinlto-action=import -thinlto-index %t.index.bc %t1.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
+
+; Do not resolve all these aliases
+; PROMOTE-DAG: @globalfuncAlias = alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-DAG: @globalfuncWeakAlias = weak alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-DAG: @globalfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-DAG: @globalfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-DAG: @globalfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-DAG: @internalfuncAlias = alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-DAG: @internalfuncWeakAlias = weak alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-DAG: @internalfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-DAG: @internalfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-DAG: @internalfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-DAG: @linkoncefuncAlias = alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-DAG: @linkoncefuncWeakAlias = weak alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-DAG: @linkoncefuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-DAG: @linkoncefuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-DAG: @linkoncefuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-DAG: @weakfuncAlias = alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-DAG: @weakfuncWeakAlias = weak alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-DAG: @weakfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-DAG: @weakfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-DAG: @weakfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-DAG: @weakODRfuncAlias = alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-DAG: @weakODRfuncWeakAlias = weak alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-DAG: @weakODRfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-DAG: @weakODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-DAG: @weakODRfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+
+; Only alias to LinkonceODR function are resolved
+; (This is a current limitation of the implementation that could be lifted)
+; PROMOTE-DAG: @linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-DAG: @linkonceODRfuncWeakAlias = weak alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-DAG: @linkonceODRfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-DAG: @linkonceODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-DAG: @linkonceODRfuncLinkonceODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+
+; PROMOTE-DAG: define void @globalfunc()
+; PROMOTE-DAG: define hidden void @internalfunc.llvm.0()
+; PROMOTE-DAG: define linkonce_odr void @linkonceODRfunc()
+; PROMOTE-DAG: define weak_odr void @weakODRfunc()
+; PROMOTE-DAG: define linkonce void @linkoncefunc()
+; PROMOTE-DAG: define weak void @weakfunc()
+
+; Only import aliases that have been resolved, i.e. alias to a linkonce_odr.
+; IMPORT-DAG:  @linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; IMPORT-DAG:  @linkonceODRfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; IMPORT-DAG:  @linkonceODRfuncWeakODRAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; IMPORT-DAG:  @linkonceODRfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; IMPORT-DAG:  define linkonce_odr void @linkonceODRfunc()
+; FIXME: not sure why we would want an extern_weak here.
+; IMPORT-DAG:  @linkonceODRfuncWeakAlias = extern_weak alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+
+
+
+; IMPORT-DAG: declare void @globalfuncAlias()
+; IMPORT-DAG: declare void @globalfuncWeakAlias()
+; IMPORT-DAG: declare void @globalfuncLinkonceAlias()
+; IMPORT-DAG: declare void @globalfuncWeakODRAlias()
+; IMPORT-DAG: declare void @globalfuncLinkonceODRAlias()
+; IMPORT-DAG: declare void @internalfuncAlias()
+; IMPORT-DAG: declare void @internalfuncWeakAlias()
+; IMPORT-DAG: declare void @internalfuncLinkonceAlias()
+; IMPORT-DAG: declare void @internalfuncWeakODRAlias()
+; IMPORT-DAG: declare void @internalfuncLinkonceODRAlias()
+; IMPORT-DAG: declare void @weakODRfuncAlias()
+; IMPORT-DAG: declare void @weakODRfuncWeakAlias()
+; IMPORT-DAG: declare void @weakODRfuncLinkonceAlias()
+; IMPORT-DAG: declare void @weakODRfuncWeakODRAlias()
+; IMPORT-DAG: declare void @weakODRfuncLinkonceODRAlias()
+; IMPORT-DAG: declare void @linkoncefuncAlias()
+; IMPORT-DAG: declare void @linkoncefuncWeakAlias()
+; IMPORT-DAG: declare void @linkoncefuncLinkonceAlias()
+; IMPORT-DAG: declare void @linkoncefuncWeakODRAlias()
+; IMPORT-DAG: declare void @linkoncefuncLinkonceODRAlias()
+; IMPORT-DAG: declare void @weakfuncAlias()
+; IMPORT-DAG: declare void @weakfuncWeakAlias()
+; IMPORT-DAG: declare void @weakfuncLinkonceAlias()
+; IMPORT-DAG: declare void @weakfuncWeakODRAlias()
+; IMPORT-DAG: declare void @weakfuncLinkonceODRAlias()
+
+
+
+
+
+
+define i32 @main() #0 {
+entry:
+  call void @globalfuncAlias()
+  call void @globalfuncWeakAlias()
+  call void @globalfuncLinkonceAlias()
+  call void @globalfuncWeakODRAlias()
+  call void @globalfuncLinkonceODRAlias()
+
+  call void @internalfuncAlias()
+  call void @internalfuncWeakAlias()
+  call void @internalfuncLinkonceAlias()
+  call void @internalfuncWeakODRAlias()
+  call void @internalfuncLinkonceODRAlias()
+  call void @linkonceODRfuncAlias()
+  call void @linkonceODRfuncWeakAlias()
+  call void @linkonceODRfuncLinkonceAlias()
+  call void @linkonceODRfuncWeakODRAlias()
+  call void @linkonceODRfuncLinkonceODRAlias()
+
+  call void @weakODRfuncAlias()
+  call void @weakODRfuncWeakAlias()
+  call void @weakODRfuncLinkonceAlias()
+  call void @weakODRfuncWeakODRAlias()
+  call void @weakODRfuncLinkonceODRAlias()
+
+  call void @linkoncefuncAlias()
+  call void @linkoncefuncWeakAlias()
+  call void @linkoncefuncLinkonceAlias()
+  call void @linkoncefuncWeakODRAlias()
+  call void @linkoncefuncLinkonceODRAlias()
+
+  call void @weakfuncAlias()
+  call void @weakfuncWeakAlias()
+  call void @weakfuncLinkonceAlias()
+  call void @weakfuncWeakODRAlias()
+  call void @weakfuncLinkonceODRAlias()
+
+  ret i32 0
+}
+
+
+declare void @globalfuncAlias()
+declare void @globalfuncWeakAlias()
+declare void @globalfuncLinkonceAlias()
+declare void @globalfuncWeakODRAlias()
+declare void @globalfuncLinkonceODRAlias()
+
+declare void @internalfuncAlias()
+declare void @internalfuncWeakAlias()
+declare void @internalfuncLinkonceAlias()
+declare void @internalfuncWeakODRAlias()
+declare void @internalfuncLinkonceODRAlias()
+
+declare void @linkonceODRfuncAlias()
+declare void @linkonceODRfuncWeakAlias()
+declare void @linkonceODRfuncLinkonceAlias()
+declare void @linkonceODRfuncWeakODRAlias()
+declare void @linkonceODRfuncLinkonceODRAlias()
+
+declare void @weakODRfuncAlias()
+declare void @weakODRfuncWeakAlias()
+declare void @weakODRfuncLinkonceAlias()
+declare void @weakODRfuncWeakODRAlias()
+declare void @weakODRfuncLinkonceODRAlias()
+
+declare void @linkoncefuncAlias()
+declare void @linkoncefuncWeakAlias()
+declare void @linkoncefuncLinkonceAlias()
+declare void @linkoncefuncWeakODRAlias()
+declare void @linkoncefuncLinkonceODRAlias()
+
+declare void @weakfuncAlias()
+declare void @weakfuncWeakAlias()
+declare void @weakfuncLinkonceAlias()
+declare void @weakfuncWeakODRAlias()
+declare void @weakfuncLinkonceODRAlias()
+
+

test/ThinLTO/X86/odr_resolution.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: opt -module-summary %s -o %t.bc
 ; RUN: opt -module-summary %p/Inputs/odr_resolution.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
 
 ; Verify that only one ODR is selected across modules, but non ODR are not affected.
 ; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=MOD1
 ; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=MOD2
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 ; Alias are not optimized
-; MOD1: @linkoncealias = linkonce_odr alias void (), void ()* @linkonceodrfuncwithalias
-; MOD2: @linkoncealias = linkonce_odr alias void (), void ()* @linkonceodrfuncwithalias
+; MOD1: @linkoncealias = weak_odr alias void (), void ()* @linkonceodrfuncwithalias
+; MOD2: @linkoncealias = available_externally alias void (), void ()* @linkonceodrfuncwithalias
 @linkoncealias = linkonce_odr alias void (), void ()* @linkonceodrfuncwithalias
 
 ; Function with an alias are not optimized
 ; MOD1: define linkonce_odr void @linkonceodrfuncwithalias()
 ; MOD2: define linkonce_odr void @linkonceodrfuncwithalias()
 define linkonce_odr void @linkonceodrfuncwithalias() #0 {
 entry:
   ret void

test/Transforms/FunctionImport/Inputs/funcimport.ll

entry:
   ret void
 }
 
 define internal void @staticfunc2() #0 {
 entry:
   ret void
 }
 
+define void @referencelargelinkonce() #0 {
+entry:
+  call void @linkonceodr()
+  ret void
+}
+
+; A large enough linkonce_odr function that should never be imported
+define linkonce_odr void @linkonceodr() #0 {
+entry:
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  call void @globalfunc2()
+  ret void
+}
+
 

test/Transforms/FunctionImport/funcimport.ll

entry:
   call void (...) @analias()
   call void (...) @linkoncealias()
   %call = call i32 (...) @referencestatics()
   %call1 = call i32 (...) @referenceglobals()
   %call2 = call i32 (...) @referencecommon()
   call void (...) @setfuncptr()
   call void (...) @callfuncptr()
   call void (...) @weakfunc()
+  call void (...) @referencelargelinkonce()
   ret i32 0
 }
 
 ; Won't import weak alias
 ; CHECK-DAG: declare void @weakalias
 declare void @weakalias(...) #1
 
 ; Cannot create an alias to available_externally

 declare void @callfuncptr(...) #1
 
 ; Ensure that all uses of local variable @P which has used in setfuncptr
 ; and callfuncptr are to the same promoted/renamed global.
 ; CHECK-DAG: @P.llvm.{{.*}} = external hidden global void ()*
 ; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.
 ; CHECK-DAG: store void ()* @staticfunc2.llvm.{{.*}}, void ()** @P.llvm.
 
+; Ensure that @referencelargelinkonce definition is pulled in, but later we
+; also check that the linkonceodr function is not.
+; CHECK-DAG: define available_externally void @referencelargelinkonce()
+; INSTLIM5-DAG: declare void @linkonceodr()
+declare void @referencelargelinkonce(...)
+
 ; Won't import weak func
 ; CHECK-DAG: declare void @weakfunc(...)
 declare void @weakfunc(...) #1
 
 ; INSTLIMDEF-DAG: Import funcwithpersonality
 ; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.{{.*}}() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
 ; INSTLIM5-DAG: declare hidden void @funcwithpersonality.llvm.{{.*}}()
 
 ; INSTLIMDEF-DAG: Import globalfunc2
-; INSTLIMDEF-DAG: 11 function-import - Number of functions imported
+; INSTLIMDEF-DAG: 13 function-import - Number of functions imported
 
 ; The actual GUID values will depend on path to test.
 ; GUID-DAG: GUID {{.*}} is weakalias
 ; GUID-DAG: GUID {{.*}} is referenceglobals
 ; GUID-DAG: GUID {{.*}} is weakfunc
 ; GUID-DAG: GUID {{.*}} is main
 ; GUID-DAG: GUID {{.*}} is referencecommon
 ; GUID-DAG: GUID {{.*}} is analias