[ThinLTO] Only promote exported locals as marked in index

Summary:
We have always speculatively promoted all renamable local values
(except const non-address taken variables) for both the exporting
and importing module. We would then internalize them back based on
the ThinLink results if they weren't actually exported. This is
inefficient, and results in unnecessary renames. It also meant we
had to check the non-renamability of a value in the summary, which
was already checked during function importing analysis in the ThinLink.

Made renameModuleForThinLTO (which does the promotion/renaming) instead
use the index when exporting, to avoid unnecessary renames/promotions.
For importing modules, we can simply promoted all values as any local
we import by definition is exported and needs promotion.

This required changes to the method used by the FunctionImport pass
(only invoked from 'opt' for testing) and when invoked from llvm-link,
since neither does a ThinLink. We simply conservatively mark all locals
in the index as promoted, which preserves the current aggressive
promotion behavior.

I also needed to change an llvm-lto based test where we had previously
been aggressively promoting values that weren't importable (aliasees),
but now will not promote.

Reviewers: mehdi_amini

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D26467

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@286871 91177308-0d34-0410-b5e6-96231b3b80d8

lib/LTO/ThinLTOCodeGenerator.cpp

@@ -534,6 +534,7 @@ 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<GVSummaryMapTy> ModuleToDefinedGVSummaries;
   Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
@@ -551,6 +552,20 @@ void ThinLTOCodeGenerator::promote(Module &TheModule,
   thinLTOResolveWeakForLinkerModule(
       TheModule, ModuleToDefinedGVSummaries[ModuleIdentifier]);
 
+  // Convert the preserved symbols set from string to GUID
+  auto GUIDPreservedSymbols = computeGUIDPreservedSymbols(
+      PreservedSymbols, Triple(TheModule.getTargetTriple()));
+
+  // Promote the exported values in the index, so that they are promoted
+  // in the module.
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+    const auto &ExportList = ExportLists.find(ModuleIdentifier);
+    return (ExportList != ExportLists.end() &&
+            ExportList->second.count(GUID)) ||
+           GUIDPreservedSymbols.count(GUID);
+  };
+  thinLTOInternalizeAndPromoteInIndex(Index, isExported);
+
   promoteModule(TheModule, Index);
 }
 

lib/Transforms/IPO/FunctionImport.cpp

@@ -770,6 +770,17 @@ static bool doImportingForModule(Module &M, const ModuleSummaryIndex *Index) {
   ComputeCrossModuleImportForModule(M.getModuleIdentifier(), *Index,
                                     ImportList);
 
+  // Conservatively mark all internal values as promoted. This interface is
+  // only used when doing importing via the function importing pass. The pass
+  // is only enabled when testing importing via the 'opt' tool, which does
+  // not do the ThinLink that would normally determine what values to promote.
+  for (auto &I : *Index) {
+    for (auto &S : I.second) {
+      if (GlobalValue::isLocalLinkage(S->linkage()))
+        S->setLinkage(GlobalValue::ExternalLinkage);
+    }
+  }
+
   // Next we need to promote to global scope and rename any local values that
   // are potentially exported to other modules.
   if (renameModuleForThinLTO(M, *Index, nullptr)) {

lib/Transforms/Utils/FunctionImportUtils.cpp

@@ -72,25 +72,27 @@ bool FunctionImportGlobalProcessing::shouldPromoteLocalToGlobal(
   // a summary in the distributed backend case (only summaries for values
   // importes as defs, not references, are included in the index passed
   // to the distributed backends).
+  if (isPerformingImport()) {
+    // We don't know for sure yet if we are importing this value (as either
+    // a reference or a def), since we are simply walking all values in the
+    // module. But by necessity if we end up importing it and it is local,
+    // it must be promoted, so unconditionally promote all values in the
+    // importing module.
+    return true;
+  }
+
+  // When exporting, consult the index.
   auto Summaries = ImportIndex.findGlobalValueSummaryList(SGV->getGUID());
-  if (Summaries == ImportIndex.end())
-    // Assert that this is an import - we should always have access to the
-    // summary when exporting.
-    assert(isPerformingImport() &&
-           "Missing summary for global value when exporting");
-  else {
-    assert(Summaries->second.size() == 1 && "Local has more than one summary");
-    if (Summaries->second.front()->noRename()) {
-      assert((isModuleExporting() || !GlobalsToImport->count(SGV)) &&
-             "Imported a non-renamable local value");
-      return false;
-    }
+  assert(Summaries != ImportIndex.end() &&
+         "Missing summary for global value when exporting");
+  assert(Summaries->second.size() == 1 && "Local has more than one summary");
+  auto Linkage = Summaries->second.front()->linkage();
+  if (!GlobalValue::isLocalLinkage(Linkage)) {
+    assert(!Summaries->second.front()->noRename());
+    return true;
   }
 
-  // Eventually we only need to promote functions in the exporting module that
-  // are referenced by a potentially exported function (i.e. one that is in the
-  // summary index).
-  return true;
+  return false;
 }
 
 std::string FunctionImportGlobalProcessing::getName(const GlobalValue *SGV,

test/ThinLTO/X86/alias_import.ll

@@ -14,11 +14,11 @@
 ; PROMOTE-DAG: @globalfuncLinkonceAlias = weak alias void (...), bitcast (void ()* @globalfunc to void (...)*)
 ; PROMOTE-DAG: @globalfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @globalfunc to void (...)*)
 ; PROMOTE-DAG: @globalfuncLinkonceODRAlias = weak_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 = weak 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 = weak_odr alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-DAG: @internalfuncAlias = alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+; PROMOTE-DAG: @internalfuncWeakAlias = weak alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+; PROMOTE-DAG: @internalfuncLinkonceAlias = weak alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+; PROMOTE-DAG: @internalfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+; PROMOTE-DAG: @internalfuncLinkonceODRAlias = weak_odr alias void (...), bitcast (void ()* @internalfunc 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 = weak alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
@@ -45,7 +45,7 @@
 
 ; These will be imported, check the linkage/renaming after promotion
 ; PROMOTE-DAG: define void @globalfunc()
-; PROMOTE-DAG: define hidden void @internalfunc.llvm.0()
+; PROMOTE-DAG: define internal void @internalfunc()
 ; PROMOTE-DAG: define weak_odr void @linkonceODRfunc()
 ; PROMOTE-DAG: define weak_odr void @weakODRfunc()
 ; PROMOTE-DAG: define weak void @linkoncefunc()
@@ -95,11 +95,11 @@
 ; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
 ; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
 ; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
 ; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
 ; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakAlias = internal alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
 ; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
@@ -121,7 +121,7 @@
 ; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
 ; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
 ; PROMOTE-INTERNALIZE-DAG: define internal void @globalfunc()
-; PROMOTE-INTERNALIZE-DAG: define internal void @internalfunc.llvm.0()
+; PROMOTE-INTERNALIZE-DAG: define internal void @internalfunc()
 ; PROMOTE-INTERNALIZE-DAG: define internal void @linkonceODRfunc()
 ; PROMOTE-INTERNALIZE-DAG: define internal void @weakODRfunc()
 ; PROMOTE-INTERNALIZE-DAG: define internal void @linkoncefunc()

tools/llvm-link/llvm-link.cpp

@@ -312,6 +312,16 @@ static bool linkFiles(const char *argv0, LLVMContext &Context, Linker &L,
       std::unique_ptr<ModuleSummaryIndex> Index =
           ExitOnErr(llvm::getModuleSummaryIndexForFile(SummaryIndex));
 
+      // Conservatively mark all internal values as promoted, since this tool
+      // does not do the ThinLink that would normally determine what values to
+      // promote.
+      for (auto &I : *Index) {
+        for (auto &S : I.second) {
+          if (GlobalValue::isLocalLinkage(S->linkage()))
+            S->setLinkage(GlobalValue::ExternalLinkage);
+        }
+      }
+
       // Promotion
       if (renameModuleForThinLTO(*M, *Index))
         return true;