[llvm] [NFC] ProfileSummaryInfo: use analysis manager mechanisms for invalidation (PR #86086)
Mircea Trofin via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 21 16:20:00 PDT 2024
https://github.com/mtrofin updated https://github.com/llvm/llvm-project/pull/86086
>From 9bf75d54f6a4fff949de2bc1c70150e1a2fb0a51 Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Wed, 20 Mar 2024 22:14:59 -0700
Subject: [PATCH 1/2] [NFC] ProfileSummaryInfo: use analysis manager mechanisms
for invalidation
The `refresh` API is doing what analysis managers' `invalidate` does. No reason to do one thing two different ways.
---
llvm/include/llvm/Analysis/ProfileSummaryInfo.h | 7 ++-----
llvm/lib/Analysis/ProfileSummaryInfo.cpp | 10 +++++-----
llvm/lib/Transforms/IPO/SampleProfile.cpp | 14 +++++++-------
.../Instrumentation/PGOInstrumentation.cpp | 16 ++++++++++++----
...new-pm-thinlto-postlink-samplepgo-defaults.ll | 2 +-
.../new-pm-thinlto-prelink-samplepgo-defaults.ll | 2 +-
6 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/llvm/include/llvm/Analysis/ProfileSummaryInfo.h b/llvm/include/llvm/Analysis/ProfileSummaryInfo.h
index 73be9e1d74a33b..587bb17c9d1cb4 100644
--- a/llvm/include/llvm/Analysis/ProfileSummaryInfo.h
+++ b/llvm/include/llvm/Analysis/ProfileSummaryInfo.h
@@ -41,7 +41,7 @@ class MachineFunction;
// units. This would require making this depend on BFI.
class ProfileSummaryInfo {
private:
- const Module *M;
+ const Module &M;
std::unique_ptr<ProfileSummary> Summary;
void computeThresholds();
// Count thresholds to answer isHotCount and isColdCount queries.
@@ -61,12 +61,9 @@ class ProfileSummaryInfo {
mutable DenseMap<int, uint64_t> ThresholdCache;
public:
- ProfileSummaryInfo(const Module &M) : M(&M) { refresh(); }
+ ProfileSummaryInfo(const Module &M);
ProfileSummaryInfo(ProfileSummaryInfo &&Arg) = default;
- /// If no summary is present, attempt to refresh.
- void refresh();
-
/// Returns true if profile summary is available.
bool hasProfileSummary() const { return Summary != nullptr; }
diff --git a/llvm/lib/Analysis/ProfileSummaryInfo.cpp b/llvm/lib/Analysis/ProfileSummaryInfo.cpp
index fdad14571dfe4f..68cc38b4113535 100644
--- a/llvm/lib/Analysis/ProfileSummaryInfo.cpp
+++ b/llvm/lib/Analysis/ProfileSummaryInfo.cpp
@@ -47,17 +47,17 @@ static cl::opt<double> PartialSampleProfileWorkingSetSizeScaleFactor(
// any backend passes (IR level instrumentation, for example). This method
// checks if the Summary is null and if so checks if the summary metadata is now
// available in the module and parses it to get the Summary object.
-void ProfileSummaryInfo::refresh() {
- if (hasProfileSummary())
- return;
+ProfileSummaryInfo::ProfileSummaryInfo(const Module &M) : M(M) {
+ assert(!hasProfileSummary());
+
// First try to get context sensitive ProfileSummary.
- auto *SummaryMD = M->getProfileSummary(/* IsCS */ true);
+ auto *SummaryMD = M.getProfileSummary(/* IsCS */ true);
if (SummaryMD)
Summary.reset(ProfileSummary::getFromMD(SummaryMD));
if (!hasProfileSummary()) {
// This will actually return PSK_Instr or PSK_Sample summary.
- SummaryMD = M->getProfileSummary(/* IsCS */ false);
+ SummaryMD = M.getProfileSummary(/* IsCS */ false);
if (SummaryMD)
Summary.reset(ProfileSummary::getFromMD(SummaryMD));
}
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index 9a8040bc4b064e..9a51be902e0d84 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -43,6 +43,7 @@
#include "llvm/Analysis/ReplayInlineAdvisor.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/Analysis/TargetTransformInfo.h"
+#include "llvm/IR/Analysis.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/DebugLoc.h"
#include "llvm/IR/DiagnosticInfo.h"
@@ -587,8 +588,7 @@ class SampleProfileLoader final : public SampleProfileLoaderBaseImpl<Function> {
: CSINLINE_DEBUG) {}
bool doInitialization(Module &M, FunctionAnalysisManager *FAM = nullptr);
- bool runOnModule(Module &M, ModuleAnalysisManager *AM,
- ProfileSummaryInfo *_PSI, LazyCallGraph &CG);
+ bool runOnModule(Module &M, ModuleAnalysisManager *AM, LazyCallGraph &CG);
protected:
bool runOnFunction(Function &F, ModuleAnalysisManager *AM);
@@ -2708,16 +2708,17 @@ void SampleProfileMatcher::distributeIRToProfileLocationMap() {
}
bool SampleProfileLoader::runOnModule(Module &M, ModuleAnalysisManager *AM,
- ProfileSummaryInfo *_PSI,
LazyCallGraph &CG) {
GUIDToFuncNameMapper Mapper(M, *Reader, GUIDToFuncNameMap);
- PSI = _PSI;
if (M.getProfileSummary(/* IsCS */ false) == nullptr) {
M.setProfileSummary(Reader->getSummary().getMD(M.getContext()),
ProfileSummary::PSK_Sample);
- PSI->refresh();
+ PreservedAnalyses PA = PreservedAnalyses::all();
+ PA.abandon<ProfileSummaryAnalysis>();
+ AM->invalidate(M, PA);
}
+ PSI = &AM->getResult<ProfileSummaryAnalysis>(M);
// Compute the total number of samples collected in this profile.
for (const auto &I : Reader->getProfiles())
TotalCollectedSamples += I.second.getTotalSamples();
@@ -2893,9 +2894,8 @@ PreservedAnalyses SampleProfileLoaderPass::run(Module &M,
if (!SampleLoader.doInitialization(M, &FAM))
return PreservedAnalyses::all();
- ProfileSummaryInfo *PSI = &AM.getResult<ProfileSummaryAnalysis>(M);
LazyCallGraph &CG = AM.getResult<LazyCallGraphAnalysis>(M);
- if (!SampleLoader.runOnModule(M, &AM, PSI, CG))
+ if (!SampleLoader.runOnModule(M, &AM, CG))
return PreservedAnalyses::all();
return PreservedAnalyses::none();
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 55728709cde556..af8a16be8e7f30 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -65,6 +65,7 @@
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/Analysis/ProfileSummaryInfo.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/IR/Analysis.h"
#include "llvm/IR/Attributes.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/CFG.h"
@@ -1948,7 +1949,8 @@ static bool annotateAllFunctions(
function_ref<TargetLibraryInfo &(Function &)> LookupTLI,
function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
function_ref<BlockFrequencyInfo *(Function &)> LookupBFI,
- ProfileSummaryInfo *PSI, bool IsCS) {
+ function_ref<ProfileSummaryInfo *(Module &)> AbandonAndRefreshPSI,
+ bool IsCS) {
LLVM_DEBUG(dbgs() << "Read in profile counters: ");
auto &Ctx = M.getContext();
// Read the counter array from file.
@@ -1991,7 +1993,7 @@ static bool annotateAllFunctions(
M.setProfileSummary(PGOReader->getSummary(IsCS).getMD(M.getContext()),
IsCS ? ProfileSummary::PSK_CSInstr
: ProfileSummary::PSK_Instr);
- PSI->refresh();
+ auto *PSI = AbandonAndRefreshPSI(M);
std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
collectComdatMembers(M, ComdatMembers);
@@ -2157,10 +2159,16 @@ PreservedAnalyses PGOInstrumentationUse::run(Module &M,
return &FAM.getResult<BlockFrequencyAnalysis>(F);
};
- auto *PSI = &MAM.getResult<ProfileSummaryAnalysis>(M);
+ auto AbandonAndRefreshPSI = [&MAM](Module &M) {
+ PreservedAnalyses PA = PreservedAnalyses::all();
+ PA.abandon<ProfileSummaryAnalysis>();
+ MAM.invalidate(M, PA);
+ return &MAM.getResult<ProfileSummaryAnalysis>(M);
+ };
if (!annotateAllFunctions(M, ProfileFileName, ProfileRemappingFileName, *FS,
- LookupTLI, LookupBPI, LookupBFI, PSI, IsCS))
+ LookupTLI, LookupBPI, LookupBFI,
+ AbandonAndRefreshPSI, IsCS))
return PreservedAnalyses::all();
return PreservedAnalyses::none();
diff --git a/llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll b/llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
index ac80a31d8fd4bc..e5aebc4850e6db 100644
--- a/llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
+++ b/llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
@@ -31,9 +31,9 @@
; CHECK-EP-PIPELINE-START: Running pass: NoOpModulePass
; CHECK-O: Running pass: SampleProfileLoaderPass
; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy
-; CHECK-O-NEXT: Running analysis: ProfileSummaryAnalysis
; CHECK-O-NEXT: Running analysis: LazyCallGraphAnalysis
; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
+; CHECK-O-NEXT: Running analysis: ProfileSummaryAnalysis
; CHECK-O-NEXT: Running pass: RequireAnalysisPass<{{.*}}ProfileSummaryAnalysis
; CHECK-O-NEXT: Running pass: PGOIndirectCallPromotion
; CHECK-O-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis
diff --git a/llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll b/llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
index 47bdbfd2d357d4..bba410bf5178d7 100644
--- a/llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ b/llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -43,8 +43,8 @@
; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass
; CHECK-O-NEXT: Running pass: SampleProfileLoaderPass
-; CHECK-O-NEXT: Running analysis: ProfileSummaryAnalysis
; CHECK-O-NEXT: Running analysis: LazyCallGraphAnalysis
+; CHECK-O-NEXT: Running analysis: ProfileSummaryAnalysis
; CHECK-O-NEXT: Running pass: RequireAnalysisPass<{{.*}}ProfileSummaryAnalysis
; CHECK-O-NEXT: Running pass: OpenMPOptPass
; CHECK-O-NEXT: Running pass: IPSCCPPass
>From 18755478cfe52204396baeba5cc249638e3707b6 Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Thu, 21 Mar 2024 16:14:07 -0700
Subject: [PATCH 2/2] simpler API in PGOInstrumentation
---
.../Instrumentation/PGOInstrumentation.cpp | 60 +++++++------------
1 file changed, 21 insertions(+), 39 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index af8a16be8e7f30..218bd331f02ea5 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1943,14 +1943,10 @@ static void verifyFuncBFI(PGOUseFunc &Func, LoopInfo &LI,
});
}
-static bool annotateAllFunctions(
- Module &M, StringRef ProfileFileName, StringRef ProfileRemappingFileName,
- vfs::FileSystem &FS,
- function_ref<TargetLibraryInfo &(Function &)> LookupTLI,
- function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
- function_ref<BlockFrequencyInfo *(Function &)> LookupBFI,
- function_ref<ProfileSummaryInfo *(Module &)> AbandonAndRefreshPSI,
- bool IsCS) {
+static bool annotateAllFunctions(Module &M, ModuleAnalysisManager &MAM,
+ StringRef ProfileFileName,
+ StringRef ProfileRemappingFileName,
+ vfs::FileSystem &FS, bool IsCS) {
LLVM_DEBUG(dbgs() << "Read in profile counters: ");
auto &Ctx = M.getContext();
// Read the counter array from file.
@@ -1993,7 +1989,11 @@ static bool annotateAllFunctions(
M.setProfileSummary(PGOReader->getSummary(IsCS).getMD(M.getContext()),
IsCS ? ProfileSummary::PSK_CSInstr
: ProfileSummary::PSK_Instr);
- auto *PSI = AbandonAndRefreshPSI(M);
+
+ PreservedAnalyses PA = PreservedAnalyses::all();
+ PA.abandon<ProfileSummaryAnalysis>();
+ MAM.invalidate(M, PA);
+ auto &PSI = MAM.getResult<ProfileSummaryAnalysis>(M);
std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
collectComdatMembers(M, ComdatMembers);
@@ -2006,19 +2006,21 @@ static bool annotateAllFunctions(
if (PGOInstrumentEntry.getNumOccurrences() > 0)
InstrumentFuncEntry = PGOInstrumentEntry;
bool HasSingleByteCoverage = PGOReader->hasSingleByteCoverage();
+ auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+
for (auto &F : M) {
if (skipPGOUse(F))
continue;
- auto &TLI = LookupTLI(F);
- auto *BPI = LookupBPI(F);
- auto *BFI = LookupBFI(F);
+ auto &TLI = FAM.getResult<TargetLibraryAnalysis>(F);
+ auto &BPI = FAM.getResult<BranchProbabilityAnalysis>(F);
+ auto &BFI = FAM.getResult<BlockFrequencyAnalysis>(F);
if (!HasSingleByteCoverage) {
// Split indirectbr critical edges here before computing the MST rather
// than later in getInstrBB() to avoid invalidating it.
- SplitIndirectBrCriticalEdges(F, /*IgnoreBlocksWithoutPHI=*/false, BPI,
- BFI);
+ SplitIndirectBrCriticalEdges(F, /*IgnoreBlocksWithoutPHI=*/false, &BPI,
+ &BFI);
}
- PGOUseFunc Func(F, &M, TLI, ComdatMembers, BPI, BFI, PSI, IsCS,
+ PGOUseFunc Func(F, &M, TLI, ComdatMembers, &BPI, &BFI, &PSI, IsCS,
InstrumentFuncEntry, HasSingleByteCoverage);
if (HasSingleByteCoverage) {
Func.populateCoverage(PGOReader.get());
@@ -2096,8 +2098,8 @@ static bool annotateAllFunctions(
// Verify BlockFrequency information.
uint64_t HotCountThreshold = 0, ColdCountThreshold = 0;
if (PGOVerifyHotBFI) {
- HotCountThreshold = PSI->getOrCompHotCountThreshold();
- ColdCountThreshold = PSI->getOrCompColdCountThreshold();
+ HotCountThreshold = PSI.getOrCompHotCountThreshold();
+ ColdCountThreshold = PSI.getOrCompColdCountThreshold();
}
verifyFuncBFI(Func, LI, NBPI, HotCountThreshold, ColdCountThreshold);
}
@@ -2147,28 +2149,8 @@ PGOInstrumentationUse::PGOInstrumentationUse(
PreservedAnalyses PGOInstrumentationUse::run(Module &M,
ModuleAnalysisManager &MAM) {
-
- auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
- auto LookupTLI = [&FAM](Function &F) -> TargetLibraryInfo & {
- return FAM.getResult<TargetLibraryAnalysis>(F);
- };
- auto LookupBPI = [&FAM](Function &F) {
- return &FAM.getResult<BranchProbabilityAnalysis>(F);
- };
- auto LookupBFI = [&FAM](Function &F) {
- return &FAM.getResult<BlockFrequencyAnalysis>(F);
- };
-
- auto AbandonAndRefreshPSI = [&MAM](Module &M) {
- PreservedAnalyses PA = PreservedAnalyses::all();
- PA.abandon<ProfileSummaryAnalysis>();
- MAM.invalidate(M, PA);
- return &MAM.getResult<ProfileSummaryAnalysis>(M);
- };
-
- if (!annotateAllFunctions(M, ProfileFileName, ProfileRemappingFileName, *FS,
- LookupTLI, LookupBPI, LookupBFI,
- AbandonAndRefreshPSI, IsCS))
+ if (!annotateAllFunctions(M, MAM, ProfileFileName, ProfileRemappingFileName,
+ *FS, IsCS))
return PreservedAnalyses::all();
return PreservedAnalyses::none();
More information about the llvm-commits
mailing list