[clang] [llvm] [InstrPGO] Instrument sampling profile based cold function (PR #109837)
Lei Wang via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 2 16:11:02 PDT 2024
================
@@ -1119,6 +1125,18 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
// removed.
MPM.addPass(
PGOIndirectCallPromotion(true /* IsInLTO */, true /* SamplePGO */));
+
+ if (InstrumentSampleColdFuncPath.getNumOccurrences() &&
+ Phase != ThinOrFullLTOPhase::ThinLTOPostLink) {
+ assert(!InstrumentSampleColdFuncPath.empty() &&
+ "File path is requeired for instrumentation generation");
+ InstrumentColdFunctionCoverage = true;
+ addPreInlinerPasses(MPM, Level, Phase);
+ addPGOInstrPasses(MPM, Level, /* RunProfileGen */ true,
+ /* IsCS */ false, /* AtomicCounterUpdate */ false,
+ InstrumentSampleColdFuncPath, "",
+ IntrusiveRefCntPtr<vfs::FileSystem>());
+ }
----------------
wlei-llvm wrote:
do you mean the `sample` in `InstrumentSampleColdFuncPath`flag or all other variables(like `IsSampleColdFuncCovGen`)? I thought something like:
```
if (IsSampleColdFuncCovGen || IsXXXColdFuncCovGen) {
addPGOInstrPasses(.., .., InstrumentColdFuncCoveragePath, .. )
}
```
`InstrumentColdFuncCoveragePath` would be a general flag that can be used for all cold function coverage case. but currently `IsSampleColdFuncCovGen` only represents for sampling PGO cold func. And If we want to extend it in future, then we can add more bool flag IsXXXColdFuncCovGen...
I also added an assertion:
```
assert((InstrumentColdFuncCoveragePath.empty() || LoadSampleProfile) &&
"Sampling-based cold functon coverage is not supported without "
"providing sampling PGO profile");
```
Seems I have to remove that if we want it to be general. (just curious) do you plan to extend it for your case?
https://github.com/llvm/llvm-project/pull/109837
More information about the cfe-commits
mailing list