================
@@ -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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to