[llvm] [ThinLTO] Allow importing based on a workload definition (PR #74545)
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 13 14:22:55 PST 2023
================
@@ -451,76 +451,79 @@ class WorkloadImportsManager : public ModuleImportsManager {
auto &ValueInfos = SetIter->second;
SmallVector<EdgeInfo, 128> GlobWorklist;
for (auto &VI : llvm::make_early_inc_range(ValueInfos)) {
+ auto It = DefinedGVSummaries.find(VI.getGUID());
+ if (It != DefinedGVSummaries.end() &&
+ IsPrevailing(VI.getGUID(), It->second)) {
+ LLVM_DEBUG(
+ dbgs() << "[Workload] " << VI.name()
+ << " has the prevailing variant already in the module "
+ << ModName << ". No need to import\n");
+ continue;
+ }
auto Candidates =
qualifyCalleeCandidates(Index, VI.getSummaryList(), ModName);
const GlobalValueSummary *GVS = nullptr;
- FunctionImporter::ImportFailureReason LastReason =
- FunctionImporter::ImportFailureReason::None;
- for (const auto &Candidate : Candidates) {
- /// We will prefer importing the prevailing candidate, if not, we'll
- /// still pick the first available candidate. The reason we want to make
- /// sure we do import the prevailing candidate is because the goal of
- /// workload-awareness is to enable optimizations specializing the call
- /// graph of that workload. Suppose a function is already defined in the
- /// module, but it's not the prevailing variant. Suppose also we do not
- /// inline it, but we could specialize it to the workload in other ways.
- /// However, the linker would drop it in the favor of the prevailing
- /// copy. Instead, by importing the prevailing variant (assuming also
- /// the use of `-avail-extern-to-local`), we keep the specialization. We
- /// could alteranatively make the non-prevailing variant local, but the
- /// prevailing one is also the one for which we would have previously
- /// collected profiles, making it preferrable.
- LastReason = Candidate.first;
- if (Candidate.first == FunctionImporter::ImportFailureReason::None) {
- const bool Prevailing = IsPrevailing(VI.getGUID(), Candidate.second);
- if (Prevailing || !GVS) {
- if (!GVS && !Prevailing)
- LLVM_DEBUG(dbgs()
- << "[Workload] Considering " << VI.name() << " from "
- << Candidate.second->modulePath() << " with linkage "
- << Candidate.second->linkage()
- << " although it's not prevailing, but it's the "
- "first available candidate.\n");
- GVS = Candidate.second;
- if (Prevailing) {
- LLVM_DEBUG(dbgs()
- << "[Workload] Considering " << VI.name() << " from "
- << GVS->modulePath() << " with linkage "
- << GVS->linkage() << " because it's prevailing.\n");
- break;
- }
- } else {
- LLVM_DEBUG(dbgs() << "[Workload] Skipping " << VI.name() << " from "
- << Candidate.second->modulePath()
- << " with linkage " << Candidate.second->linkage()
- << " because it's not prevailing\n");
- }
- }
- }
- if (!GVS) {
+ auto PotentialCandidates = llvm::map_range(
+ llvm::make_filter_range(
+ Candidates,
+ [&](const auto &Candidate) {
+ LLVM_DEBUG(dbgs() << "[Workflow] Candidate for " << VI.name()
+ << " from " << Candidate.second->modulePath()
+ << " ImportFailureReason: "
+ << getFailureName(Candidate.first) << "\n");
+ return Candidate.first ==
+ FunctionImporter::ImportFailureReason::None;
+ }),
+ [](const auto &Candidate) { return Candidate.second; });
+ if (PotentialCandidates.empty()) {
LLVM_DEBUG(dbgs() << "[Workload] Not importing " << VI.name()
<< " because can't find eligible Callee. Guid is: "
- << Function::getGUID(VI.name())
- << ". The reason was: " << getFailureName(LastReason)
- << "\n");
+ << Function::getGUID(VI.name()) << "\n");
continue;
}
- const auto *CFS = cast<FunctionSummary>(GVS->getBaseObject());
- auto ExportingModule = CFS->modulePath();
+ /// We will prefer importing the prevailing candidate, if not, we'll
+ /// still pick the first available candidate. The reason we want to make
+ /// sure we do import the prevailing candidate is because the goal of
+ /// workload-awareness is to enable optimizations specializing the call
+ /// graph of that workload. Suppose a function is already defined in the
+ /// module, but it's not the prevailing variant. Suppose also we do not
+ /// inline it (in fact, if it were interposable, we can't inline it),
+ /// but we could specialize it to the workload in other ways. However,
+ /// the linker would drop it in the favor of the prevailing copy.
+ /// Instead, by importing the prevailing variant (assuming also the use
+ /// of `-avail-extern-to-local`), we keep the specialization. We could
+ /// alteranatively make the non-prevailing variant local, but the
+ /// prevailing one is also the one for which we would have previously
+ /// collected profiles, making it preferrable.
+ auto PrevailingCandidates = llvm::make_filter_range(
+ PotentialCandidates, [&](const auto *Candidate) {
+ return IsPrevailing(VI.getGUID(), Candidate);
----------------
teresajohnson wrote:
There can only be one, so rather than check every single candidate, just quit when the prevailing copy found. But I have a suggestion above for combining this with potential candidates.
https://github.com/llvm/llvm-project/pull/74545
More information about the llvm-commits
mailing list