[PATCH] D15178: FunctionImporter: implement bulk function importing for efficiency

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 3 08:18:37 PST 2015


tejohnson added a comment.

With this approach we consider a function for importing exactly once, and I'm concerned that this will lose opportunities where we might decide to import it after other imports are done (e.g. exposing more hot calls to it). Perhaps when we decide not to import it in the worklist iteration in GetImportList it should also be removed from the CalledFunctions set so that it can be reconsidered when new calls to it are seen.

What was the memory overhead difference with and without this patch? I guess having the materialized functions on all the source modules before importing is ok since they will all eventually be moved into the dest module during linking anyway. But there will still be overhead of keeping all the source modules which have some other overhead.


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:205
@@ -175,1 +204,3 @@
+    // Process the newly imported functions and add callees to the worklist.
+    findExternalCalls(*F, CalledFunctions, Worklist);
 
----------------
How do you ensure that F is materialized in the lazy-loaded SrcModule? With the current code at HEAD it is the dest copy we walk, which was materialized in the source and then copied over during module linking.

Another issue with using the source copy here - the local functions haven't been renamed/promoted, so there may be apparent duplicates in CalledFunctions due to same-named local functions from other modules.



http://reviews.llvm.org/D15178





More information about the llvm-commits mailing list