[PATCH] D19556: [ThinLTO] Emit individual index files for distributed backends

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:286
@@ +285,3 @@
+      : BitcodeWriter(Buffer), Index(Index),
+        ModuleToSummariesForIndex(ModuleToSummariesForIndex) {
+    // Assign unique value ids to all summaries to be written, for use
----------------
joker.eph wrote:
> > The ImportMapTy is a map to FunctionsToImportTy, which provides the GUID of the functions to import. I would then need to perform an extra step in the bitcode writer to find the corresponding summary object for that module. It was more natural and efficient to get that directly from the ModuleToDefinedGVSummaries map already populated in the client (and in fact this map and ModuleToDefinedGVSummaries use the same GVSummaryMapTy).
> 
> OK I missed this the first time, seems obvious afterward...
> It seems like something that could be changed in whatever `ComputeCrossModuleImportForModule` produces, but we can refactor/change that later (indeed this is the kind of information I was expected to gather from your current series of patches to feed the design a common API for linkers).
> 
> 
Yes, I agree that some of these steps can/should be combined when we refactor into a common library.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:423
@@ +422,3 @@
+/// \p ModulePath.
+void llvm::ComputeSummariesForFile(
+    StringRef ModulePath,
----------------
joker.eph wrote:
> bikeshed: what about `llvm::gatherImportedSummariesForModule()` (i.e. we don't really "compute", and "File" seems odd compare to the usual use of "Module")
Good idea, will switch to that name.


http://reviews.llvm.org/D19556