wmi created this revision.
wmi added a reviewer: tejohnson.
Herald added subscribers: steven_wu, hiraditya, inglorion.
wmi requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D99554
Files:
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
Index: llvm/lib/LTO/LTOBackend.cpp
===================================================================
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -539,7 +539,7 @@
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 @@
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 @@
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 @@
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;
}
Index: llvm/lib/LTO/LTO.cpp
===================================================================
--- llvm/lib/LTO/LTO.cpp
+++ llvm/lib/LTO/LTO.cpp
@@ -1215,7 +1215,7 @@
return MOrErr.takeError();
return thinBackend(Conf, Task, AddStream, **MOrErr, CombinedIndex,
- ImportList, DefinedGlobals, ModuleMap);
+ ImportList, DefinedGlobals, &ModuleMap);
};
auto ModuleID = BM.getModuleIdentifier();
Index: llvm/include/llvm/LTO/LTOBackend.h
===================================================================
--- llvm/include/llvm/LTO/LTOBackend.h
+++ llvm/include/llvm/LTO/LTOBackend.h
@@ -50,7 +50,7 @@
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 +62,11 @@
/// 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);
}
}
Index: clang/test/CodeGen/thinlto_backend.ll
===================================================================
--- clang/test/CodeGen/thinlto_backend.ll
+++ 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"
Index: clang/lib/CodeGen/BackendUtil.cpp
===================================================================
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1503,10 +1503,7 @@
// 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 @@
if (Error E =
thinBackend(Conf, -1, AddStream, *M, *CombinedIndex, ImportList,
ModuleToDefinedGVSummaries[M->getModuleIdentifier()],
- ModuleMap, CGOpts.CmdArgs)) {
+ nullptr, CGOpts.CmdArgs)) {
handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
errs() << "Error running ThinLTO backend: " << EIB.message() << '\n';
});
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
