[clang] [llvm] [InstrPGO] Instrument sampling profile based cold function (PR #109837)

via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 16 18:01:03 PDT 2024


================
@@ -649,6 +649,24 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
     }
   }
 
+  if (auto *ColdFuncCoverageArg = Args.getLastArg(
+          options::OPT_fprofile_generate_cold_function_coverage,
+          options::OPT_fprofile_generate_cold_function_coverage_EQ)) {
+    SmallString<128> Path(
+        ColdFuncCoverageArg->getOption().matches(
+            options::OPT_fprofile_generate_cold_function_coverage_EQ)
+            ? ColdFuncCoverageArg->getValue()
+            : "");
+    llvm::sys::path::append(Path, "default_%m.profraw");
+    CmdArgs.push_back("-mllvm");
+    CmdArgs.push_back(Args.MakeArgString(
+        Twine("--instrument-cold-function-coverage-path=") + Path));
+    CmdArgs.push_back("-mllvm");
+    CmdArgs.push_back("--instrument-cold-function-coverage");
----------------
WenleiHe wrote:

These two flags seem duplicative. Path is guaranteed to be not empty here, so the boolean flag seems unnecessary? 

Relatedly, I know this might not be easy. But while it's convenient to accept a new driver flag, I still kept wondering if we can communicate the single driver flag to LLVM using existing and orthogonal flags as much as possible. Like this:

```
-fprofile-instrument-path= // reuse existing flag to communicate file path
-pgo-function-entry-coverage // reuse existing flag to communicate coverage only
-pgo-instrument-cold-function-only // add new flag to communicate cold function only
```   
I know that the current implementation has assumption when `InstrProfileOutput` is not empty, but I still wonder is there a way recognize the use of these three flags together, and special case for that.

https://github.com/llvm/llvm-project/pull/109837


More information about the cfe-commits mailing list