[LTO] Simplify APIs and constify (NFC)

[mehdi_amini]

Summary

Multiple APIs were taking a StringMap for the ImportLists containing
the entries for for all the modules while operating on a single entry
for the current module. Instead we can pass the desired ModuleImport
directly. Also some of the APIs were not const, I believe just to be
able to use operator[] on the StringMap.

[tejohnson]

LGTM, thanks. A nit and a comment below.

include/llvm/Transforms/IPO/FunctionImport.h

117 ↗

(On Diff #68117)

Suggest making the argument name here and in EmitImportsFiles "ImportList" to be consistent with some of the other places where a single module's import list is passed.

lib/Transforms/IPO/FunctionImport.cpp

449 ↗

(On Diff #68117)

There's a behavior difference here in that we used to check here and in EmitImportsFiles whether the path was in the map, but with the new handling we will construct a default entry if it wasn't in the map when we do ImportLists[ModulePath]. I don't think this is a big deal though.

[mehdi_amini]

include/llvm/Transforms/IPO/FunctionImport.h

117 ↗

(On Diff #68117)

Done.
I also separately committed a renaming in all FunctionImport.cpp where ImportListsForModule was used multiple times (I initially took it from there).

llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h

 /// should be imported.
 //
 /// \p ModuleToSummariesForIndex will be populated with the needed summaries
 /// from each required module path. Use a std::map instead of StringMap to get
 /// stable order for bitcode emission.
 void gatherImportedSummariesForModule(
     StringRef ModulePath,
     const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
-    const StringMap<FunctionImporter::ImportMapTy> &ImportLists,
+    const FunctionImporter::ImportMapTy &ImportList,
     std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
 
+/// Emit into \p OutputFilename the files module \p ModulePath will import from.
 std::error_code
 EmitImportsFiles(StringRef ModulePath, StringRef OutputFilename,
-                 const StringMap<FunctionImporter::ImportMapTy> &ImportLists);
+                 const FunctionImporter::ImportMapTy &ModuleImports);
 
 /// Resolve WeakForLinker values in \p TheModule based on the information
 /// recorded in the summaries during global summary-based analysis.
 void thinLTOResolveWeakForLinkerModule(Module &TheModule,
                                        const GVSummaryMapTy &DefinedGlobals);
 
 /// Internalize \p TheModule based on the information recorded in the summaries
 /// during global summary-based analysis.
 void thinLTOInternalizeModule(Module &TheModule,
                               const GVSummaryMapTy &DefinedGlobals);
 }
 
 #endif // LLVM_FUNCTIONIMPORT_H

llvm/trunk/lib/LTO/LTO.cpp

public:
   ThinBackendProc(Config &Conf, ModuleSummaryIndex &CombinedIndex,
                   AddStreamFn AddStream,
                   StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries)
       : Conf(Conf), CombinedIndex(CombinedIndex), AddStream(AddStream),
         ModuleToDefinedGVSummaries(ModuleToDefinedGVSummaries) {}
 
   virtual ~ThinBackendProc() {}
   virtual Error start(unsigned Task, MemoryBufferRef MBRef,
-                      StringMap<FunctionImporter::ImportMapTy> &ImportLists,
+                      const FunctionImporter::ImportMapTy &ImportList,
                       MapVector<StringRef, MemoryBufferRef> &ModuleMap) = 0;
   virtual Error wait() = 0;
 };
 
 class InProcessThinBackend : public ThinBackendProc {
   ThreadPool BackendThreadPool;
 
   Optional<Error> Err;
ErrorOr<std::unique_ptr<Module>> MOrErr =
         parseBitcodeFile(MBRef, BackendContext);
     assert(MOrErr && "Unable to load module in thread?");
 
     return thinBackend(Conf, Task, AddStream, **MOrErr, CombinedIndex,
                        ImportList, DefinedGlobals, ModuleMap);
   }
 
   Error start(unsigned Task, MemoryBufferRef MBRef,
-              StringMap<FunctionImporter::ImportMapTy> &ImportLists,
+              const FunctionImporter::ImportMapTy &ImportList,
               MapVector<StringRef, MemoryBufferRef> &ModuleMap) override {
     StringRef ModulePath = MBRef.getBufferIdentifier();
     BackendThreadPool.async(
         [=](MemoryBufferRef MBRef, ModuleSummaryIndex &CombinedIndex,
             const FunctionImporter::ImportMapTy &ImportList,
             GVSummaryMapTy &DefinedGlobals,
             MapVector<StringRef, MemoryBufferRef> &ModuleMap) {
           Error E =
               runThinLTOBackendThread(AddStream, Task, MBRef, CombinedIndex,
                                       ImportList, DefinedGlobals, ModuleMap);
           if (E) {
             std::unique_lock<std::mutex> L(ErrMu);
             if (Err)
               Err = joinErrors(std::move(*Err), std::move(E));
             else
               Err = std::move(E);
           }
         },
-        MBRef, std::ref(CombinedIndex), std::ref(ImportLists[ModulePath]),
+        MBRef, std::ref(CombinedIndex), std::ref(ImportList),
         std::ref(ModuleToDefinedGVSummaries[ModulePath]), std::ref(ModuleMap));
     return Error();
   }
 
   Error wait() override {
     BackendThreadPool.wait();
     if (Err)
       return std::move(*Err);
if (!ParentPath.empty()) {
       if (std::error_code EC = llvm::sys::fs::create_directories(ParentPath))
         llvm::errs() << "warning: could not create directory '" << ParentPath
                      << "': " << EC.message() << '\n';
     }
     return NewPath.str();
   }
 
   Error start(unsigned Task, MemoryBufferRef MBRef,
-              StringMap<FunctionImporter::ImportMapTy> &ImportLists,
+              const FunctionImporter::ImportMapTy &ImportList,
               MapVector<StringRef, MemoryBufferRef> &ModuleMap) override {
     StringRef ModulePath = MBRef.getBufferIdentifier();
     std::string NewModulePath =
         getThinLTOOutputFile(ModulePath, OldPrefix, NewPrefix);
 
     std::error_code EC;
     if (!LinkedObjectsFileName.empty()) {
       if (!LinkedObjectsFile) {
         LinkedObjectsFile = llvm::make_unique<raw_fd_ostream>(
             LinkedObjectsFileName, EC, sys::fs::OpenFlags::F_None);
         if (EC)
           return errorCodeToError(EC);
       }
       *LinkedObjectsFile << NewModulePath << '\n';
     }
 
     std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
     gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
-                                     ImportLists, ModuleToSummariesForIndex);
+                                     ImportList, ModuleToSummariesForIndex);
 
     raw_fd_ostream OS(NewModulePath + ".thinlto.bc", EC,
                       sys::fs::OpenFlags::F_None);
     if (EC)
       return errorCodeToError(EC);
     WriteIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
 
     if (ShouldEmitImportsFiles)
-      return errorCodeToError(EmitImportsFiles(
-          ModulePath, NewModulePath + ".imports", ImportLists));
+      return errorCodeToError(
+          EmitImportsFiles(ModulePath, NewModulePath + ".imports", ImportList));
     return Error();
   }
 
   Error wait() override { return Error(); }
 };
 
 ThinBackend lto::createWriteIndexesThinBackend(std::string OldPrefix,
                                                std::string NewPrefix,
Error LTO::runThinLTO(AddStreamFn AddStream) {
   // GlobalResolution in LTO.h). Task numbers, however, start at
   // ParallelCodeGenParallelismLevel, as tasks 0 through
   // ParallelCodeGenParallelismLevel-1 are reserved for parallel code generation
   // partitions.
   unsigned Task = RegularLTO.ParallelCodeGenParallelismLevel;
   unsigned Partition = 1;
 
   for (auto &Mod : ThinLTO.ModuleMap) {
-    if (Error E = BackendProc->start(Task, Mod.second, ImportLists,
+    if (Error E = BackendProc->start(Task, Mod.second, ImportLists[Mod.first],
                                      ThinLTO.ModuleMap))
       return E;
 
     ++Task;
     ++Partition;
   }
 
   return BackendProc->wait();
 }

llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp

void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
                            ExportLists);
 
   llvm::gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
-                                         ImportLists,
+                                         ImportLists[ModulePath],
                                          ModuleToSummariesForIndex);
 }
 
 /**
  * Emit the list of files needed for importing into module.
  */
 void ThinLTOCodeGenerator::emitImports(StringRef ModulePath,
                                        StringRef OutputName,
                                        ModuleSummaryIndex &Index) {
   auto ModuleCount = Index.modulePaths().size();
 
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
   Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
                            ExportLists);
 
   std::error_code EC;
-  if ((EC = EmitImportsFiles(ModulePath, OutputName, ImportLists)))
+  if ((EC = EmitImportsFiles(ModulePath, OutputName, ImportLists[ModulePath])))
     report_fatal_error(Twine("Failed to open ") + OutputName +
                        " to save imports lists\n");
 }
 
 /**
  * Perform internalization. Index is updated to reflect linkage changes.
  */
 void ThinLTOCodeGenerator::internalize(Module &TheModule,

llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp

 #endif
 }
 
 /// Compute the set of summaries needed for a ThinLTO backend compilation of
 /// \p ModulePath.
 void llvm::gatherImportedSummariesForModule(
     StringRef ModulePath,
     const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
-    const StringMap<FunctionImporter::ImportMapTy> &ImportLists,
+    const FunctionImporter::ImportMapTy &ImportList,
     std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex) {
   // Include all summaries from the importing module.
   ModuleToSummariesForIndex[ModulePath] =
       ModuleToDefinedGVSummaries.lookup(ModulePath);
-  auto ModuleImports = ImportLists.find(ModulePath);
-  if (ModuleImports != ImportLists.end()) {
-    // Include summaries for imports.
-    for (auto &ILI : ModuleImports->second) {
-      auto &SummariesForIndex = ModuleToSummariesForIndex[ILI.first()];
-      const auto &DefinedGVSummaries =
-          ModuleToDefinedGVSummaries.lookup(ILI.first());
-      for (auto &GI : ILI.second) {
-        const auto &DS = DefinedGVSummaries.find(GI.first);
-        assert(DS != DefinedGVSummaries.end() &&
-               "Expected a defined summary for imported global value");
-        SummariesForIndex[GI.first] = DS->second;
-      }
-    }
-  }
-}
+  for (auto &ILI : ImportListForModule) {
 
 /// Emit the files \p ModulePath will import from into \p OutputFilename.
-std::error_code llvm::EmitImportsFiles(
-    StringRef ModulePath, StringRef OutputFilename,
-    const StringMap<FunctionImporter::ImportMapTy> &ImportLists) {
-  auto ModuleImports = ImportLists.find(ModulePath);
+std::error_code
+llvm::EmitImportsFiles(StringRef ModulePath, StringRef OutputFilename,
+                       const FunctionImporter::ImportMapTy &ModuleImports) {
   std::error_code EC;
   raw_fd_ostream ImportsOS(OutputFilename, EC, sys::fs::OpenFlags::F_None);
   if (EC)
     return EC;
-  if (ModuleImports != ImportLists.end())
-    for (auto &ILI : ModuleImports->second)
-      ImportsOS << ILI.first() << "\n";
+  for (auto &ILI : ModuleImports)
   return std::error_code();
 }
 
 /// Fixup WeakForLinker linkages in \p TheModule based on summary analysis.
 void llvm::thinLTOResolveWeakForLinkerModule(
     Module &TheModule, const GVSummaryMapTy &DefinedGlobals) {
   auto updateLinkage = [&](GlobalValue &GV) {
     if (!GlobalValue::isWeakForLinker(GV.getLinkage()))