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

[Lei Wang via cfe-commits]

https://github.com/wlei-llvm edited 
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

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH 1/8] [InstrPGO] Instrument sampling profile based cold
 function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH 1/8] [InstrPGO] Instrument sampling profile based cold
 function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx

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

[via cfe-commits]

@@ -53,6 +53,8 @@ class PGOInstrumentationGenCreateVar
   bool ProfileSampling;
 };
 
+enum class InstrColdFuncCovMode { Conservative = 0, Optimistic };

WenleiHe wrote:

remove this? 

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

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

[via cfe-commits]

https://github.com/WenleiHe approved this pull request.

Looks good enough to me with the FIXME :) Thanks!

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

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

[via cfe-commits]

https://github.com/WenleiHe edited 
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

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH 1/7] [InstrPGO] Instrument sampling profile based cold
 function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx

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

[Lei Wang via cfe-commits]

@@ -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");

wlei-llvm wrote:

I really like this idea, however, this seems indeed not easy(needs a lot of 
refactoring). 

As you said, the `InstrProfileOutput` filed is shared with other PGO flag(in 
our case, it's the `fprofile-sample-use=`) , see 
[`PGOOptions.ProfileFIle`](https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/PGOOptions.h#L43).
 One thing we can do is to split this field into two: `ProfileUseFile` and 
`ProfileGenFole`, let all `profile--gererate` go to  `ProfileGenFile` 
and let `sample-use/instr-use go` to `ProfileUseFile`. I think that's doable 
and not a big change(maybe a separate diff). Then in sample-use case, we can 
use the  `ProfileGenFIle` path for cold coverage work. WDYT?

Though it would still not be perfect, the logic here 
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/BackendUtil.cpp#L811-L841
  is complicated/ would still need more refactoring. I was stuck here when 
thinking if `-fprofile-sample-use` and `--fprofile-instrument-path=` are both 
used, which branch(`CodeGenOpts.hasProfileIRInstr()` or 
`!CodeGenOpts.SampleProfileFile.empty()`) should it belongs to.

For this version, I just removed the duplicated 
flag(`--pgo-instrument-cold-function-only`) and added a FIXME comment to 
explain this. 




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

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

[Lei Wang via cfe-commits]

@@ -319,6 +319,29 @@ static cl::opt PGOFunctionCriticalEdgeThreshold(
 cl::desc("Do not instrument functions with the number of critical edges "
  " greater than this threshold."));
 
+static cl::opt ColdFuncCoverageMaxEntryCount(
+"cold-function-coverage-max-entry-count", cl::init(0), cl::Hidden,
+cl::desc("When enabling cold function coverage instrumentation, skip "
+ "instrumenting the function whose entry count is above the given "
+ "value"));
+
+static cl::opt InstrumentColdFunctionCoverageMode(
+"instrument-cold-function-coverage-mode",

wlei-llvm wrote:

Agreed, I can't see other modes so far, changed to `pgo-treat-unknown-as-cold`

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

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

[Lei Wang via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;

wlei-llvm wrote:

thanks for the suggestion!

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

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

[via cfe-commits]

@@ -319,6 +319,29 @@ static cl::opt PGOFunctionCriticalEdgeThreshold(
 cl::desc("Do not instrument functions with the number of critical edges "
  " greater than this threshold."));
 
+static cl::opt ColdFuncCoverageMaxEntryCount(
+"cold-function-coverage-max-entry-count", cl::init(0), cl::Hidden,
+cl::desc("When enabling cold function coverage instrumentation, skip "
+ "instrumenting the function whose entry count is above the given "
+ "value"));
+
+static cl::opt InstrumentColdFunctionCoverageMode(
+"instrument-cold-function-coverage-mode",

WenleiHe wrote:

Practically do you think we will end up using more than two modes? If not, how 
about simplify this with 
`pgo-treat-unknown-as-cold`/`pgo-treat-unprofiled-as-cold`?

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

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

[via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

WenleiHe wrote:

+1 on single driver flag for convenience.

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

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

[via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;

WenleiHe wrote:

The help text doesn't parse. Did you mean something like:

```
Generate instrumented code to collect coverage info for cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)
```

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

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

[via cfe-commits]

@@ -319,6 +319,29 @@ static cl::opt PGOFunctionCriticalEdgeThreshold(
 cl::desc("Do not instrument functions with the number of critical edges "
  " greater than this threshold."));
 
+static cl::opt ColdFuncCoverageMaxEntryCount(
+"cold-function-coverage-max-entry-count", cl::init(0), cl::Hidden,

WenleiHe wrote:

how about `pgo-cold-instrument-entry-threshold`?

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

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

[via cfe-commits]

@@ -319,6 +319,29 @@ static cl::opt PGOFunctionCriticalEdgeThreshold(
 cl::desc("Do not instrument functions with the number of critical edges "
  " greater than this threshold."));
 
+static cl::opt ColdFuncCoverageMaxEntryCount(
+"cold-function-coverage-max-entry-count", cl::init(0), cl::Hidden,
+cl::desc("When enabling cold function coverage instrumentation, skip "
+ "instrumenting the function whose entry count is above the given "
+ "value"));
+
+static cl::opt InstrumentColdFunctionCoverageMode(
+"instrument-cold-function-coverage-mode",
+cl::init(InstrColdFuncCovMode::Conservative), cl::Hidden,
+cl::desc("Control whether to instrument unprofiled functions for cold "
+ "function coverage."),
+cl::values(
+clEnumValN(InstrColdFuncCovMode::Conservative, "conservative",
+   "Assume unprofiled functions are not cold, skip "
+   "instrumenting them."),
+clEnumValN(InstrColdFuncCovMode::Optimistic, "optimistic",
+   "Treat unprofiled functions as cold and instrument 
them.")));
+
+cl::opt InstrumentColdFunctionCoverage(
+"instrument-cold-function-coverage", cl::init(false), cl::Hidden,

WenleiHe wrote:

We already have `pgo-function-entry-coverage` flag to ask for coverage. Ideally 
we want flags to be orthogonal and not overlap. So how about 
`pgo-instrument-cold-function-only` (add `pgo-` prefix to be consistent)? 

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

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

[via cfe-commits]

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

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH 1/6] [InstrPGO] Instrument sampling profile based cold
 function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx

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

[Ellis Hoag via cfe-commits]

@@ -1891,6 +1914,12 @@ static bool skipPGOGen(const Function &F) {
 return true;
   if (F.getInstructionCount() < PGOFunctionSizeThreshold)
 return true;
+  if (InstrumentColdFunctionCoverage) {
+if (!F.getEntryCount())
+  return InstrumentColdFunctionCoverageMode ==
+ InstrColdFuncCovMode::Conservative;
+return F.getEntryCount()->getCount() > ColdFuncCoverageMaxEntryCount;

ellishg wrote:

```suggestion
if (auto EntryCount = F.getEntryCount())
  return EntryCount->getCount() > ColdFuncCoverageMaxEntryCount;
return InstrumentColdFunctionCoverageMode ==
 InstrColdFuncCovMode::Conservative;
```

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

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

[Ellis Hoag via cfe-commits]

https://github.com/ellishg edited 
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

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

[Ellis Hoag via cfe-commits]

@@ -319,6 +319,29 @@ static cl::opt PGOFunctionCriticalEdgeThreshold(
 cl::desc("Do not instrument functions with the number of critical edges "
  " greater than this threshold."));
 
+static cl::opt ColdFuncCoverageMaxEntryCount(
+"cold-function-coverage-max-entry-count", cl::init(0), cl::Hidden,
+cl::desc("When enabling cold function coverage instrumentation, skip "
+ "instrumenting the function whose entry count is above the given "
+ "value"));
+
+static cl::opt InstrumentColdFunctionCoverageMode(
+"instrument-cold-function-coverage-mode",
+cl::init(InstrColdFuncCovMode::Conservative), cl::Hidden,
+cl::desc("Control whether instrumenting unprofiled functions for cold "
+ "function coverage."),

ellishg wrote:

```suggestion
cl::desc("Control whether to instrument unprofiled functions for cold "
 "function coverage."),
```

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

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

[Ellis Hoag via cfe-commits]

https://github.com/ellishg approved this pull request.

The backend looks good to me! (after resolving my last comments of course)
Thanks for adding this feature!

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

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH 1/5] [InstrPGO] Instrument sampling profile based cold
 function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx

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

[Lei Wang via cfe-commits]

@@ -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());
+}

wlei-llvm wrote:

Good point, that's more flexible, thanks!

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

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

[Ellis Hoag via cfe-commits]

@@ -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());
+}

ellishg wrote:

> if a function symbol shows in the binary but not in the profile, we can treat 
> it as cold function(set 0 entry count)

I wasn't aware that SamplePGO did this. This is indeed different than IRPGO. 
Perhaps it makes more sense to use a separate flag to control this case. Maybe 
something like 
`-instrument-cold-function-coverage-mode={optimistic,conservative}` where 
`optimistic` means we optimistically treat unprofiled functions as cold and 
`conservative` means we conservatively assume unprofiled functions is hot. 

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

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH 1/4] [InstrPGO] Instrument sampling profile based cold
 function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx

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

[Lei Wang via cfe-commits]

@@ -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());
+}

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

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

[Ellis Hoag via cfe-commits]

@@ -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());
+}

ellishg 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.

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

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

[Lei Wang via cfe-commits]

@@ -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());
+}

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

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

[Snehasish Kumar via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

snehasish wrote:

> Is it that the backend can't actually reliably point the finger at the 
> specific flag causing the conflict

Yes, if we know that this instrumentation mode should not be mixed with e.g. 
sanitizers or something else we can enforce these checks early. I don't see a 
particular downside to adding a frontend flag. The convenience of bundling the 
2 lld flags and 3 mllvm flags into a single frontend flag seems like a good 
enough motivation to do so.

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

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

[Ellis Hoag via cfe-commits]

@@ -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());
+}

ellishg wrote:

Thanks! Now I think this should be independent of SamplePGO. Can we remove 
"sample" from the flag and variable names?

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

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH 1/3] [InstrPGO] Instrument sampling profile based cold
 function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx

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

[Lei Wang via cfe-commits]

@@ -0,0 +1,24 @@
+; RUN: opt < %s  --passes=pgo-instr-gen -instrument-cold-function-coverage -S  
| FileCheck --check-prefixes=COLD %s
+; RUN: opt < %s  --passes=pgo-instr-gen -instrument-cold-function-coverage 
-cold-function-coverage-max-entry-count=1 -S  | FileCheck 
--check-prefixes=ENTRY-COUNT %s
+
+; COLD: call void @llvm.instrprof.increment(ptr @__profn_foo, i64  [[#]], i32 
1, i32 0)

wlei-llvm wrote:

Sounds good!(it should still work without `-pgo-function-entry-coverage`, but 
agree to test the most common case)

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

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

[Lei Wang via cfe-commits]

@@ -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());
+}

wlei-llvm wrote:

> After looking at this a bit closer, I'm not sure why it needs to be tied to 
> closely to SamplePGO. Couldn't we move this out of the `LoadSampleProfile` 
> and move it after `IsPGOInstrUse/IsCtxProfUse`? That way we can use IRPGO, 
> CSIRPGO, and SamplePGO profile counts to block instrumentation hot functions.

Ah, good point, moved them closer to other IRPGO passes. Thanks for the 
suggestion!

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

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

[Lei Wang via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

wlei-llvm wrote:

>Would -Wl,-lclang_rt.profile work?

Got it, I think it should work, except it requires another linker flag: the 
`--u__llvm_runtime_variable`, we can configure it to linker too, but basically 
[those instr PGO 
flags](https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L885-L893)
 control 
[addProfileRTLibs](https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Linux.cpp#L841-L849)
 (which seems not equivalent to `-Wl,-lclang_rt.profile`), I just wanted to 
mirror those flags so that we don't need to maintain if anything changes to 
`addProfileRTLibs` in the future. (Though I feel this code should be very 
stable, should not be a big problem, so mainly for the convenience for compiler 
user to use one flag instead of using/maintaining multiple flags )
Overall, I agree that it's feasible to do it without clang flag. I'm not very 
familiar with the convention for adding driver flags, so if you think this 
doesn't justify it, I will drop it from this patch. Thanks for the discussion!

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

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

[Mircea Trofin via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

mtrofin wrote:

Why would incompatibilities and ability to provide useful error messages be 
dependent on the flag being in the frontend? Is it that the backend can't 
actually reliably point the finger at the specific flag causing the conflict, 
so a message would be diluted to "sampling won't work with this"?

(probably a subject for a different forum tho)

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

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

[Snehasish Kumar via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

snehasish wrote:

>  also to centralize the configuration for the convenience

+1 I think a frontend flag is useful. It allows us to identify 
incompatibilities early and provide clear error messages instead of more 
obscure failures down the line.

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

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

[Mircea Trofin via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

mtrofin wrote:

Would `-Wl,-lclang_rt.profile` work?

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

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

[Lei Wang via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

wlei-llvm wrote:

> I meant, why not just use `clang ... -mllvm 
> -instrument-cold-function-coverage`? Is this a clang - only feature (i.e. 
> rust can't use it?) Is it just for symmetry with the current PGO flags?
> 
> (This is not a pushback, mainly curious. Also the patch would be quite 
> smaller if you didn't need to pass through the flags from clang to llvm)

I see, thanks for the suggestion!  We also need to link runtime 
lib(`compiler_rt.profile.a`)
but yeah, I agree it's possible to pass directly to llvm and linker without 
through clang. Then the full command line would be like 
```
clang ... -mllvm -instrument-cold-function-coverage -mllvm 
-instrument-sample-cold-function-path= -mllvm 
--pgo-function-entry-coverage 
ld.lld ... --u__llvm_runtime_variable  
.../lib/x86_64-unknown-linux-gnu/libclang_rt.profile.a
```
So yes, adding the clang driver flag is kind of to mirror current[ IRPGO 
flags](https://fburl.com/na3cp3gn), for easy maintenance purpose(given that 
`-fprofile-generate` doesn't work with `-fprofile-sample-use`) and also to 
centralize the configuration for the convenience. IMO, the `compiler_rt` is 
probably the main blocker here, I didn't find an easy way to bundle it with a 
llvm flag. 

Appreciate any further suggestions!

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

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

[Mircea Trofin via cfe-commits]

https://github.com/mtrofin edited 
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

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

[Mircea Trofin via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

mtrofin wrote:

I meant, why not just use `clang ... -mllvm 
-instrument-cold-function-coverage`? Is this a clang - only feature (i.e. rust 
can't use it?) Is it just for symmetry with the current PGO flags?

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

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

[Lei Wang via cfe-commits]

https://github.com/wlei-llvm edited 
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

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

[Ellis Hoag via cfe-commits]

@@ -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());
+}

ellishg wrote:

After looking at this a bit closer, I'm not sure why it needs to be tied to 
closely to SamplePGO. Couldn't we move this out of the `LoadSampleProfile` and 
move it after `IsPGOInstrUse/IsCtxProfUse`? That way we can use IRPGO, CSIRPGO, 
and SamplePGO profile counts to block instrumentation hot functions.

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

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

[Ellis Hoag via cfe-commits]

@@ -0,0 +1,24 @@
+; RUN: opt < %s  --passes=pgo-instr-gen -instrument-cold-function-coverage -S  
| FileCheck --check-prefixes=COLD %s
+; RUN: opt < %s  --passes=pgo-instr-gen -instrument-cold-function-coverage 
-cold-function-coverage-max-entry-count=1 -S  | FileCheck 
--check-prefixes=ENTRY-COUNT %s
+
+; COLD: call void @llvm.instrprof.increment(ptr @__profn_foo, i64  [[#]], i32 
1, i32 0)

ellishg wrote:

Do you plan to support `-instrument-cold-function-coverage` without 
`-pgo-function-entry-coverage`? If not, I think we should add the flag here so 
we get the `llvm.instrprof.cover` intrinsic to more closely align with expected 
behavior.

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

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

[Ellis Hoag via cfe-commits]

@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");

ellishg wrote:

You probably want to add a driver test case for this change like
https://github.com/llvm/llvm-project/blob/2f4327294dccc27fc9d5febe71196f6f854d66ff/clang/test/Driver/fcs-profile-generate.c#L1-L4

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

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

[Ellis Hoag via cfe-commits]

@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx/yyy/ 
-fprofile-sample-accurate -fprofile-sample-use=%S/Inputs/pgo-cold-func.prof  -S 
-emit-llvm -o - %s | FileCheck %s

ellishg wrote:

You can use `split-file` to avoid needing to checkin a separate test file in 
`Inputs/`.
https://llvm.org/docs/TestingGuide.html#extra-files

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

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

[Ellis Hoag via cfe-commits]

ellishg wrote:

> > Why not use the existing `-pgo-function-entry-coverage` 
> > (https://discourse.llvm.org/t/instrprofiling-lightweight-instrumentation/59113/14?u=ellishg)
> >  LLVM flag? It takes advantage of the `llvm.instrprof.cover` intrinsic 
> > which has less size and runtime overhead than `llvm.instrprof.increment`.
> 
> We do use the `-pgo-function-entry-coverage` in this PR, see 
> [here](https://github.com/llvm/llvm-project/pull/109837/files#diff-bac41c71569f27df21a843bcd74d2e604ed508afbdf14161dfb545c5d228R666-R667).
>  but furthermore, we skip instrumenting the functions that are covered by 
> sampling PGO profile.

Oh, I missed that and your [earlier 
comment](https://github.com/llvm/llvm-project/pull/109837#discussion_r1783584037).
 Makes more sense, thanks!

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

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH 1/2] [InstrPGO] Instrument sampling profile based cold
 function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx

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

[Lei Wang via cfe-commits]

wlei-llvm wrote:

Thanks for the context! 
> Why not use the existing `-pgo-function-entry-coverage` 
> (https://discourse.llvm.org/t/instrprofiling-lightweight-instrumentation/59113/14?u=ellishg)
>  LLVM flag? It takes advantage of the `llvm.instrprof.cover` intrinsic which 
> has less size and runtime overhead than `llvm.instrprof.increment`.

We do use the  `-pgo-function-entry-coverage` in this PR, see 
[here](https://github.com/llvm/llvm-project/pull/109837/files#diff-bac41c71569f27df21a843bcd74d2e604ed508afbdf14161dfb545c5d228R666-R667).
 but furthermore, we skip instrumenting the functions that are covered by 
sampling PGO profile. 

> It also supports IRPGO and CSIRPGO while it seems that this PR requires a 
> sample PGO input.

Yeah, as I commented above, unfortunately currently the IRPGO main flag is 
incompatible with the Sampling PGO flag(also a bit diverged for the pipeline 
passes), that's why we have to add extra instr passes under sampling PGO 
pipeline and added a new driver flag. 


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

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

[Lei Wang via cfe-commits]

wlei-llvm wrote:

> For your use case, can you use 
> [ProfileSymbolList](https://github.com/llvm/llvm-project/blob/32ffc9fdc2cd422c88c926b862adb3de726e3888/llvm/include/llvm/ProfileData/SampleProf.h#L1509-L1512)
>  to identify very cold functions (executed but unsampled) or are you indeed 
> looking for functions that are never executed?

We are indeed focusing on functions that are never executed. The 
ProfileSymbolList is already used to identify functions with a “0” entry count, 
which is the target of the instrumentation in this work. We then aim to further 
distinguish the dead functions from them.

> 
> Will this be used to guide developers with diagnostics or more aggressive 
> compiler driven optimizations?

We haven’t considered to use them for any compiler optimization. The primary 
goal is to improve the developer experience, just to take the general benefits 
from removing dead code: improving readability, good codebase maintainability, 
reducing build time, etc. Another potential use might be to verify the sampling 
PGO profile coverage/quality, say to check if we missed any functions that are 
actually hot. 


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

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

[Lei Wang via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

wlei-llvm wrote:

Sorry if my PR description is not clear. Note that there is no use for  
`-fprofile-generate` and  `-fprofile-instr-generate` here, so a driver flag 
here is to configure the instr file path and make linker to link the 
compiler.profile object files (just similar to `-fprofile-[instr]-generate=`).

The reason for not using `-fprofile-[instr]-generate` is because those flags 
conflict with `-fprofile-sample-use`, see 
[PGOOptions](https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/PGOOptions.h#L27-L43),
 `ProfileFile` is a shared file path which means the two flags are actually 
mutually exclusive. 

Another option is to make `-fprofile-generate` compatible with 
`-fprofile-samle-use`(like refactoring PGOOptions or adding another flag to 
configure the file path things), this seems to me they are more complicated 
than the current one. But I’m open to any suggestions on this. 

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

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

[Ellis Hoag via cfe-commits]

ellishg wrote:

Why not use the existing `-pgo-function-entry-coverage` 
(https://discourse.llvm.org/t/instrprofiling-lightweight-instrumentation/59113/14?u=ellishg)
 LLVM flag? It takes advantage of the `llvm.instrprof.cover` intrinsic which 
has less size and runtime overhead than `llvm.instrprof.increment`. It also 
supports IRPGO and CSIRPGO while it seems that this PR requires a sample PGO 
input.

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

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

[Snehasish Kumar via cfe-commits]

snehasish wrote:

> The major motivation is to detect dead functions for the services that are 
> optimized with sampling PGO.

For your use case, can you use 
[ProfileSymbolList](https://github.com/llvm/llvm-project/blob/32ffc9fdc2cd422c88c926b862adb3de726e3888/llvm/include/llvm/ProfileData/SampleProf.h#L1509-L1512)
 to identify very cold functions (executed but unsampled) or are you indeed 
looking for functions that are never executed?

Will this be used to guide developers with diagnostics or more aggressive 
compiler driven optimizations?

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

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

[Mircea Trofin via cfe-commits]

@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 

mtrofin wrote:

Is it necessary to expose this through the clang frontend?

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

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

[Mircea Trofin via cfe-commits]

@@ -319,6 +319,18 @@ static cl::opt PGOFunctionCriticalEdgeThreshold(
 cl::desc("Do not instrument functions with the number of critical edges "
  " greater than this threshold."));
 
+cl::opt InstrumentColdFunctionCoverage(
+"instrument-cold-function-coverage", cl::init(false), cl::Hidden,
+cl::desc("Enable cold function coverage instrumentation (currently only "
+ "used under sampling "

mtrofin wrote:

nit: here and below, the formatting looks odd. Perhaps re-join the doc string 
and re-run clang-format?

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

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

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-pgo

Author: Lei Wang (wlei-llvm)


Changes

This patch adds support for cold function coverage instrumentation based on 
sampling PGO counts. The major motivation is to detect dead functions for the 
services that are optimized with sampling PGO. If a function is covered by 
sampling profile count (e.g., those with an entry count > 0), we choose to 
skip instrumenting those functions, which significantly reduces the 
instrumentation overhead. 

More details about the implementation and flags:
- Added a flag `--instrument-cold-function-coverage` in 
`PGOInstrumentation.cpp` as the main switch to control skipping the 
instrumentation.
- Built the extra instrumentation passes(a bundle of passes in 
`addPGOInstrPasses`) under sampling PGO pipeline. This is controlled by 
`--instrument-sample-cold-function-path` flag.  
- Added a driver flag `-fprofile-generate-cold-function-coverage`: 
  - 1) Config the flags in one place, i,e. adding 
`--instrument-sample-cold-function-path=<...>` and 
`--instrument-cold-function-coverage`, and `--pgo-function-entry-coverage`.  
Note that the instrumentation file path is passed through 
`--instrument-sample-cold-function-path`, because we cannot use the 
`PGOOptions.ProfileFile` as it's already used by 
`-fprofile-sample-use=<...>`.
  - 2) makes linker to link `compiler_rt.profile` lib(see 
[ToolChain.cpp#L1125-L1131](https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L1125-L1131)
 ). 
- Added a flag(`--cold-function-coverage-max-entry-count`) to config entry 
count to determine cold function.

Overall, the full command is like:

```
clang++ -O2 -fprofile-generate-cold-function-coverage=<...> 
-fprofile-sample-use=<...>  code.cc -o code
```



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


8 Files Affected:

- (modified) clang/include/clang/Driver/Options.td (+6) 
- (modified) clang/lib/Driver/ToolChain.cpp (+3-1) 
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+18) 
- (added) clang/test/CodeGen/Inputs/pgo-cold-func.prof (+2) 
- (added) clang/test/CodeGen/pgo-cold-function-coverage.c (+12) 
- (modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+18) 
- (modified) llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (+16) 
- (added) llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll (+24) 


``diff
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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)) {
+SmallSt

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

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Lei Wang (wlei-llvm)


Changes

This patch adds support for cold function coverage instrumentation based on 
sampling PGO counts. The major motivation is to detect dead functions for the 
services that are optimized with sampling PGO. If a function is covered by 
sampling profile count (e.g., those with an entry count > 0), we choose to 
skip instrumenting those functions, which significantly reduces the 
instrumentation overhead. 

More details about the implementation and flags:
- Added a flag `--instrument-cold-function-coverage` in 
`PGOInstrumentation.cpp` as the main switch to control skipping the 
instrumentation.
- Built the extra instrumentation passes(a bundle of passes in 
`addPGOInstrPasses`) under sampling PGO pipeline. This is controlled by 
`--instrument-sample-cold-function-path` flag.  
- Added a driver flag `-fprofile-generate-cold-function-coverage`: 
  - 1) Config the flags in one place, i,e. adding 
`--instrument-sample-cold-function-path=<...>` and 
`--instrument-cold-function-coverage`, and `--pgo-function-entry-coverage`.  
Note that the instrumentation file path is passed through 
`--instrument-sample-cold-function-path`, because we cannot use the 
`PGOOptions.ProfileFile` as it's already used by 
`-fprofile-sample-use=<...>`.
  - 2) makes linker to link `compiler_rt.profile` lib(see 
[ToolChain.cpp#L1125-L1131](https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L1125-L1131)
 ). 
- Added a flag(`--cold-function-coverage-max-entry-count`) to config entry 
count to determine cold function.

Overall, the full command is like:

```
clang++ -O2 -fprofile-generate-cold-function-coverage=<...> 
-fprofile-sample-use=<...>  code.cc -o code
```



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


8 Files Affected:

- (modified) clang/include/clang/Driver/Options.td (+6) 
- (modified) clang/lib/Driver/ToolChain.cpp (+3-1) 
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+18) 
- (added) clang/test/CodeGen/Inputs/pgo-cold-func.prof (+2) 
- (added) clang/test/CodeGen/pgo-cold-function-coverage.c (+12) 
- (modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+18) 
- (modified) llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (+16) 
- (added) llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll (+24) 


``diff
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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)) {
+Small

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

[Lei Wang via cfe-commits]

https://github.com/wlei-llvm ready_for_review 
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

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

[Lei Wang via cfe-commits]

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

>From 07a2cab3fa5df2965f9f7da9ee2d3603581a47f1 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH] [InstrPGO] Instrument sampling profile based cold function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 18 ++
 clang/test/CodeGen/Inputs/pgo-cold-func.prof  |  2 ++
 .../test/CodeGen/pgo-cold-function-coverage.c | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 18 ++
 .../Instrumentation/PGOInstrumentation.cpp| 16 +
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 8 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/Inputs/pgo-cold-func.prof
 create mode 100644 clang/test/CodeGen/pgo-cold-function-coverage.c
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..6bb92427f2d53f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"], 
"fprofile-generate-cold-function-coverage">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
+def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], 
"fprofile-generate-cold-function-coverage=">, 
+Group, Visibility<[ClangOption, CLOption]>, 
MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions into 
/default.profraw (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..e56db150c6b47e 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage) ||
+ Args.hasArg(options::OPT_fprofile_generate_cold_function_coverage_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..00602a08232ba2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -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-sample-cold-function-path=") + Path));
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--instrument-cold-function-coverage");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("--pgo-function-entry-coverage");
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/clang/test/CodeGen/Inputs/pgo-cold-func.prof 
b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
new file mode 100644
index 00..e50be02e0a8545
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/pgo-cold-func.prof
@@ -0,0 +1,2 @@
+foo:1:1
+ 1: 1
diff --git a/clang/test/CodeGen/pgo-cold-function-coverage.c 
b/clang/test/CodeGen/pgo-cold-function-coverage.c
new file mode 100644
index 00..0d2767c022b9f1
--- /dev/null
+++ b/clang/test/CodeGen/pgo-cold-function-coverage.c
@@ -0,0 +1,12 @@
+// Test -fprofile-generate-cold-function-coverage 
+// RUN: %clang -O2 -fprofile-generate-cold-function-coverage=/xxx/yyy/

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

[Lei Wang via cfe-commits]

https://github.com/wlei-llvm edited 
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

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

[Lei Wang via cfe-commits]

https://github.com/wlei-llvm edited 
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

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

[Lei Wang via cfe-commits]

https://github.com/wlei-llvm edited 
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

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

[Lei Wang via cfe-commits]

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

>From 436ab3bfb3d7f1978ba1598ec7d3ca006706cbd3 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH] [InstrPGO] Instrument sampling profile based cold function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 13 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 17 +
 .../Instrumentation/PGOInstrumentation.cpp| 14 +++
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 6 files changed, 77 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..0ac289089839d2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_sample_cold_function : Flag<["-"], 
"fprofile-sample-cold-function">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions guided by 
sampling-based profile into default.profraw file (overridden by '=' form of 
option or LLVM_PROFILE_FILE env var)">;
+def fprofile_sample_cold_function_EQ : Joined<["-"], 
"fprofile-sample-cold-function=">, 
+Group, Visibility<[ClangOption, CLOption]>, MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions guided by 
sampling-based profile into  (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..5cd821cfe780c4 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_sample_cold_function) ||
+ Args.hasArg(options::OPT_fprofile_sample_cold_function_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..f4a073d2036d12 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -649,6 +649,19 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, 
Compilation &C,
 }
   }
 
+  if (auto *SampleColdArg =
+  Args.getLastArg(options::OPT_fprofile_sample_cold_function,
+  options::OPT_fprofile_sample_cold_function_EQ)) {
+SmallString<128> Path(SampleColdArg->getOption().matches(
+  options::OPT_fprofile_sample_cold_function_EQ)
+  ? SampleColdArg->getValue()
+  : "");
+llvm::sys::path::append(Path, "default_%m.profraw");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--instrument-sample-cold-function-path=") + Path));
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 8f151a99b11709..0514d17db20721 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -296,7 +296,13 @@ static cl::opt UseLoopVersioningLICM(
 "enable-loop-versioning-licm", cl::init(false), cl::Hidden,
 cl::desc("Enable the experimental Loop Versioning LICM pass"));
 
+static cl::opt InstrumentSampleColdFuncPath(
+"instrument-sample-cold-function-path", cl::init(""),
+cl::desc("File path for instrumenting sampling PGO guided cold functions"),
+cl::Hidden);
+
 extern cl::opt UseCtxProfile;
+extern cl::opt InstrumentColdFunction;
 
 namespace llvm {
 extern cl::opt EnableMemProfContextDisambiguation;
@@ -1119,6 +1125,17 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // removed.
   MPM.addPass(
   PGOIndirectCallPromotion(true /* IsInLTO */, true /* SamplePGO */));
+
+if (InstrumentSampleColdFuncPath.getNumOccurrences() &&
+Phase != ThinOrFullLTOPhase

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

[Lei Wang via cfe-commits]

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

>From 36a747b5ef2123dd10909b91ad963e30af63b3b8 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH] [InstrPGO] Instrument sampling profile based cold function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 12 ++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 17 +
 .../Instrumentation/PGOInstrumentation.cpp| 14 +++
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 6 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..0ac289089839d2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_sample_cold_function : Flag<["-"], 
"fprofile-sample-cold-function">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions guided by 
sampling-based profile into default.profraw file (overridden by '=' form of 
option or LLVM_PROFILE_FILE env var)">;
+def fprofile_sample_cold_function_EQ : Joined<["-"], 
"fprofile-sample-cold-function=">, 
+Group, Visibility<[ClangOption, CLOption]>, MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions guided by 
sampling-based profile into  (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..5cd821cfe780c4 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_sample_cold_function) ||
+ Args.hasArg(options::OPT_fprofile_sample_cold_function_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..6e128dfc888ab4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -649,6 +649,18 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, 
Compilation &C,
 }
   }
 
+  if (auto *SampleColdArg =
+  Args.getLastArg(options::OPT_fprofile_sample_cold_function,
+  options::OPT_fprofile_sample_cold_function_EQ)) {
+SmallString<128> Path(SampleColdArg->getOption().matches(
+  options::OPT_fprofile_sample_cold_function_EQ)
+  ? SampleColdArg->getValue()
+  : "default_%m.profraw");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--instrument-sample-cold-function-path=") + Path));
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 8f151a99b11709..0514d17db20721 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -296,7 +296,13 @@ static cl::opt UseLoopVersioningLICM(
 "enable-loop-versioning-licm", cl::init(false), cl::Hidden,
 cl::desc("Enable the experimental Loop Versioning LICM pass"));
 
+static cl::opt InstrumentSampleColdFuncPath(
+"instrument-sample-cold-function-path", cl::init(""),
+cl::desc("File path for instrumenting sampling PGO guided cold functions"),
+cl::Hidden);
+
 extern cl::opt UseCtxProfile;
+extern cl::opt InstrumentColdFunction;
 
 namespace llvm {
 extern cl::opt EnableMemProfContextDisambiguation;
@@ -1119,6 +1125,17 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // removed.
   MPM.addPass(
   PGOIndirectCallPromotion(true /* IsInLTO */, true /* SamplePGO */));
+
+if (InstrumentSampleColdFuncPath.getNumOccurrences() &&
+Phase != ThinOrFullLTOPhase::ThinLTOPostLink) {
+  assert(!Inst

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

[Lei Wang via cfe-commits]

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

>From 6a9b51130269975b5832a5439897dcb34b9dc942 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH] [InstrPGO] Instrument sampling profile based cold function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 10 
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 17 +
 .../Instrumentation/PGOInstrumentation.cpp| 14 +++
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 6 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..0ac289089839d2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_sample_cold_function : Flag<["-"], 
"fprofile-sample-cold-function">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions guided by 
sampling-based profile into default.profraw file (overridden by '=' form of 
option or LLVM_PROFILE_FILE env var)">;
+def fprofile_sample_cold_function_EQ : Joined<["-"], 
"fprofile-sample-cold-function=">, 
+Group, Visibility<[ClangOption, CLOption]>, MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions guided by 
sampling-based profile into  (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..5cd821cfe780c4 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_sample_cold_function) ||
+ Args.hasArg(options::OPT_fprofile_sample_cold_function_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..2b3e0047412d3e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -649,6 +649,16 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, 
Compilation &C,
 }
   }
 
+  if (auto *SampleColdArg =
+  Args.getLastArg(options::OPT_fprofile_sample_cold_function,
+  options::OPT_fprofile_sample_cold_function_EQ)) {
+SmallString<128> Path (SampleColdArg->getOption().matches(
+options::OPT_fprofile_sample_cold_function_EQ) ? 
SampleColdArg->getValue(): "default_%m.profraw");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--instrument-sample-cold-function-path=") + Path));
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 8f151a99b11709..0514d17db20721 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -296,7 +296,13 @@ static cl::opt UseLoopVersioningLICM(
 "enable-loop-versioning-licm", cl::init(false), cl::Hidden,
 cl::desc("Enable the experimental Loop Versioning LICM pass"));
 
+static cl::opt InstrumentSampleColdFuncPath(
+"instrument-sample-cold-function-path", cl::init(""),
+cl::desc("File path for instrumenting sampling PGO guided cold functions"),
+cl::Hidden);
+
 extern cl::opt UseCtxProfile;
+extern cl::opt InstrumentColdFunction;
 
 namespace llvm {
 extern cl::opt EnableMemProfContextDisambiguation;
@@ -1119,6 +1125,17 @@ 
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 ins

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

[Lei Wang via cfe-commits]

https://github.com/wlei-llvm edited 
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

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

[via cfe-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff f4042077e2e3946ee35c1df8cab8237de6086480 
b693ff3c33dcf21c1343d9d3432b1d7410a8b579 --extensions cpp -- 
clang/lib/Driver/ToolChain.cpp clang/lib/Driver/ToolChains/Clang.cpp 
llvm/lib/Passes/PassBuilderPipelines.cpp 
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 85e9e02ca8..26123bbf00 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -653,14 +653,15 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, 
Compilation &C,
   Args.getLastArg(options::OPT_fprofile_sample_cold_function,
   options::OPT_fprofile_sample_cold_function_EQ)) {
 SmallString<128> Path;
-if 
(SampleColdArg->getOption().matches(options::OPT_fprofile_sample_cold_function_EQ))
+if (SampleColdArg->getOption().matches(
+options::OPT_fprofile_sample_cold_function_EQ))
   Path.append(SampleColdArg->getValue());
-else 
+else
   Path.append("default_%m.profraw");
 
 CmdArgs.push_back("-mllvm");
 CmdArgs.push_back(Args.MakeArgString(
-Twine("--instrument-sample-cold-function-path=") + Path));
+Twine("--instrument-sample-cold-function-path=") + Path));
   }
 
   Arg *PGOGenArg = nullptr;
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 5d1d8480b1..0514d17db2 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1125,8 +1125,9 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // removed.
   MPM.addPass(
   PGOIndirectCallPromotion(true /* IsInLTO */, true /* SamplePGO */));
-
-if (InstrumentSampleColdFuncPath.getNumOccurrences() && Phase != 
ThinOrFullLTOPhase::ThinLTOPostLink) {
+
+if (InstrumentSampleColdFuncPath.getNumOccurrences() &&
+Phase != ThinOrFullLTOPhase::ThinLTOPostLink) {
   assert(!InstrumentSampleColdFuncPath.empty() &&
  "File path is requeired for instrumentation generation");
   InstrumentColdFunction = true;

``




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

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

[Lei Wang via cfe-commits]

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

None

>From b693ff3c33dcf21c1343d9d3432b1d7410a8b579 Mon Sep 17 00:00:00 2001
From: wlei 
Date: Sun, 22 Sep 2024 20:23:20 -0700
Subject: [PATCH] [InstrPGO] Instrument sampling profile based cold function

---
 clang/include/clang/Driver/Options.td |  6 +
 clang/lib/Driver/ToolChain.cpp|  4 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 14 +++
 llvm/lib/Passes/PassBuilderPipelines.cpp  | 16 +
 .../Instrumentation/PGOInstrumentation.cpp| 14 +++
 .../PGOProfile/instr-gen-cold-function.ll | 24 +++
 6 files changed, 77 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/PGOProfile/instr-gen-cold-function.ll

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 002f60350543d9..0ac289089839d2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling : 
BoolFOption<"debug-info-for-profiling",
   PosFlag,
   NegFlag>;
+def fprofile_sample_cold_function : Flag<["-"], 
"fprofile-sample-cold-function">, 
+Group, Visibility<[ClangOption, CLOption]>,
+HelpText<"Generate instrumented code to cold functions guided by 
sampling-based profile into default.profraw file (overridden by '=' form of 
option or LLVM_PROFILE_FILE env var)">;
+def fprofile_sample_cold_function_EQ : Joined<["-"], 
"fprofile-sample-cold-function=">, 
+Group, Visibility<[ClangOption, CLOption]>, MetaVarName<"">,
+HelpText<"Generate instrumented code to cold functions guided by 
sampling-based profile into  (overridden by LLVM_PROFILE_FILE env var)">; 
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 16f9b629fc538c..5cd821cfe780c4 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -889,7 +889,9 @@ bool ToolChain::needsProfileRT(const ArgList &Args) {
  Args.hasArg(options::OPT_fprofile_instr_generate) ||
  Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
  Args.hasArg(options::OPT_fcreate_profile) ||
- Args.hasArg(options::OPT_forder_file_instrumentation);
+ Args.hasArg(options::OPT_forder_file_instrumentation) ||
+ Args.hasArg(options::OPT_fprofile_sample_cold_function) ||
+ Args.hasArg(options::OPT_fprofile_sample_cold_function_EQ);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..85e9e02ca8a4ee 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -649,6 +649,20 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, 
Compilation &C,
 }
   }
 
+  if (auto *SampleColdArg =
+  Args.getLastArg(options::OPT_fprofile_sample_cold_function,
+  options::OPT_fprofile_sample_cold_function_EQ)) {
+SmallString<128> Path;
+if 
(SampleColdArg->getOption().matches(options::OPT_fprofile_sample_cold_function_EQ))
+  Path.append(SampleColdArg->getValue());
+else 
+  Path.append("default_%m.profraw");
+
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--instrument-sample-cold-function-path=") + Path));
+  }
+
   Arg *PGOGenArg = nullptr;
   if (PGOGenerateArg) {
 assert(!CSPGOGenerateArg);
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 8f151a99b11709..5d1d8480b1472d 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -296,7 +296,13 @@ static cl::opt UseLoopVersioningLICM(
 "enable-loop-versioning-licm", cl::init(false), cl::Hidden,
 cl::desc("Enable the experimental Loop Versioning LICM pass"));
 
+static cl::opt InstrumentSampleColdFuncPath(
+"instrument-sample-cold-function-path", cl::init(""),
+cl::desc("File path for instrumenting sampling PGO guided cold functions"),
+cl::Hidden);
+
 extern cl::opt UseCtxProfile;
+extern cl::opt InstrumentColdFunction;
 
 namespace llvm {
 extern cl::opt EnableMemProfContextDisambiguation;
@@ -1119,6 +1125,16 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // removed.
   MPM.addPass(
   PGOIndirectCallPromotion(true /* IsInLTO */, true /* SamplePGO */));
+
+if (InstrumentSampleColdFuncPath.getNumOccurrences() && Phase != 
ThinOrFullLTOPhase::ThinLTOPostLink) {
+  assert(!InstrumentSampleColdFuncPath.e