[llvm] d535a05 - [ThinLTO] During module importing, close one source module before open

Wei Mi via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 30 14:42:08 PDT 2021


Author: Wei Mi
Date: 2021-03-30T14:37:29-07:00
New Revision: d535a05ca1a6b959399762d5e14efde1fcfe6af8

URL: https://github.com/llvm/llvm-project/commit/d535a05ca1a6b959399762d5e14efde1fcfe6af8
DIFF: https://github.com/llvm/llvm-project/commit/d535a05ca1a6b959399762d5e14efde1fcfe6af8.diff

LOG: [ThinLTO] During module importing, close one source module before open
another one for distributed mode.

Currently during module importing, ThinLTO opens all the source modules,
collect functions to be imported and append them to the destination module,
then leave all the modules open through out the lto backend pipeline. This
patch refactors it in the way that one source module will be closed before
another source module is opened. All the source modules will be closed after
importing phase is done. It will save some amount of memory when there are
many source modules to be imported.

Note that this patch only changes the distributed thinlto mode. For in
process thinlto mode, one source module is shared acorss different thinlto
backend threads so it is not changed in this patch.

Differential Revision: https://reviews.llvm.org/D99554

Added: 
    

Modified: 
    clang/lib/CodeGen/BackendUtil.cpp
    clang/test/CodeGen/thinlto_backend.ll
    llvm/include/llvm/LTO/LTOBackend.h
    llvm/lib/LTO/LTO.cpp
    llvm/lib/LTO/LTOBackend.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 6de482ea74f52..41eafd13d97c3 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1503,10 +1503,7 @@ static void runThinLTOBackend(
   // we should only invoke this using the individual indexes written out
   // via a WriteIndexesThinBackend.
   FunctionImporter::ImportMapTy ImportList;
-  std::vector<std::unique_ptr<llvm::MemoryBuffer>> OwnedImports;
-  MapVector<llvm::StringRef, llvm::BitcodeModule> ModuleMap;
-  if (!lto::loadReferencedModules(*M, *CombinedIndex, ImportList, ModuleMap,
-                                  OwnedImports))
+  if (!lto::initImportList(*M, *CombinedIndex, ImportList))
     return;
 
   auto AddStream = [&](size_t Task) {
@@ -1583,7 +1580,7 @@ static void runThinLTOBackend(
   if (Error E =
           thinBackend(Conf, -1, AddStream, *M, *CombinedIndex, ImportList,
                       ModuleToDefinedGVSummaries[M->getModuleIdentifier()],
-                      ModuleMap, CGOpts.CmdArgs)) {
+                      /* ModuleMap */ nullptr, CGOpts.CmdArgs)) {
     handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
       errs() << "Error running ThinLTO backend: " << EIB.message() << '\n';
     });

diff  --git a/clang/test/CodeGen/thinlto_backend.ll b/clang/test/CodeGen/thinlto_backend.ll
index 715ff1ec229ea..c8b840e400066 100644
--- a/clang/test/CodeGen/thinlto_backend.ll
+++ b/clang/test/CodeGen/thinlto_backend.ll
@@ -47,7 +47,7 @@
 ; Ensure we get expected error for input files without summaries
 ; RUN: opt -o %t2.o %s
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-ERROR2
-; CHECK-ERROR2: Error loading imported file '{{.*}}': Could not find module summary
+; CHECK-ERROR2: Error loading imported file {{.*}}: Could not find module summary
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

diff  --git a/llvm/include/llvm/LTO/LTOBackend.h b/llvm/include/llvm/LTO/LTOBackend.h
index 32b7984787b01..de89f4bb10dff 100644
--- a/llvm/include/llvm/LTO/LTOBackend.h
+++ b/llvm/include/llvm/LTO/LTOBackend.h
@@ -46,11 +46,16 @@ Error backend(const Config &C, AddStreamFn AddStream,
               ModuleSummaryIndex &CombinedIndex);
 
 /// Runs a ThinLTO backend.
+/// If \p ModuleMap is not nullptr, all the module files to be imported have
+/// already been mapped to memory and the corresponding BitcodeModule objects
+/// are saved in the ModuleMap. If \p ModuleMap is nullptr, module files will
+/// be mapped to memory on demand and at any given time during importing, only
+/// one source module will be kept open at the most.
 Error thinBackend(const Config &C, unsigned Task, AddStreamFn AddStream,
                   Module &M, const ModuleSummaryIndex &CombinedIndex,
                   const FunctionImporter::ImportMapTy &ImportList,
                   const GVSummaryMapTy &DefinedGlobals,
-                  MapVector<StringRef, BitcodeModule> &ModuleMap,
+                  MapVector<StringRef, BitcodeModule> *ModuleMap,
                   const std::vector<uint8_t> &CmdArgs = std::vector<uint8_t>());
 
 Error finalizeOptimizationRemarks(
@@ -62,15 +67,11 @@ BitcodeModule *findThinLTOModule(MutableArrayRef<BitcodeModule> BMs);
 /// Variant of the above.
 Expected<BitcodeModule> findThinLTOModule(MemoryBufferRef MBRef);
 
-/// Distributed ThinLTO: load the referenced modules, keeping their buffers
-/// alive in the provided OwnedImportLifetimeManager. Returns false if the
+/// Distributed ThinLTO: collect the referenced modules based on
+/// module summary and initialize ImportList. Returns false if the
 /// operation failed.
-bool loadReferencedModules(
-    const Module &M, const ModuleSummaryIndex &CombinedIndex,
-    FunctionImporter::ImportMapTy &ImportList,
-    MapVector<llvm::StringRef, llvm::BitcodeModule> &ModuleMap,
-    std::vector<std::unique_ptr<llvm::MemoryBuffer>>
-        &OwnedImportsLifetimeManager);
+bool initImportList(const Module &M, const ModuleSummaryIndex &CombinedIndex,
+                    FunctionImporter::ImportMapTy &ImportList);
 }
 }
 

diff  --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 3cd8c78c42e6f..f81e911acbc1e 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1215,7 +1215,7 @@ class InProcessThinBackend : public ThinBackendProc {
         return MOrErr.takeError();
 
       return thinBackend(Conf, Task, AddStream, **MOrErr, CombinedIndex,
-                         ImportList, DefinedGlobals, ModuleMap);
+                         ImportList, DefinedGlobals, &ModuleMap);
     };
 
     auto ModuleID = BM.getModuleIdentifier();

diff  --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index d49cdd596fee0..c81f08d04925e 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -539,7 +539,7 @@ Error lto::thinBackend(const Config &Conf, unsigned Task, AddStreamFn AddStream,
                        Module &Mod, const ModuleSummaryIndex &CombinedIndex,
                        const FunctionImporter::ImportMapTy &ImportList,
                        const GVSummaryMapTy &DefinedGlobals,
-                       MapVector<StringRef, BitcodeModule> &ModuleMap,
+                       MapVector<StringRef, BitcodeModule> *ModuleMap,
                        const std::vector<uint8_t> &CmdArgs) {
   Expected<const Target *> TOrErr = initAndLookupTarget(Conf, Mod);
   if (!TOrErr)
@@ -608,11 +608,35 @@ Error lto::thinBackend(const Config &Conf, unsigned Task, AddStreamFn AddStream,
   auto ModuleLoader = [&](StringRef Identifier) {
     assert(Mod.getContext().isODRUniquingDebugTypes() &&
            "ODR Type uniquing should be enabled on the context");
-    auto I = ModuleMap.find(Identifier);
-    assert(I != ModuleMap.end());
-    return I->second.getLazyModule(Mod.getContext(),
-                                   /*ShouldLazyLoadMetadata=*/true,
-                                   /*IsImporting*/ true);
+    if (ModuleMap) {
+      auto I = ModuleMap->find(Identifier);
+      assert(I != ModuleMap->end());
+      return I->second.getLazyModule(Mod.getContext(),
+                                     /*ShouldLazyLoadMetadata=*/true,
+                                     /*IsImporting*/ true);
+    }
+
+    ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MBOrErr =
+        llvm::MemoryBuffer::getFile(Identifier);
+    if (!MBOrErr)
+      return Expected<std::unique_ptr<llvm::Module>>(make_error<StringError>(
+          Twine("Error loading imported file ") + Identifier + " : ",
+          MBOrErr.getError()));
+
+    Expected<BitcodeModule> BMOrErr = findThinLTOModule(**MBOrErr);
+    if (!BMOrErr)
+      return Expected<std::unique_ptr<llvm::Module>>(make_error<StringError>(
+          Twine("Error loading imported file ") + Identifier + " : " +
+              toString(BMOrErr.takeError()),
+          inconvertibleErrorCode()));
+
+    Expected<std::unique_ptr<Module>> MOrErr =
+        BMOrErr->getLazyModule(Mod.getContext(),
+                               /*ShouldLazyLoadMetadata=*/true,
+                               /*IsImporting*/ true);
+    if (MOrErr)
+      (*MOrErr)->setOwnedMemoryBuffer(std::move(*MBOrErr));
+    return MOrErr;
   };
 
   FunctionImporter Importer(CombinedIndex, ModuleLoader,
@@ -652,12 +676,9 @@ Expected<BitcodeModule> lto::findThinLTOModule(MemoryBufferRef MBRef) {
                                  inconvertibleErrorCode());
 }
 
-bool lto::loadReferencedModules(
-    const Module &M, const ModuleSummaryIndex &CombinedIndex,
-    FunctionImporter::ImportMapTy &ImportList,
-    MapVector<llvm::StringRef, llvm::BitcodeModule> &ModuleMap,
-    std::vector<std::unique_ptr<llvm::MemoryBuffer>>
-        &OwnedImportsLifetimeManager) {
+bool lto::initImportList(const Module &M,
+                         const ModuleSummaryIndex &CombinedIndex,
+                         FunctionImporter::ImportMapTy &ImportList) {
   if (ThinLTOAssumeMerged)
     return true;
   // We can simply import the values mentioned in the combined index, since
@@ -678,26 +699,5 @@ bool lto::loadReferencedModules(
       ImportList[Summary->modulePath()].insert(GUID);
     }
   }
-
-  for (auto &I : ImportList) {
-    ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MBOrErr =
-        llvm::MemoryBuffer::getFile(I.first());
-    if (!MBOrErr) {
-      errs() << "Error loading imported file '" << I.first()
-             << "': " << MBOrErr.getError().message() << "\n";
-      return false;
-    }
-
-    Expected<BitcodeModule> BMOrErr = findThinLTOModule(**MBOrErr);
-    if (!BMOrErr) {
-      handleAllErrors(BMOrErr.takeError(), [&](ErrorInfoBase &EIB) {
-        errs() << "Error loading imported file '" << I.first()
-               << "': " << EIB.message() << '\n';
-      });
-      return false;
-    }
-    ModuleMap.insert({I.first(), *BMOrErr});
-    OwnedImportsLifetimeManager.push_back(std::move(*MBOrErr));
-  }
   return true;
 }


        


More information about the llvm-commits mailing list