[PATCH] D18945: [ThinLTO] Only compute imports for current module in FunctionImport pass

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In http://reviews.llvm.org/D18945#396620, @joker.eph wrote:

> I suspect the most efficient implementation for a distributed is that the ThinLink would provide for each file:
>
> - the list of imports (module id + list of functions)
> - the list of export
>
>   This is exactly what you need to send to the backend, not the index itself.


In some ways, but I'd like to enable more flexibility in the backends (locals have to be handled suitably conservatively if we promote based on the link time decisions).

If we eventually decide to go that route, passing this info from the link step to the backends requires additional/new plumbing (maybe write out a subset of the index with just the desired imports, e.g.).  In that case, the invocation of ComputeCrossModuleImportForModule here goes away completely in that model, and I don't see a need to compute the lists for all modules redundantly until then.


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:313
@@ +312,3 @@
+    }
+  }
+
----------------
joker.eph wrote:
> Should be refactored with `ModuleSummaryIndex::collectDefinedFunctionsPerModule()` after D18908, the code looks close.
Meaning refactor this out into a ModuleSummaryIndex method as you did with the version of this loop from ComputeCrossModuleImport()? Sure, I can do that. Note this version is different than the one from ComputeCrossModuleImport (which D18908 pulls out into collectDefinedFunctionsPerModule) in that it is looking for a specific module path, and is only building a std::map, not StringMap<std::map>.


http://reviews.llvm.org/D18945