[InstrProf] Add frontend temporal profiling flag #122385
Open
+53
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Ellis Hoag (ellishg) ChangesAs discussed in #121514 add the frontent flag Full diff: https://github.com/llvm/llvm-project/pull/122385.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 52823430919de4..7df5141499552c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1796,6 +1796,9 @@ def fprofile_generate_cold_function_coverage : Flag<["-"], "fprofile-generate-co
def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], "fprofile-generate-cold-function-coverage=">,
Group<f_Group>, Visibility<[ClangOption, CLOption]>, MetaVarName<"<directory>">,
HelpText<"Generate instrumented code to collect coverage info for cold functions into <directory>/default.profraw (overridden by LLVM_PROFILE_FILE env var)">;
+def fprofile_generate_temporal_instrumentation : Flag<["-"], "fprofile-generate-temporal-instrumentation">,
+ Group<f_Group>, Visibility<[ClangOption, CLOption]>,
+ HelpText<"Generate instrumented code to collect temporal information. See this RFC for details: https://discourse.llvm.org/t/rfc-temporal-profiling-extension-for-irpgo/68068">;
def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
Group<f_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)">;
@@ -1891,7 +1894,7 @@ defm pseudo_probe_for_profiling : BoolFOption<"pseudo-probe-for-profiling",
" pseudo probes for sample profiling">>;
def forder_file_instrumentation : Flag<["-"], "forder-file-instrumentation">,
Group<f_Group>, Visibility<[ClangOption, CC1Option, CLOption]>,
- HelpText<"Generate instrumented code to collect order file into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var). Deprecated, please use temporal profiling.">;
+ HelpText<"Generate instrumented code to collect order file into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var). Deprecated, please use -fprofile-generate-temporal-instrumentation">;
def fprofile_list_EQ : Joined<["-"], "fprofile-list=">,
Group<f_Group>, Visibility<[ClangOption, CC1Option, CLOption]>,
HelpText<"Filename defining the list of functions/files to instrument. "
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index a0002371da2f1b..a196bceba1f2d3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -662,6 +662,15 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
CmdArgs.push_back("--pgo-function-entry-coverage");
}
+ if (auto *A = Args.getLastArg(
+ options::OPT_fprofile_generate_temporal_instrumentation)) {
+ if (!PGOGenerateArg && !CSPGOGenerateArg)
+ D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+ << A->getSpelling() << "-fprofile-generate or -fcs-profile-generate";
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back("--pgo-temporal-instrumentation");
+ }
+
Arg *PGOGenArg = nullptr;
if (PGOGenerateArg) {
assert(!CSPGOGenerateArg);
@@ -8050,7 +8059,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.getLastArg(options::OPT_forder_file_instrumentation)) {
D.Diag(diag::warn_drv_deprecated_arg)
<< A->getAsString(Args) << /*hasReplacement=*/true
- << "-mllvm -pgo-temporal-instrumentation";
+ << "-fprofile-generate-temporal-instrumentation";
CmdArgs.push_back("-forder-file-instrumentation");
// Enable order file instrumentation when ThinLTO is not on. When ThinLTO is
// on, we need to pass these flags as linker flags and that will be handled
diff --git a/clang/test/Driver/clang_f_opts.c b/clang/test/Driver/clang_f_opts.c
index 2b72068eae1eeb..7448076cf367e5 100644
--- a/clang/test/Driver/clang_f_opts.c
+++ b/clang/test/Driver/clang_f_opts.c
@@ -424,7 +424,7 @@
// CHECK-WARNING-DAG: optimization flag '-fno-devirtualize-speculatively' is not supported
// CHECK-WARNING-DAG: the flag '-fslp-vectorize-aggressive' has been deprecated and will be ignored
// CHECK-WARNING-DAG: the flag '-fno-slp-vectorize-aggressive' has been deprecated and will be ignored
-// CHECK-WARNING-DAG: argument '-forder-file-instrumentation' is deprecated, use '-mllvm -pgo-temporal-instrumentation' instead
+// CHECK-WARNING-DAG: argument '-forder-file-instrumentation' is deprecated, use '-fprofile-generate-temporal-instrumentation' instead
// Test that we mute the warning on these
// RUN: %clang -### -finline-limit=1000 -Wno-invalid-command-line-argument \
diff --git a/clang/test/Driver/fprofile-generate-temporal-instrumentation.c b/clang/test/Driver/fprofile-generate-temporal-instrumentation.c
new file mode 100644
index 00000000000000..e4e5b5f49bcacf
--- /dev/null
+++ b/clang/test/Driver/fprofile-generate-temporal-instrumentation.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### -c -fprofile-generate -fprofile-generate-temporal-instrumentation %s 2>&1 | FileCheck %s
+// RUN: %clang -### -c -fcs-profile-generate -fprofile-generate-temporal-instrumentation %s 2>&1 | FileCheck %s
+// RUN: not %clang -### -c -fprofile-generate-temporal-instrumentation %s 2>&1 | FileCheck %s --check-prefix=ERR
+
+// CHECK: "-mllvm" "--pgo-temporal-instrumentation"
+
+// ERR: '-fprofile-generate-temporal-instrumentation' only allowed with '-fprofile-generate or -fcs-profile-generate'
|
|
@llvm/pr-subscribers-clang-driver Author: Ellis Hoag (ellishg) ChangesAs discussed in #121514 add the frontent flag Full diff: https://github.com/llvm/llvm-project/pull/122385.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 52823430919de4..7df5141499552c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1796,6 +1796,9 @@ def fprofile_generate_cold_function_coverage : Flag<["-"], "fprofile-generate-co
def fprofile_generate_cold_function_coverage_EQ : Joined<["-"], "fprofile-generate-cold-function-coverage=">,
Group<f_Group>, Visibility<[ClangOption, CLOption]>, MetaVarName<"<directory>">,
HelpText<"Generate instrumented code to collect coverage info for cold functions into <directory>/default.profraw (overridden by LLVM_PROFILE_FILE env var)">;
+def fprofile_generate_temporal_instrumentation : Flag<["-"], "fprofile-generate-temporal-instrumentation">,
+ Group<f_Group>, Visibility<[ClangOption, CLOption]>,
+ HelpText<"Generate instrumented code to collect temporal information. See this RFC for details: https://discourse.llvm.org/t/rfc-temporal-profiling-extension-for-irpgo/68068">;
def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
Group<f_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)">;
@@ -1891,7 +1894,7 @@ defm pseudo_probe_for_profiling : BoolFOption<"pseudo-probe-for-profiling",
" pseudo probes for sample profiling">>;
def forder_file_instrumentation : Flag<["-"], "forder-file-instrumentation">,
Group<f_Group>, Visibility<[ClangOption, CC1Option, CLOption]>,
- HelpText<"Generate instrumented code to collect order file into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var). Deprecated, please use temporal profiling.">;
+ HelpText<"Generate instrumented code to collect order file into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var). Deprecated, please use -fprofile-generate-temporal-instrumentation">;
def fprofile_list_EQ : Joined<["-"], "fprofile-list=">,
Group<f_Group>, Visibility<[ClangOption, CC1Option, CLOption]>,
HelpText<"Filename defining the list of functions/files to instrument. "
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index a0002371da2f1b..a196bceba1f2d3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -662,6 +662,15 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
CmdArgs.push_back("--pgo-function-entry-coverage");
}
+ if (auto *A = Args.getLastArg(
+ options::OPT_fprofile_generate_temporal_instrumentation)) {
+ if (!PGOGenerateArg && !CSPGOGenerateArg)
+ D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+ << A->getSpelling() << "-fprofile-generate or -fcs-profile-generate";
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back("--pgo-temporal-instrumentation");
+ }
+
Arg *PGOGenArg = nullptr;
if (PGOGenerateArg) {
assert(!CSPGOGenerateArg);
@@ -8050,7 +8059,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.getLastArg(options::OPT_forder_file_instrumentation)) {
D.Diag(diag::warn_drv_deprecated_arg)
<< A->getAsString(Args) << /*hasReplacement=*/true
- << "-mllvm -pgo-temporal-instrumentation";
+ << "-fprofile-generate-temporal-instrumentation";
CmdArgs.push_back("-forder-file-instrumentation");
// Enable order file instrumentation when ThinLTO is not on. When ThinLTO is
// on, we need to pass these flags as linker flags and that will be handled
diff --git a/clang/test/Driver/clang_f_opts.c b/clang/test/Driver/clang_f_opts.c
index 2b72068eae1eeb..7448076cf367e5 100644
--- a/clang/test/Driver/clang_f_opts.c
+++ b/clang/test/Driver/clang_f_opts.c
@@ -424,7 +424,7 @@
// CHECK-WARNING-DAG: optimization flag '-fno-devirtualize-speculatively' is not supported
// CHECK-WARNING-DAG: the flag '-fslp-vectorize-aggressive' has been deprecated and will be ignored
// CHECK-WARNING-DAG: the flag '-fno-slp-vectorize-aggressive' has been deprecated and will be ignored
-// CHECK-WARNING-DAG: argument '-forder-file-instrumentation' is deprecated, use '-mllvm -pgo-temporal-instrumentation' instead
+// CHECK-WARNING-DAG: argument '-forder-file-instrumentation' is deprecated, use '-fprofile-generate-temporal-instrumentation' instead
// Test that we mute the warning on these
// RUN: %clang -### -finline-limit=1000 -Wno-invalid-command-line-argument \
diff --git a/clang/test/Driver/fprofile-generate-temporal-instrumentation.c b/clang/test/Driver/fprofile-generate-temporal-instrumentation.c
new file mode 100644
index 00000000000000..e4e5b5f49bcacf
--- /dev/null
+++ b/clang/test/Driver/fprofile-generate-temporal-instrumentation.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### -c -fprofile-generate -fprofile-generate-temporal-instrumentation %s 2>&1 | FileCheck %s
+// RUN: %clang -### -c -fcs-profile-generate -fprofile-generate-temporal-instrumentation %s 2>&1 | FileCheck %s
+// RUN: not %clang -### -c -fprofile-generate-temporal-instrumentation %s 2>&1 | FileCheck %s --check-prefix=ERR
+
+// CHECK: "-mllvm" "--pgo-temporal-instrumentation"
+
+// ERR: '-fprofile-generate-temporal-instrumentation' only allowed with '-fprofile-generate or -fcs-profile-generate'
|
david-xl
reviewed
Jan 9, 2025
carlocab
reviewed
Jan 10, 2025
| @@ -3035,6 +3035,38 @@ indexed format, regardeless whether it is produced by frontend or the IR pass. | |||
| overhead. ``prefer-atomic`` will be transformed to ``atomic`` when supported | |||
| by the target, or ``single`` otherwise. | |||
|
|
|||
| .. option:: -fprofile-generate-temporal | |||
There was a problem hiding this comment.
Mild suggestion: -fgenerate-temporal-profile or -ftemporal-profile-generate seems less awkward to me.
There was a problem hiding this comment.
I somewhat agree, but I think it's more important to keep consistent with the other flags. What do others think?
There was a problem hiding this comment.
That's fair. Maybe -fprofile-temporal-generate? Putting temporal closer to profile seems like an improvement. But I'm fine with leaving things as-is if others feel strongly about it.
There was a problem hiding this comment.
Since this option is used together with the primary option -fprofile-generate, the naming consistency is not important. I suggest making it even simpler: -ftemporal-profile
MaskRay
reviewed
Jan 10, 2025
|
|
||
| .. code-block:: console | ||
|
|
||
| $ clang++ -O2 -Wl,--irpgo-profile=code.profdata,--bp-startup-sort=function code.cc -o code |
There was a problem hiding this comment.
-fuse-ld=lld for ELF. (Other linkers will unlikely get the feature in the short time)
You probably want to mention the Mach-O way as well
|
|
||
| // CHECK: "-mllvm" "--pgo-temporal-instrumentation" | ||
|
|
||
| // ERR: '-fprofile-generate-temporal' only allowed with '-fprofile-generate or -fcs-profile-generate' |
There was a problem hiding this comment.
Include error: for driver error messages
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed in #121514 add the frontent flag
-fprofile-generate-temporal-instrumentationto enable temporal profiling (https://discourse.llvm.org/t/rfc-temporal-profiling-extension-for-irpgo/68068) as a replacement for-forder-file-instrumentation(https://discourse.llvm.org/t/deprecate-forder-file-instrumentation-in-favor-of-temporal-profiling/83903)