[ThinLTO] Recommit of import global variables

This wasreverted in r326638 due to link problems and fixed
afterwards

llvm-svn: 327254

lib/Linker/IRMover.cpp

@@ -1051,14 +1051,10 @@ void IRLinker::prepareCompileUnitsForImport() {
     ValueMap.MD()[CU->getRawEnumTypes()].reset(nullptr);
     ValueMap.MD()[CU->getRawMacros()].reset(nullptr);
     ValueMap.MD()[CU->getRawRetainedTypes()].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");
+    // We import global variables only temporarily in order for instcombine
+    // and globalopt to perform constant folding and static constructor
+    // evaluation. After that elim-avail-extern will covert imported globals
+    // back to declarations, so we don't need debug info for them.
     ValueMap.MD()[CU->getRawGlobalVariables()].reset(nullptr);
 
     // Imported entities only need to be mapped in if they have local

lib/Transforms/IPO/FunctionImport.cpp

@@ -61,6 +61,7 @@ using namespace llvm;
 #define DEBUG_TYPE "function-import"
 
 STATISTIC(NumImportedFunctions, "Number of functions imported");
+STATISTIC(NumImportedGlobalVars, "Number of global variables imported");
 STATISTIC(NumImportedModules, "Number of modules imported from");
 STATISTIC(NumDeadSymbols, "Number of dead stripped symbols in index");
 STATISTIC(NumLiveSymbols, "Number of live symbols in index");
@@ -238,6 +239,32 @@ updateValueInfoForIndirectCalls(const ModuleSummaryIndex &Index, ValueInfo VI) {
   return Index.getValueInfo(GUID);
 }
 
+static void computeImportForReferencedGlobals(
+    const FunctionSummary &Summary, const GVSummaryMapTy &DefinedGVSummaries,
+    FunctionImporter::ImportMapTy &ImportList,
+    StringMap<FunctionImporter::ExportSetTy> *ExportLists) {
+  for (auto &VI : Summary.refs()) {
+    if (DefinedGVSummaries.count(VI.getGUID())) {
+      DEBUG(dbgs() << "Ref ignored! Target already in destination module.\n");
+      continue;
+    }
+
+    DEBUG(dbgs() << " ref -> " << VI.getGUID() << "\n");
+
+    for (auto &RefSummary : VI.getSummaryList())
+      if (RefSummary->getSummaryKind() == GlobalValueSummary::GlobalVarKind &&
+          // Don't try to import regular LTO summaries added to dummy module.
+          !RefSummary->modulePath().empty() &&
+          !GlobalValue::isInterposableLinkage(RefSummary->linkage()) &&
+          RefSummary->refs().empty()) {
+        ImportList[RefSummary->modulePath()][VI.getGUID()] = 1;
+        if (ExportLists)
+          (*ExportLists)[RefSummary->modulePath()].insert(VI.getGUID());
+        break;
+      }
+  }
+}
+
 /// Compute the list of functions to import for a given caller. Mark these
 /// imported functions and the symbols they reference in their source module as
 /// exported from their source module.
@@ -247,6 +274,8 @@ static void computeImportForFunction(
     SmallVectorImpl<EdgeInfo> &Worklist,
     FunctionImporter::ImportMapTy &ImportList,
     StringMap<FunctionImporter::ExportSetTy> *ExportLists = nullptr) {
+  computeImportForReferencedGlobals(Summary, DefinedGVSummaries, ImportList,
+                                    ExportLists);
   for (auto &Edge : Summary.calls()) {
     ValueInfo VI = Edge.first;
     DEBUG(dbgs() << " edge -> " << VI.getGUID() << " Threshold:" << Threshold
@@ -389,6 +418,34 @@ static void ComputeImportForModule(
   }
 }
 
+#ifndef NDEBUG
+static bool isGlobalVarSummary(const ModuleSummaryIndex &Index,
+                               GlobalValue::GUID G) {
+  if (const auto &VI = Index.getValueInfo(G)) {
+    auto SL = VI.getSummaryList();
+    if (!SL.empty())
+      return SL[0]->getSummaryKind() == GlobalValueSummary::GlobalVarKind;
+  }
+  return false;
+}
+
+static GlobalValue::GUID getGUID(GlobalValue::GUID G) { return G; }
+
+static GlobalValue::GUID
+getGUID(const std::pair<const GlobalValue::GUID, unsigned> &P) {
+  return P.first;
+}
+
+template <class T>
+unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, T &Cont) {
+  unsigned NumGVS = 0;
+  for (auto &V : Cont)
+    if (isGlobalVarSummary(Index, getGUID(V)))
+      ++NumGVS;
+  return NumGVS;
+}
+#endif
+
 /// Compute all the import and export for every module using the Index.
 void llvm::ComputeCrossModuleImport(
     const ModuleSummaryIndex &Index,
@@ -426,12 +483,17 @@ void llvm::ComputeCrossModuleImport(
   for (auto &ModuleImports : ImportLists) {
     auto ModName = ModuleImports.first();
     auto &Exports = ExportLists[ModName];
-    DEBUG(dbgs() << "* Module " << ModName << " exports " << Exports.size()
-                 << " functions. Imports from " << ModuleImports.second.size()
-                 << " modules.\n");
+    unsigned NumGVS = numGlobalVarSummaries(Index, Exports);
+        DEBUG(dbgs() << "* Module " << ModName << " exports "
+                     << Exports.size() - NumGVS << " functions and " << NumGVS
+                     << " vars. Imports from "
+                     << ModuleImports.second.size() << " modules.\n");
     for (auto &Src : ModuleImports.second) {
       auto SrcModName = Src.first();
-      DEBUG(dbgs() << " - " << Src.second.size() << " functions imported from "
+      unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
+      DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
+                   << " functions imported from " << SrcModName << "\n");
+      DEBUG(dbgs() << " - " << NumGVSPerMod << " global vars imported from "
                    << SrcModName << "\n");
     }
   }
@@ -439,13 +501,17 @@ void llvm::ComputeCrossModuleImport(
 }
 
 #ifndef NDEBUG
-static void dumpImportListForModule(StringRef ModulePath,
+static void dumpImportListForModule(const ModuleSummaryIndex &Index,
+                                    StringRef ModulePath,
                                     FunctionImporter::ImportMapTy &ImportList) {
   DEBUG(dbgs() << "* Module " << ModulePath << " imports from "
                << ImportList.size() << " modules.\n");
   for (auto &Src : ImportList) {
     auto SrcModName = Src.first();
-    DEBUG(dbgs() << " - " << Src.second.size() << " functions imported from "
+    unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
+    DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
+                 << " functions imported from " << SrcModName << "\n");
+    DEBUG(dbgs() << " - " << NumGVSPerMod << " vars imported from "
                  << SrcModName << "\n");
   }
 }
@@ -465,7 +531,7 @@ void llvm::ComputeCrossModuleImportForModule(
   ComputeImportForModule(FunctionSummaryMap, Index, ImportList);
 
 #ifndef NDEBUG
-  dumpImportListForModule(ModulePath, ImportList);
+  dumpImportListForModule(Index, ModulePath, ImportList);
 #endif
 }
 
@@ -492,7 +558,7 @@ void llvm::ComputeCrossModuleImportForModuleFromIndex(
     ImportList[Summary->modulePath()][GUID] = 1;
   }
 #ifndef NDEBUG
-  dumpImportListForModule(ModulePath, ImportList);
+  dumpImportListForModule(Index, ModulePath, ImportList);
 #endif
 }
 
@@ -770,7 +836,7 @@ Expected<bool> FunctionImporter::importFunctions(
     Module &DestModule, const FunctionImporter::ImportMapTy &ImportList) {
   DEBUG(dbgs() << "Starting import for Module "
                << DestModule.getModuleIdentifier() << "\n");
-  unsigned ImportedCount = 0;
+  unsigned ImportedCount = 0, ImportedGVCount = 0;
 
   IRMover Mover(DestModule);
   // Do the actual import of functions now, one Module at a time
@@ -830,7 +896,7 @@ Expected<bool> FunctionImporter::importFunctions(
       if (Import) {
         if (Error Err = GV.materialize())
           return std::move(Err);
-        GlobalsToImport.insert(&GV);
+        ImportedGVCount += GlobalsToImport.insert(&GV);
       }
     }
     for (GlobalAlias &GA : SrcModule->aliases()) {
@@ -887,9 +953,14 @@ Expected<bool> FunctionImporter::importFunctions(
     NumImportedModules++;
   }
 
-  NumImportedFunctions += ImportedCount;
+  NumImportedFunctions += (ImportedCount - ImportedGVCount);
+  NumImportedGlobalVars += ImportedGVCount;
 
-  DEBUG(dbgs() << "Imported " << ImportedCount << " functions for Module "
+  DEBUG(dbgs() << "Imported " << ImportedCount - ImportedGVCount
+               << " functions for Module " << DestModule.getModuleIdentifier()
+               << "\n");
+  DEBUG(dbgs() << "Imported " << ImportedGVCount
+               << " global variables for Module "
                << DestModule.getModuleIdentifier() << "\n");
   return ImportedCount;
 }

test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll

@@ -0,0 +1,4 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+@baz = local_unnamed_addr constant i32 10, align 4

test/ThinLTO/X86/Inputs/globals-import.ll

@@ -0,0 +1,9 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+@baz = internal constant i32 10, align 4
+
+define linkonce_odr i32 @foo() {
+  %1 = load i32, i32* @baz, align 4
+  ret i32 %1
+}

test/ThinLTO/X86/globals-import-const-fold.ll

@@ -0,0 +1,23 @@
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/globals-import-cf-baz.ll -o %t2.bc
+; RUN: llvm-lto -thinlto-action=thinlink %t1.bc %t2.bc -o %t3.index.bc
+
+; RUN: llvm-lto -thinlto-action=import %t1.bc %t2.bc -thinlto-index=%t3.index.bc
+; RUN: llvm-dis %t1.bc.thinlto.imported.bc -o - | FileCheck --check-prefix=IMPORT %s
+; RUN: llvm-lto -thinlto-action=optimize %t1.bc.thinlto.imported.bc -o %t1.bc.thinlto.opt.bc
+; RUN: llvm-dis %t1.bc.thinlto.opt.bc -o - | FileCheck --check-prefix=OPTIMIZE %s
+
+; IMPORT: @baz = available_externally local_unnamed_addr constant i32 10
+
+; OPTIMIZE:       define i32 @main()
+; OPTIMIZE-NEXT:    ret i32 10
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+@baz = external local_unnamed_addr constant i32, align 4
+
+define i32 @main() local_unnamed_addr {
+  %1 = load i32, i32* @baz, align 4
+  ret i32 %1
+}

test/ThinLTO/X86/globals-import.ll

@@ -0,0 +1,35 @@
+; Test to ensure that we import a single copy of a global variable. This is
+; important when we link in an object file twice, which is normally works when
+; all symbols have either weak or internal linkage. If we import an internal
+; global variable twice it will get promoted in each module, and given the same
+; name as the IR hash will be identical, resulting in multiple defs when linking.
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/globals-import.ll -o %t2.bc
+; RUN: opt -module-summary %p/Inputs/globals-import.ll -o %t2b.bc
+; RUN: llvm-lto -thinlto-action=thinlink %t1.bc %t2.bc %t2b.bc -o %t3.index.bc
+
+; RUN: llvm-lto -thinlto-action=import %t1.bc -thinlto-index=%t3.index.bc
+; RUN: llvm-dis %t1.bc.thinlto.imported.bc -o - | FileCheck --check-prefix=IMPORT %s
+; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.index.bc
+; RUN: llvm-lto -thinlto-action=promote %t2b.bc -thinlto-index=%t3.index.bc
+; RUN: llvm-dis %t2.bc.thinlto.promoted.bc -o - | FileCheck --check-prefix=PROMOTE1 %s
+; RUN: llvm-dis %t2b.bc.thinlto.promoted.bc -o - | FileCheck --check-prefix=PROMOTE2 %s
+
+; IMPORT: @baz.llvm.0 = available_externally hidden constant i32 10, align 4
+
+; PROMOTE1: @baz.llvm.0 = hidden constant i32 10, align 4
+; PROMOTE1: define weak_odr i32 @foo() {
+
+; Second copy of IR object should not have any symbols imported/promoted.
+; PROMOTE2: @baz = internal constant i32 10, align 4
+; PROMOTE2: define available_externally i32 @foo() {
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+declare i32 @foo()
+
+define i32 @main() local_unnamed_addr {
+  %1 = call i32 @foo()
+  ret i32 %1
+}

test/Transforms/FunctionImport/funcimport.ll

@@ -7,7 +7,8 @@
 ; RUN: opt -function-import -stats -print-imports -enable-import-metadata -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
 ; Try again with new pass manager
 ; RUN: opt -passes='function-import' -stats -print-imports -enable-import-metadata -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
-; "-stats" requires +Asserts.
+; RUN: opt -passes='function-import' -debug-only=function-import -enable-import-metadata -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=DUMP
+; "-stats" and "-debug-only" require +Asserts.
 ; REQUIRES: asserts
 
 ; Test import with smaller instruction limit
@@ -80,7 +81,7 @@ 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: @P.llvm.{{.*}} = available_externally hidden global void ()* null
 ; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.
 ; CHECK-DAG: store void ()* @staticfunc2.llvm.{{.*}}, void ()** @P.llvm.
 
@@ -107,6 +108,8 @@ declare void @variadic(...)
 
 ; INSTLIMDEF-DAG: Import globalfunc2
 ; INSTLIMDEF-DAG: 13 function-import - Number of functions imported
+; INSTLIMDEF-DAG: 4 function-import - Number of global variables imported
+
 ; CHECK-DAG: !0 = !{!"{{.*}}/Inputs/funcimport.ll"}
 
 ; The actual GUID values will depend on path to test.
@@ -142,3 +145,9 @@ declare void @variadic(...)
 ; GUID-DAG: GUID {{.*}} is linkoncealias
 ; GUID-DAG: GUID {{.*}} is callfuncptr
 ; GUID-DAG: GUID {{.*}} is linkoncefunc
+
+; DUMP:       Module [[M1:.*]] imports from 1 module
+; DUMP-NEXT:  13 functions imported from [[M2:.*]]
+; DUMP-NEXT:  4 vars imported from [[M2]]
+; DUMP:       Imported 13 functions for Module [[M1]]
+; DUMP-NEXT:  Imported 4 global variables for Module [[M1]]

test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll

@@ -6,7 +6,7 @@
 
 ; Test to make sure importing and dead stripping works in the
 ; case where the target is a local function that also indirectly calls itself.
-; RUN: llvm-lto2 run -save-temps -o %t3 %t.bc %t2.bc -r %t.bc,fptr,plx -r %t.bc,main,plx -r %t2.bc,_Z6updatei,pl -r %t2.bc,fptr,l -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS
+; RUN: llvm-lto2 run -thinlto-threads=1 -save-temps -o %t3 %t.bc %t2.bc -r %t.bc,fptr,plx -r %t.bc,main,plx -r %t2.bc,_Z6updatei,pl -r %t2.bc,fptr,l -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS
 ; Make sure we import the promted indirectly called target
 ; IMPORTS: Import _ZL3foov.llvm.0