[PATCH] D48783: [ThinLTO] Use std::map for import lists to get determistic imports files

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
tejohnson added a reviewer: pcc.
Herald added subscribers: dexonsmith, steven_wu, eraman, inglorion, mehdi_amini.

I noticed that the .imports files emitted for distributed ThinLTO
backends do not have consistent ordering. This is because StringMap
iteration order is not guaranteed to be deterministic. Change to using a
std::map instead. Note that the maps used to generate the module paths
in the emitted individual index files already use std::map
(ModuleToSummariesForIndex passed to llvm::WriteIndexToFile).

This issue is likely causing some unnecessary rebuilds of the ThinLTO
backends in our distributed build system as the imports files are inputs
to those backends.


Repository:
  rL LLVM

https://reviews.llvm.org/D48783

Files:
  include/llvm/Transforms/IPO/FunctionImport.h
  lib/LTO/LTO.cpp
  lib/LTO/ThinLTOCodeGenerator.cpp
  lib/Transforms/IPO/FunctionImport.cpp


Index: lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- lib/Transforms/IPO/FunctionImport.cpp
+++ lib/Transforms/IPO/FunctionImport.cpp
@@ -510,7 +510,7 @@
                       << " vars. Imports from " << ModuleImports.second.size()
                       << " modules.\n");
     for (auto &Src : ModuleImports.second) {
-      auto SrcModName = Src.first();
+      auto SrcModName = Src.first;
       unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
       LLVM_DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
                         << " functions imported from " << SrcModName << "\n");
@@ -528,7 +528,7 @@
   LLVM_DEBUG(dbgs() << "* Module " << ModulePath << " imports from "
                     << ImportList.size() << " modules.\n");
   for (auto &Src : ImportList) {
-    auto SrcModName = Src.first();
+    auto SrcModName = Src.first;
     unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
     LLVM_DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
                       << " functions imported from " << SrcModName << "\n");
@@ -694,9 +694,9 @@
       ModuleToDefinedGVSummaries.lookup(ModulePath);
   // Include summaries for imports.
   for (auto &ILI : ImportList) {
-    auto &SummariesForIndex = ModuleToSummariesForIndex[ILI.first()];
+    auto &SummariesForIndex = ModuleToSummariesForIndex[ILI.first];
     const auto &DefinedGVSummaries =
-        ModuleToDefinedGVSummaries.lookup(ILI.first());
+        ModuleToDefinedGVSummaries.lookup(ILI.first);
     for (auto &GI : ILI.second) {
       const auto &DS = DefinedGVSummaries.find(GI.first);
       assert(DS != DefinedGVSummaries.end() &&
@@ -715,7 +715,7 @@
   if (EC)
     return EC;
   for (auto &ILI : ModuleImports)
-    ImportsOS << ILI.first() << "\n";
+    ImportsOS << ILI.first << "\n";
   return std::error_code();
 }
 
@@ -884,7 +884,7 @@
   // Do the actual import of functions now, one Module at a time
   std::set<StringRef> ModuleNameOrderedList;
   for (auto &FunctionsToImportPerModule : ImportList) {
-    ModuleNameOrderedList.insert(FunctionsToImportPerModule.first());
+    ModuleNameOrderedList.insert(FunctionsToImportPerModule.first);
   }
   for (auto &Name : ModuleNameOrderedList) {
     // Get the module for the import
Index: lib/LTO/ThinLTOCodeGenerator.cpp
===================================================================
--- lib/LTO/ThinLTOCodeGenerator.cpp
+++ lib/LTO/ThinLTOCodeGenerator.cpp
@@ -359,7 +359,7 @@
 
     // Include the hash for every module we import functions from
     for (auto &Entry : ImportList) {
-      auto ModHash = Index.getModuleHash(Entry.first());
+      auto ModHash = Index.getModuleHash(Entry.first);
       Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
     }
 
Index: lib/LTO/LTO.cpp
===================================================================
--- lib/LTO/LTO.cpp
+++ lib/LTO/LTO.cpp
@@ -151,7 +151,7 @@
   // imported symbols for each module may affect code generation and is
   // sensitive to link order, so include that as well.
   for (auto &Entry : ImportList) {
-    auto ModHash = Index.getModuleHash(Entry.first());
+    auto ModHash = Index.getModuleHash(Entry.first);
     Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
 
     AddUint64(Entry.second.size());
@@ -221,7 +221,7 @@
   // so we need to collect their used resolutions as well.
   for (auto &ImpM : ImportList)
     for (auto &ImpF : ImpM.second)
-      AddUsedThings(Index.findSummaryInModule(ImpF.first, ImpM.first()));
+      AddUsedThings(Index.findSummaryInModule(ImpF.first, ImpM.first));
 
   auto AddTypeIdSummary = [&](StringRef TId, const TypeIdSummary &S) {
     AddString(TId);
Index: include/llvm/Transforms/IPO/FunctionImport.h
===================================================================
--- include/llvm/Transforms/IPO/FunctionImport.h
+++ include/llvm/Transforms/IPO/FunctionImport.h
@@ -42,7 +42,7 @@
   /// The map contains an entry for every module to import from, the key being
   /// the module identifier to pass to the ModuleLoader. The value is the set of
   /// functions to import.
-  using ImportMapTy = StringMap<FunctionsToImportTy>;
+  using ImportMapTy = std::map<std::string,FunctionsToImportTy>;
 
   /// The set contains an entry for every global value the module exports.
   using ExportSetTy = std::unordered_set<GlobalValue::GUID>;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48783.153548.patch
Type: text/x-patch
Size: 4479 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180629/c91c9aec/attachment-0001.bin>