[Coverage] Support -fprofile-list for cold function coverage#136333
[Coverage] Support -fprofile-list for cold function coverage#136333
Conversation
|
@llvm/pr-subscribers-clang-driver Author: Lei Wang (wlei-llvm) ChangesAdd a new instrumentation section type Full diff: https://github.com/llvm/llvm-project/pull/136333.diff 8 Files Affected:
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 69256527f40c9..47bd591d20a27 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -3394,9 +3394,9 @@ This can be done using the ``-fprofile-list`` option.
$ clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code
-Supported sections are ``[clang]``, ``[llvm]``, and ``[csllvm]`` representing
-clang PGO, IRPGO, and CSIRPGO, respectively. Supported prefixes are ``function``
-and ``source``. Supported categories are ``allow``, ``skip``, and ``forbid``.
+Supported sections are ``[clang]``, ``[llvm]``, ``[csllvm]``, and ``[coldcov]`` representing
+clang PGO, IRPGO, CSIRPGO and cold function coverage, respectively. Supported prefixes are
+``function`` and ``source``. Supported categories are ``allow``, ``skip``, and ``forbid``.
``skip`` adds the ``skipprofile`` attribute while ``forbid`` adds the
``noprofile`` attribute to the appropriate function. Use
``default:<allow|skip|forbid>`` to specify the default category.
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index c5990fb248689..fced5d7dcf6b7 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -223,7 +223,7 @@ AFFECTING_VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is
CODEGENOPT(AtomicProfileUpdate , 1, 0) ///< Set -fprofile-update=atomic
CODEGENOPT(ContinuousProfileSync, 1, 0) ///< Enable continuous instrumentation profiling
/// Choose profile instrumenation kind or no instrumentation.
-ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 2, ProfileNone)
+ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 4, ProfileNone)
/// Choose profile kind for PGO use compilation.
ENUM_CODEGENOPT(ProfileUse, ProfileInstrKind, 2, ProfileNone)
/// Partition functions into N groups and select only functions in group i to be
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index e39a73bdb13ac..60001cfc62218 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -86,6 +86,8 @@ class CodeGenOptions : public CodeGenOptionsBase {
// to use with PGO.
ProfileIRInstr, // IR level PGO instrumentation in LLVM.
ProfileCSIRInstr, // IR level PGO context sensitive instrumentation in LLVM.
+ ProfileIRColdCov, // IR level cold function coverage instrumentation in
+ // LLVM.
};
enum EmbedBitcodeKind {
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 830d3459a1320..a76f90efc4a51 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -7679,9 +7679,9 @@ def fpatchable_function_entry_section_EQ
HelpText<"Use Section instead of __patchable_function_entries">,
MarshallingInfoString<CodeGenOpts<"PatchableFunctionEntrySection">>;
def fprofile_instrument_EQ : Joined<["-"], "fprofile-instrument=">,
- HelpText<"Enable PGO instrumentation">, Values<"none,clang,llvm,csllvm">,
+ HelpText<"Enable PGO instrumentation">, Values<"none,clang,llvm,csllvm,coldcov">,
NormalizedValuesScope<"CodeGenOptions">,
- NormalizedValues<["ProfileNone", "ProfileClangInstr", "ProfileIRInstr", "ProfileCSIRInstr"]>,
+ NormalizedValues<["ProfileNone", "ProfileClangInstr", "ProfileIRInstr", "ProfileCSIRInstr", "ProfileIRColdCov"]>,
MarshallingInfoEnum<CodeGenOpts<"ProfileInstr">, "ProfileNone">;
def fprofile_instrument_path_EQ : Joined<["-"], "fprofile-instrument-path=">,
HelpText<"Generate instrumented code to collect execution counts into "
diff --git a/clang/lib/Basic/ProfileList.cpp b/clang/lib/Basic/ProfileList.cpp
index 8fa16e2eb069a..0991867e1ac3a 100644
--- a/clang/lib/Basic/ProfileList.cpp
+++ b/clang/lib/Basic/ProfileList.cpp
@@ -81,6 +81,8 @@ static StringRef getSectionName(CodeGenOptions::ProfileInstrKind Kind) {
return "llvm";
case CodeGenOptions::ProfileCSIRInstr:
return "csllvm";
+ case CodeGenOptions::ProfileIRColdCov:
+ return "coldcov";
}
llvm_unreachable("Unhandled CodeGenOptions::ProfileInstrKind enum");
}
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 67a800a643cbe..d9e7a6561856d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -628,6 +628,7 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
CmdArgs.push_back("--pgo-instrument-cold-function-only");
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("--pgo-function-entry-coverage");
+ CmdArgs.push_back("-fprofile-instrument=coldcov");
}
if (auto *A = Args.getLastArg(options::OPT_ftemporal_profile)) {
diff --git a/clang/test/CodeGen/profile-filter.c b/clang/test/CodeGen/profile-filter.c
index e33e4a0a60b3d..d62871d95c393 100644
--- a/clang/test/CodeGen/profile-filter.c
+++ b/clang/test/CodeGen/profile-filter.c
@@ -9,6 +9,9 @@
// RUN: echo -e "[clang]\nfun:test1\n[llvm]\nfun:test2" > %t-section.list
// RUN: %clang_cc1 -fprofile-instrument=llvm -fprofile-list=%t-section.list -emit-llvm %s -o - | FileCheck %s --check-prefix=SECTION
+// RUN: echo -e "[coldcov]\nfun:test*\n!fun:test2" > %t-cold-func.list
+// RUN: %clang_cc1 -fprofile-instrument=coldcov -fprofile-list=%t-cold-func.list -emit-llvm %s -o - | FileCheck %s --check-prefix=COLDCOV
+
// RUN: echo -e "fun:test*\n!fun:test1" > %t-exclude.list
// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fprofile-list=%t-exclude.list -emit-llvm %s -o - | FileCheck %s --check-prefix=EXCLUDE
@@ -36,6 +39,8 @@ unsigned i;
// SECTION: @test1
// EXCLUDE: noprofile
// EXCLUDE: @test1
+// COLDCOV-NOT: noprofile
+// COLDCOV: @test1
unsigned test1(void) {
// CHECK: %pgocount = load i64, ptr @__profc_{{_?}}test1
// FUNC: %pgocount = load i64, ptr @__profc_{{_?}}test1
@@ -55,6 +60,8 @@ unsigned test1(void) {
// SECTION: @test2
// EXCLUDE-NOT: noprofile
// EXCLUDE: @test2
+// COLDCOV: noprofile
+// COLDCOV: @test2
unsigned test2(void) {
// CHECK: %pgocount = load i64, ptr @__profc_{{_?}}test2
// FUNC-NOT: %pgocount = load i64, ptr @__profc_{{_?}}test2
diff --git a/clang/test/Driver/fprofile-generate-cold-function-coverage.c b/clang/test/Driver/fprofile-generate-cold-function-coverage.c
index 135acf2e736f7..834150415e28d 100644
--- a/clang/test/Driver/fprofile-generate-cold-function-coverage.c
+++ b/clang/test/Driver/fprofile-generate-cold-function-coverage.c
@@ -2,7 +2,7 @@
// CHECK: "--instrument-cold-function-only-path=default_%m.profraw"
// CHECK: "--pgo-instrument-cold-function-only"
// CHECK: "--pgo-function-entry-coverage"
-// CHECK-NOT: "-fprofile-instrument"
+// CHECK: "-fprofile-instrument=coldcov"
// CHECK-NOT: "-fprofile-instrument-path=
// RUN: %clang -### -c -fprofile-generate-cold-function-coverage=dir %s 2>&1 | FileCheck %s --check-prefix=CHECK-EQ
|
|
@llvm/pr-subscribers-clang Author: Lei Wang (wlei-llvm) ChangesAdd a new instrumentation section type Full diff: https://github.com/llvm/llvm-project/pull/136333.diff 8 Files Affected:
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 69256527f40c9..47bd591d20a27 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -3394,9 +3394,9 @@ This can be done using the ``-fprofile-list`` option.
$ clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code
-Supported sections are ``[clang]``, ``[llvm]``, and ``[csllvm]`` representing
-clang PGO, IRPGO, and CSIRPGO, respectively. Supported prefixes are ``function``
-and ``source``. Supported categories are ``allow``, ``skip``, and ``forbid``.
+Supported sections are ``[clang]``, ``[llvm]``, ``[csllvm]``, and ``[coldcov]`` representing
+clang PGO, IRPGO, CSIRPGO and cold function coverage, respectively. Supported prefixes are
+``function`` and ``source``. Supported categories are ``allow``, ``skip``, and ``forbid``.
``skip`` adds the ``skipprofile`` attribute while ``forbid`` adds the
``noprofile`` attribute to the appropriate function. Use
``default:<allow|skip|forbid>`` to specify the default category.
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index c5990fb248689..fced5d7dcf6b7 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -223,7 +223,7 @@ AFFECTING_VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is
CODEGENOPT(AtomicProfileUpdate , 1, 0) ///< Set -fprofile-update=atomic
CODEGENOPT(ContinuousProfileSync, 1, 0) ///< Enable continuous instrumentation profiling
/// Choose profile instrumenation kind or no instrumentation.
-ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 2, ProfileNone)
+ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 4, ProfileNone)
/// Choose profile kind for PGO use compilation.
ENUM_CODEGENOPT(ProfileUse, ProfileInstrKind, 2, ProfileNone)
/// Partition functions into N groups and select only functions in group i to be
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index e39a73bdb13ac..60001cfc62218 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -86,6 +86,8 @@ class CodeGenOptions : public CodeGenOptionsBase {
// to use with PGO.
ProfileIRInstr, // IR level PGO instrumentation in LLVM.
ProfileCSIRInstr, // IR level PGO context sensitive instrumentation in LLVM.
+ ProfileIRColdCov, // IR level cold function coverage instrumentation in
+ // LLVM.
};
enum EmbedBitcodeKind {
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 830d3459a1320..a76f90efc4a51 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -7679,9 +7679,9 @@ def fpatchable_function_entry_section_EQ
HelpText<"Use Section instead of __patchable_function_entries">,
MarshallingInfoString<CodeGenOpts<"PatchableFunctionEntrySection">>;
def fprofile_instrument_EQ : Joined<["-"], "fprofile-instrument=">,
- HelpText<"Enable PGO instrumentation">, Values<"none,clang,llvm,csllvm">,
+ HelpText<"Enable PGO instrumentation">, Values<"none,clang,llvm,csllvm,coldcov">,
NormalizedValuesScope<"CodeGenOptions">,
- NormalizedValues<["ProfileNone", "ProfileClangInstr", "ProfileIRInstr", "ProfileCSIRInstr"]>,
+ NormalizedValues<["ProfileNone", "ProfileClangInstr", "ProfileIRInstr", "ProfileCSIRInstr", "ProfileIRColdCov"]>,
MarshallingInfoEnum<CodeGenOpts<"ProfileInstr">, "ProfileNone">;
def fprofile_instrument_path_EQ : Joined<["-"], "fprofile-instrument-path=">,
HelpText<"Generate instrumented code to collect execution counts into "
diff --git a/clang/lib/Basic/ProfileList.cpp b/clang/lib/Basic/ProfileList.cpp
index 8fa16e2eb069a..0991867e1ac3a 100644
--- a/clang/lib/Basic/ProfileList.cpp
+++ b/clang/lib/Basic/ProfileList.cpp
@@ -81,6 +81,8 @@ static StringRef getSectionName(CodeGenOptions::ProfileInstrKind Kind) {
return "llvm";
case CodeGenOptions::ProfileCSIRInstr:
return "csllvm";
+ case CodeGenOptions::ProfileIRColdCov:
+ return "coldcov";
}
llvm_unreachable("Unhandled CodeGenOptions::ProfileInstrKind enum");
}
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 67a800a643cbe..d9e7a6561856d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -628,6 +628,7 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
CmdArgs.push_back("--pgo-instrument-cold-function-only");
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("--pgo-function-entry-coverage");
+ CmdArgs.push_back("-fprofile-instrument=coldcov");
}
if (auto *A = Args.getLastArg(options::OPT_ftemporal_profile)) {
diff --git a/clang/test/CodeGen/profile-filter.c b/clang/test/CodeGen/profile-filter.c
index e33e4a0a60b3d..d62871d95c393 100644
--- a/clang/test/CodeGen/profile-filter.c
+++ b/clang/test/CodeGen/profile-filter.c
@@ -9,6 +9,9 @@
// RUN: echo -e "[clang]\nfun:test1\n[llvm]\nfun:test2" > %t-section.list
// RUN: %clang_cc1 -fprofile-instrument=llvm -fprofile-list=%t-section.list -emit-llvm %s -o - | FileCheck %s --check-prefix=SECTION
+// RUN: echo -e "[coldcov]\nfun:test*\n!fun:test2" > %t-cold-func.list
+// RUN: %clang_cc1 -fprofile-instrument=coldcov -fprofile-list=%t-cold-func.list -emit-llvm %s -o - | FileCheck %s --check-prefix=COLDCOV
+
// RUN: echo -e "fun:test*\n!fun:test1" > %t-exclude.list
// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fprofile-list=%t-exclude.list -emit-llvm %s -o - | FileCheck %s --check-prefix=EXCLUDE
@@ -36,6 +39,8 @@ unsigned i;
// SECTION: @test1
// EXCLUDE: noprofile
// EXCLUDE: @test1
+// COLDCOV-NOT: noprofile
+// COLDCOV: @test1
unsigned test1(void) {
// CHECK: %pgocount = load i64, ptr @__profc_{{_?}}test1
// FUNC: %pgocount = load i64, ptr @__profc_{{_?}}test1
@@ -55,6 +60,8 @@ unsigned test1(void) {
// SECTION: @test2
// EXCLUDE-NOT: noprofile
// EXCLUDE: @test2
+// COLDCOV: noprofile
+// COLDCOV: @test2
unsigned test2(void) {
// CHECK: %pgocount = load i64, ptr @__profc_{{_?}}test2
// FUNC-NOT: %pgocount = load i64, ptr @__profc_{{_?}}test2
diff --git a/clang/test/Driver/fprofile-generate-cold-function-coverage.c b/clang/test/Driver/fprofile-generate-cold-function-coverage.c
index 135acf2e736f7..834150415e28d 100644
--- a/clang/test/Driver/fprofile-generate-cold-function-coverage.c
+++ b/clang/test/Driver/fprofile-generate-cold-function-coverage.c
@@ -2,7 +2,7 @@
// CHECK: "--instrument-cold-function-only-path=default_%m.profraw"
// CHECK: "--pgo-instrument-cold-function-only"
// CHECK: "--pgo-function-entry-coverage"
-// CHECK-NOT: "-fprofile-instrument"
+// CHECK: "-fprofile-instrument=coldcov"
// CHECK-NOT: "-fprofile-instrument-path=
// RUN: %clang -### -c -fprofile-generate-cold-function-coverage=dir %s 2>&1 | FileCheck %s --check-prefix=CHECK-EQ
|
|
ping:) |
ellishg
left a comment
There was a problem hiding this comment.
LGTM, but I'd like to bikeshed the name a bit. Since this is only used for the Sample PGO pipeline, should we include Sample PGO in the name?
clang/test/CodeGen/profile-filter.c
Outdated
| // RUN: echo -e "[coldcov]\nfun:test*\n!fun:test2" > %t-cold-func.list | ||
| // RUN: %clang_cc1 -fprofile-instrument=coldcov -fprofile-list=%t-cold-func.list -emit-llvm %s -o - | FileCheck %s --check-prefix=COLDCOV | ||
|
|
There was a problem hiding this comment.
I know the rest of the test uses echo, but it should be simple enough to use split-file for at least this case.
There was a problem hiding this comment.
improved the test with split-file, thanks!
clang/test/CodeGen/profile-filter.c
Outdated
| // RUN: %clang_cc1 -fprofile-instrument=llvm -fprofile-list=%t-section.list -emit-llvm %s -o - | FileCheck %s --check-prefix=SECTION | ||
|
|
||
| // RUN: echo -e "[coldcov]\nfun:test*\n!fun:test2" > %t-cold-func.list | ||
| // RUN: %clang_cc1 -fprofile-instrument=coldcov -fprofile-list=%t-cold-func.list -emit-llvm %s -o - | FileCheck %s --check-prefix=COLDCOV |
There was a problem hiding this comment.
| // RUN: %clang_cc1 -fprofile-instrument=coldcov -fprofile-list=%t-cold-func.list -emit-llvm %s -o - | FileCheck %s --check-prefix=COLDCOV | |
| // RUN: %clang_cc1 -fprofile-instrument=coldcov -fprofile-list=%t-cold-func.list -emit-llvm %s -o - | FileCheck %s --check-prefix=COLDCOV --implicit-check-not=noprofile |
Can we do this to avoid the CHECK-NOT lines?
There was a problem hiding this comment.
There is also a explicit check noprofile ("// COLDCOV: noprofile"), so I guess implicit-check-not doesn't work here.
Sounds good, I will go with |
* main: (420 commits) [AArch64] Merge scaled and unscaled narrow zero stores (llvm#136705) [RISCV] One last migration to getInsertSubvector [nfc] [flang][OpenMP] Update `do concurrent` mapping pass to use `fir.do_concurrent` op (llvm#138489) [MLIR][LLVM] Fix llvm.mlir.global mismatching print and parser order (llvm#138986) [lld][NFC] Fix minor typo in docs (llvm#138898) [RISCV] Migrate getConstant indexed insert/extract subvector to new API (llvm#139111) GlobalISel: Translate minimumnum and maximumnum (llvm#139106) [MemProf] Simplify unittest save and restore of options (llvm#139117) [BOLT][AArch64] Patch functions targeted by optional relocs (llvm#138750) [Coverage] Support -fprofile-list for cold function coverage (llvm#136333) Remove unused forward decl (llvm#139108) [AMDGPU][NFC] Get rid of OPW constants. (llvm#139074) [CIR] Upstream extract op for VectorType (llvm#138413) [mlir][xegpu] Handle scalar uniform ops in SIMT distribution. (llvm#138593) [GlobalISel][AMDGPU] Fix handling of v2i128 type for AND, OR, XOR (llvm#138574) AMDGPU][True16][CodeGen] FP_Round f64 to f16 in true16 (llvm#128911) Reland [Clang] Deprecate `__is_trivially_relocatable` (llvm#139061) [HLSL][NFC] Stricter Overload Tests (clamp,max,min,pow) (llvm#138993) [MLIR] Fixing the memref linearization size computation for non-packed memref (llvm#138922) [TableGen][NFC] Use early exit to simplify large block in emitAction. (llvm#138220) ...
Add a new instrumentation section type
[sample-coldcov]to support-fprofile-listfor sample pgo based cold function coverage. Note that the current cold function coverage is based on sampling PGO pipeline, which is incompatible with the existing [llvm] option(see PGOOptions), so we can't reuse the IR-PGO(-fprofile-instrument=llvm) flag.