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

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.

Reviewers: tejohnson, rafael

Subscribers: joker.eph, llvm-commits

Differential Revision: http://reviews.llvm.org/D19096

From: Mehdi Amini <mehdi.amini@apple.com>
llvm-svn: 266948

llvm/include/llvm/Linker/Linker.h

@@ -29,7 +29,10 @@ public:
     None = 0,
     OverrideFromSrc = (1 << 0),
     LinkOnlyNeeded = (1 << 1),
-    InternalizeLinkedSymbols = (1 << 2)
+    InternalizeLinkedSymbols = (1 << 2),
+    /// Don't force link referenced linkonce definitions, import declaration.
+    DontForceLinkLinkonceODR = (1 << 3)
+
   };
 
   Linker(Module &M);

llvm/include/llvm/Transforms/IPO/FunctionImport.h

@@ -48,7 +48,11 @@ public:
       : Index(Index), ModuleLoader(ModuleLoader) {}
 
   /// Import functions in Module \p M based on the supplied import list.
-  bool importFunctions(Module &M, const ImportMapTy &ImportList);
+  /// \p ForceImportReferencedDiscardableSymbols will set the ModuleLinker in
+  /// a mode where referenced discarable symbols in the source modules will be
+  /// imported as well even if they are not present in the ImportList.
+  bool importFunctions(Module &M, const ImportMapTy &ImportList,
+                       bool ForceImportReferencedDiscardableSymbols = false);
 
 private:
   /// The summaries index used to trigger importing.

llvm/lib/Linker/LinkModules.cpp

@@ -45,6 +45,9 @@ class ModuleLinker {
   /// to Add.
   void addLazyFor(GlobalValue &GV, IRMover::ValueAdder Add);
 
+  bool shouldLinkReferencedLinkOnce() {
+    return !(Flags & Linker::DontForceLinkLinkonceODR);
+  }
   bool shouldOverrideFromSrc() { return Flags & Linker::OverrideFromSrc; }
   bool shouldLinkOnlyNeeded() { return Flags & Linker::LinkOnlyNeeded; }
   bool shouldInternalizeLinkedSymbols() {
@@ -413,6 +416,12 @@ bool ModuleLinker::linkIfNeeded(GlobalValue &GV) {
 }
 
 void ModuleLinker::addLazyFor(GlobalValue &GV, IRMover::ValueAdder Add) {
+  if (!shouldLinkReferencedLinkOnce())
+    // For ThinLTO we don't import more than what was required.
+    // The client has to guarantee that the linkonce will be availabe at link
+    // time (by promoting it to weak for instance).
+    return;
+
   // Add these to the internalize list
   if (!GV.hasLinkOnceLinkage())
     return;

llvm/lib/Transforms/IPO/FunctionImport.cpp

@@ -49,6 +49,12 @@ static cl::opt<float>
 static cl::opt<bool> PrintImports("print-imports", cl::init(false), cl::Hidden,
                                   cl::desc("Print imported functions"));
 
+// Temporary allows the function import pass to disable always linking
+// referenced discardable symbols.
+static cl::opt<bool>
+    DontForceImportReferencedDiscardableSymbols("disable-force-link-odr",
+                                                cl::init(false), cl::Hidden);
+
 // Load lazily a module from \p FileName in \p Context.
 static std::unique_ptr<Module> loadFile(const std::string &FileName,
                                         LLVMContext &Context) {
@@ -327,7 +333,8 @@ void llvm::ComputeCrossModuleImportForModule(
 // index.
 //
 bool FunctionImporter::importFunctions(
-    Module &DestModule, const FunctionImporter::ImportMapTy &ImportList) {
+    Module &DestModule, const FunctionImporter::ImportMapTy &ImportList,
+    bool ForceImportReferencedDiscardableSymbols) {
   DEBUG(dbgs() << "Starting import for Module "
                << DestModule.getModuleIdentifier() << "\n");
   unsigned ImportedCount = 0;
@@ -420,8 +427,12 @@ bool FunctionImporter::importFunctions(
                << " from " << SrcModule->getSourceFileName() << "\n";
     }
 
-    if (TheLinker.linkInModule(std::move(SrcModule), Linker::Flags::None,
-                               &GlobalsToImport))
+    // Instruct the linker that the client will take care of linkonce resolution
+    unsigned Flags = Linker::Flags::None;
+    if (!ForceImportReferencedDiscardableSymbols)
+      Flags |= Linker::Flags::DontForceLinkLinkonceODR;
+
+    if (TheLinker.linkInModule(std::move(SrcModule), Flags, &GlobalsToImport))
       report_fatal_error("Function Import: link error");
 
     ImportedCount += GlobalsToImport.size();
@@ -523,7 +534,8 @@ public:
       return loadFile(Identifier, M.getContext());
     };
     FunctionImporter Importer(*Index, ModuleLoader);
-    return Importer.importFunctions(M, ImportList);
+    return Importer.importFunctions(
+        M, ImportList, !DontForceImportReferencedDiscardableSymbols);
   }
 };
 } // anonymous namespace

llvm/test/Linker/funcimport2.ll

@@ -3,7 +3,7 @@
 ; 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
 }

llvm/test/Transforms/FunctionImport/Inputs/funcimport.ll

@@ -99,4 +99,46 @@ 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
+}
+
 

llvm/test/Transforms/FunctionImport/funcimport.ll

@@ -4,14 +4,19 @@
 ; RUN: llvm-lto -thinlto -print-summary-global-ids -o %t3 %t.bc %t2.bc 2>&1 | FileCheck %s --check-prefix=GUID
 
 ; Do the import now
-; RUN: opt -function-import -stats -print-imports -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
+; RUN: opt -disable-force-link-odr -function-import -stats -print-imports -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
 ; "-stats" requires +Asserts.
 ; REQUIRES: asserts
 
 ; Test import with smaller instruction limit
-; RUN: opt -function-import -summary-file %t3.thinlto.bc %t.bc -import-instr-limit=5 -S | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIM5
+; RUN: opt -disable-force-link-odr -function-import -summary-file %t3.thinlto.bc %t.bc -import-instr-limit=5 -S | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIM5
 ; INSTLIM5-NOT: @staticfunc.llvm.
 
+; Test import with smaller instruction limit and without the -disable-force-link-odr
+; RUN: opt -function-import -summary-file %t3.thinlto.bc %t.bc -import-instr-limit=5 -S | FileCheck %s --check-prefix=INSTLIM5ODR
+; INSTLIM5ODR: define linkonce_odr void @linkonceodr()
+
+
 define i32 @main() #0 {
 entry:
   call void (...) @weakalias()
@@ -23,6 +28,7 @@ entry:
   call void (...) @setfuncptr()
   call void (...) @callfuncptr()
   call void (...) @weakfunc()
+  call void (...) @referencelargelinkonce()
   ret i32 0
 }
 
@@ -78,6 +84,12 @@ declare void @callfuncptr(...) #1
 ; 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
@@ -87,7 +99,7 @@ declare void @weakfunc(...) #1
 ; 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

llvm/tools/llvm-link/llvm-link.cpp

@@ -276,8 +276,10 @@ static bool importFunctions(const char *argv0, LLVMContext &Context,
     if (renameModuleForThinLTO(*SrcModule, *Index, &GlobalsToImport))
       return true;
 
-    if (L.linkInModule(std::move(SrcModule), Linker::Flags::None,
-                       &GlobalsToImport))
+    // Instruct the linker to not automatically import linkonce defintion.
+    unsigned Flags = Linker::Flags::DontForceLinkLinkonceODR;
+
+    if (L.linkInModule(std::move(SrcModule), Flags, &GlobalsToImport))
       return false;
   }