[PATCH] D18455: [ThinLTO] Use bulk importing in llvm-link

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: tools/llvm-link/llvm-link.cpp:130
@@ +129,3 @@
+/// Helper to load on demand a Module from file and cache it for subsequent
+/// queries. It can be used with the FunctionImporter.
+class ModuleLazyLoaderCache {
----------------
joker.eph wrote:
> ` It can be used with the FunctionImporter`
> 
> The function importer is not involved here right? Comment may be updated.
> 
Will fix. This was pretty much just cloned with minor tweaks to the arguments from what is in the function importer pass, since we want to do the same thing here.

================
Comment at: tools/llvm-link/llvm-link.cpp:259
@@ +258,3 @@
+  // Do the actual import of globals now, one Module at a time
+  for (auto &GlobalsToImportPerModule : ModuleToGlobalsToImportMap) {
+    // Get the module for the import
----------------
joker.eph wrote:
> Do we actually want bulk import for llvm-link? Maybe doing one import at a time is a "feature", since it not testing the same thing.
> It's probably good for now...
Yes, we do need to do the bulk importing to get the metadata importing to work as expected without post-pass metadata linking.


http://reviews.llvm.org/D18455