[PATCH] D18122: Rework linkInModule(), making it oblivious to ThinLTO

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 15 23:42:57 PDT 2016


joker.eph added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:360
@@ -352,3 +359,3 @@
     if (TheLinker.linkInModule(std::move(SrcModule), Linker::Flags::None,
-                               &Index, &FunctionsToImport))
+                               &FunctionsToImport))
       report_fatal_error("Function Import: link error");
----------------
tejohnson wrote:
> Rename FunctionsToImport to GlobalsToImport here as well?
> 
> Although currently with this change it looks like we will never import a global variable definition since the importer is only adding functions to this list, right? I wonder if there is a performance loss. Will apply this and try with spec.
Yes no import of globals yet, we operate only on functions.  It makes sense only for constant, right? I don't know if we have this information in the summary?


================
Comment at: test/Transforms/FunctionImport/Inputs/funcimport.ll:23
@@ -21,1 +22,3 @@
 
+define void @globalfunc3() #0 {
+entry:
----------------
tejohnson wrote:
> Why was globalfunc3 and staticvar2 added? Don't see anything checking them in the test.
This is a partial leftover from further changes that haven't been submitted yet (testing that we promote only what is imported elsewhere). Will remove from this patch.



http://reviews.llvm.org/D18122





More information about the llvm-commits mailing list