-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[ThinLTO][NFC] Improve performance of addThinLTO
#166067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Avoid the construction of `GUID` when not required. This improves the performance of a LLD `--thinlto-index-only` link of `clang` by ~4-5% on both Windows and Linux.
|
@llvm/pr-subscribers-lto Author: Andrew Ng (nga888) ChangesAvoid the construction of Full diff: https://github.com/llvm/llvm-project/pull/166067.diff 1 Files Affected:
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index b6182222f6f80..23be42f9d60ce 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1076,63 +1076,59 @@ Expected<ArrayRef<SymbolResolution>>
LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
ArrayRef<SymbolResolution> Res) {
llvm::TimeTraceScope timeScope("LTO add thin LTO");
+ const auto BMID = BM.getModuleIdentifier();
ArrayRef<SymbolResolution> ResTmp = Res;
for (const InputFile::Symbol &Sym : Syms) {
assert(!ResTmp.empty());
const SymbolResolution &R = ResTmp.consume_front();
- if (!Sym.getIRName().empty()) {
+ if (!Sym.getIRName().empty() && R.Prevailing) {
auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::getGlobalIdentifier(Sym.getIRName(),
GlobalValue::ExternalLinkage, ""));
- if (R.Prevailing)
- ThinLTO.setPrevailingModuleForGUID(GUID, BM.getModuleIdentifier());
+ ThinLTO.setPrevailingModuleForGUID(GUID, BMID);
}
}
- if (Error Err =
- BM.readSummary(ThinLTO.CombinedIndex, BM.getModuleIdentifier(),
- [&](GlobalValue::GUID GUID) {
- return ThinLTO.isPrevailingModuleForGUID(
- GUID, BM.getModuleIdentifier());
- }))
+ if (Error Err = BM.readSummary(
+ ThinLTO.CombinedIndex, BMID, [&](GlobalValue::GUID GUID) {
+ return ThinLTO.isPrevailingModuleForGUID(GUID, BMID);
+ }))
return Err;
- LLVM_DEBUG(dbgs() << "Module " << BM.getModuleIdentifier() << "\n");
+ LLVM_DEBUG(dbgs() << "Module " << BMID << "\n");
for (const InputFile::Symbol &Sym : Syms) {
assert(!Res.empty());
const SymbolResolution &R = Res.consume_front();
- if (!Sym.getIRName().empty()) {
+ if (!Sym.getIRName().empty() &&
+ (R.Prevailing || R.FinalDefinitionInLinkageUnit)) {
auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::getGlobalIdentifier(Sym.getIRName(),
GlobalValue::ExternalLinkage, ""));
if (R.Prevailing) {
- assert(
- ThinLTO.isPrevailingModuleForGUID(GUID, BM.getModuleIdentifier()));
+ assert(ThinLTO.isPrevailingModuleForGUID(GUID, BMID));
// For linker redefined symbols (via --wrap or --defsym) we want to
// switch the linkage to `weak` to prevent IPOs from happening.
// Find the summary in the module for this very GV and record the new
// linkage so that we can switch it when we import the GV.
if (R.LinkerRedefined)
- if (auto S = ThinLTO.CombinedIndex.findSummaryInModule(
- GUID, BM.getModuleIdentifier()))
+ if (auto S = ThinLTO.CombinedIndex.findSummaryInModule(GUID, BMID))
S->setLinkage(GlobalValue::WeakAnyLinkage);
}
// If the linker resolved the symbol to a local definition then mark it
// as local in the summary for the module we are adding.
if (R.FinalDefinitionInLinkageUnit) {
- if (auto S = ThinLTO.CombinedIndex.findSummaryInModule(
- GUID, BM.getModuleIdentifier())) {
+ if (auto S = ThinLTO.CombinedIndex.findSummaryInModule(GUID, BMID)) {
S->setDSOLocal(true);
}
}
}
}
- if (!ThinLTO.ModuleMap.insert({BM.getModuleIdentifier(), BM}).second)
+ if (!ThinLTO.ModuleMap.insert({BMID, BM}).second)
return make_error<StringError>(
"Expected at most one ThinLTO module per bitcode file",
inconvertibleErrorCode());
@@ -1143,10 +1139,10 @@ LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
// This is a fuzzy name matching where only modules with name containing the
// specified switch values are going to be compiled.
for (const std::string &Name : Conf.ThinLTOModulesToCompile) {
- if (BM.getModuleIdentifier().contains(Name)) {
- ThinLTO.ModulesToCompile->insert({BM.getModuleIdentifier(), BM});
- LLVM_DEBUG(dbgs() << "[ThinLTO] Selecting " << BM.getModuleIdentifier()
- << " to compile\n");
+ if (BMID.contains(Name)) {
+ ThinLTO.ModulesToCompile->insert({BMID, BM});
+ LLVM_DEBUG(dbgs() << "[ThinLTO] Selecting " << BMID << " to compile\n");
+ break;
}
}
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Avoid the construction of `GUID` when not required. This improves the performance of a LLD `--thinlto-index-only` link of `clang` by ~4-5% on both Windows and Linux.
Avoid the construction of `GUID` when not required. This improves the performance of a LLD `--thinlto-index-only` link of `clang` by ~4-5% on both Windows and Linux. (cherry picked from commit 564c3de)
Avoid the construction of
GUIDwhen not required. This improves the performance of a LLD--thinlto-index-onlylink ofclangby ~4-5% on both Windows and Linux.