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

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In http://reviews.llvm.org/D19556#415035, @joker.eph wrote:

> Is it possible to have some coverage in a test based on llvm-lto (or other standard tools)? 
>  The availability of the gold plugin is rather limited usually.


Ok, looking at the code added here to gold-plugin, I think it makes most sense to move the setup of the ModuleToSummariesForIndex map into a helper method in FunctionImport.cpp. I would leave the actual invocation of WriteIndexToFile here, so that I don't need to add a dependence from Transforms/IPO to libBitWriter (which is doable, won't introduce a circular dependence, but seems somewhat strange). I would also leave the invocation of collectDefinedGVSummariesPerModule and ComputeCrossModuleImport here for now - eventually we will want to do similar optimizations in gold to what you are doing in libLTO, which means refactoring that handling out of libLTO, but that is an orthogonal thing I would like to leave for another day.

Then I can add a similar mechanism to libLTO (similar interface to e.g. ThinLTOGenerator::internalize - take a module and the combined index as input, but then invoke the new mechanism to compute and return the individual index summaries map. Then I can add a mechanism to llvm-lto to invoke it and write out the individual index files, and add a llvm-lto based test.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:283
@@ +282,3 @@
+      SmallVectorImpl<char> &Buffer, const ModuleSummaryIndex &Index,
+      std::map<StringRef, GVSummaryMapTy> *ModuleToSummariesForIndex = nullptr)
+      : BitcodeWriter(Buffer), Index(Index),
----------------
joker.eph wrote:
> Why not use directly the `ImportMapTy` populated by `ComputeCrossModuleImportForModule`?
> That would avoid restructuring the data. If the only thing is the use of std::map instead of StringMap for the purpose of stable ordering, let's change what ComputeCrossModuleImportForModule operates on.
Initially I planned to do exactly that. It required refactoring ImportMapTy out of FunctionImport.h, since I needed to use it in BitcodeWriter.cpp, so I moved it to ModuleSummaryIndex.h and made the necessary changes. I put it back when I realized the following:

But the bigger issue is that it didn't really give me the information I needed:
- 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).
- I also need to emit the summaries for the module we are importing into (see response to your other comment below on why), which are not in the import map, so I would have had to pass in additional info and handle it separately inside the bitcode writer.
- I don't need the other information in the FunctionsToImportTy (the threshold), although that is not a big issue

================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:396
@@ +395,3 @@
+        return ModuleGVSummariesIter == RHS.ModuleGVSummariesIter;
+      } else {
+        // First ensure RHS also writing the full index, and that both are
----------------
joker.eph wrote:
> no else after return.
Will fix.

================
Comment at: tools/gold/gold-plugin.cpp:1247
@@ +1246,3 @@
+      ModuleToSummariesForIndex[InputFile.file().name] =
+          ModuleToDefinedGVSummaries[InputFile.file().name];
+      auto ModuleImports = ImportLists.find(InputFile.file().name);
----------------
joker.eph wrote:
> Why?
Because when we make global optimization decisions here (i.e. promotion, ODR resolution, etc), we will need to note the info (e.g. updated linkage type) in the summaries for this module in its individual backend index, so that the linkage changes can be made in the backend process.

See also the "Individual Module Index Files" overview section in http://lists.llvm.org/pipermail/llvm-dev/2016-April/098272.html.


http://reviews.llvm.org/D19556