[clang] [llvm] [InstrPGO] Instrument sampling profile based cold function (PR #109837)
Lei Wang via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 8 11:04:53 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:
> I don't see how there would be any functional difference between using `IsSampleColdFuncCovGen` and `IsXXXColdFuncCovGen`, but I could be missing something. If we can have a single flag for all cases, I think that would be cleaner and I can also try to use it for my case.
I see, sounds good.
My previous thought is: there could be functional difference for using the `-instrument-cold-function-coverage` flag with sampling PGO flag(`-fprofile-sample-use`) vs without sampling PGO flags. With the sampling PGO flag, if a function symbol shows in the binary but not in the profile, we can treat it as cold function(set 0 entry count), we then would instrument those function. But without sampling PGO flag(also no other PGO options), the entry count is unknown(-1), then we don't instrument them.
So user needs to be aware of the combination use of those flags, I was then trying to prevent the misuse, to disallow the use of `--instrument-cold-function-coverage` without (sampling) PGO use options(use the assertion in the version).
But on a second thought, if we want to extend for other PGO options, say if the profile annotation and instrumentation are not in the same pipeline, it's probably hard to check this locally, we can leave it to the build system.
https://github.com/llvm/llvm-project/pull/109837
More information about the cfe-commits
mailing list