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

diff --git a/llvm/include/llvm/Linker/Linker.h b/llvm/include/llvm/Linker/Linker.h
index e1605cb..b077c37 100644 (file)
--- a/llvm/include/llvm/Linker/Linker.h
+++ b/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);

diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
index 253c5df..95230ea 100644 (file)
--- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ b/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.

diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp
index cd3cd52..8ed8856 100644 (file)
--- a/llvm/lib/Linker/LinkModules.cpp
+++ b/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;

diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 9d7d78a..25d4130 100644 (file)
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/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

diff --git a/llvm/test/Linker/funcimport2.ll b/llvm/test/Linker/funcimport2.ll
index 6c30dda..bf8f96b 100644 (file)
--- a/llvm/test/Linker/funcimport2.ll
+++ b/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
 }

diff --git a/llvm/test/Transforms/FunctionImport/Inputs/funcimport.ll b/llvm/test/Transforms/FunctionImport/Inputs/funcimport.ll
index 79b766b..03ea364 100644 (file)
--- a/llvm/test/Transforms/FunctionImport/Inputs/funcimport.ll
+++ b/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
+}
+
 

diff --git a/llvm/test/Transforms/FunctionImport/funcimport.ll b/llvm/test/Transforms/FunctionImport/funcimport.ll
index 8e059ca..f0df320 100644 (file)
--- a/llvm/test/Transforms/FunctionImport/funcimport.ll
+++ b/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

diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index 73ac872..0e720bd 100644 (file)
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/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;
   }