[Clang][Driver][SamplePGO] Enable -fsample-profile-use-profi by default #146795
Conversation
This file contains hidden or 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
nilanjana87
requested review from
MaskRay,
amharc,
dtellenbach,
snehasish,
spupyrev and
vedantk
MaskRay
reviewed
Jul 3, 2025
snehasish
approved these changes
Jul 3, 2025
llvmbot
added
clang
|
@llvm/pr-subscribers-pgo @llvm/pr-subscribers-clang Author: Nilanjana Basu (nilanjana87) ChangesSince profile inference improves sample coverage, it can be turned on by default. Ref. to part1 of this patch: #145957 Full diff: https://github.com/llvm/llvm-project/pull/146795.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 42323b2fe63bc..96ea7e9a849cc 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6282,7 +6282,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
if (getLastProfileSampleUseArg(Args) &&
Args.hasFlag(options::OPT_fsample_profile_use_profi,
- options::OPT_fno_sample_profile_use_profi, false)) {
+ options::OPT_fno_sample_profile_use_profi, true)) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-sample-profile-use-profi");
}
diff --git a/clang/test/Driver/pgo-sample-use-profi.c b/clang/test/Driver/pgo-sample-use-profi.c
index a8c8e81f96dcb..a695cb16a2564 100644
--- a/clang/test/Driver/pgo-sample-use-profi.c
+++ b/clang/test/Driver/pgo-sample-use-profi.c
@@ -1,12 +1,12 @@
/// Test if profi flag is enabled/disabled correctly based on user-specified configuration.
-/// Ensure that profi flag is disabled by default
+/// Ensure that profi flag is enabled by default
// Target specific checks:
-// RUN: %clang --target=x86_64 -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
-// RUN: %clang --target=AArch64 -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
+// RUN: %clang --target=x86_64 -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
+// RUN: %clang --target=AArch64 -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
// Target agnostic checks:
-// RUN: %clang -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
+// RUN: %clang -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
// RUN: %clang -c -fsample-profile-use-profi -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
// RUN: %clang -c -fno-sample-profile-use-profi -fsample-profile-use-profi -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
|
|
@llvm/pr-subscribers-clang-driver Author: Nilanjana Basu (nilanjana87) ChangesSince profile inference improves sample coverage, it can be turned on by default. Ref. to part1 of this patch: #145957 Full diff: https://github.com/llvm/llvm-project/pull/146795.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 42323b2fe63bc..96ea7e9a849cc 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6282,7 +6282,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
if (getLastProfileSampleUseArg(Args) &&
Args.hasFlag(options::OPT_fsample_profile_use_profi,
- options::OPT_fno_sample_profile_use_profi, false)) {
+ options::OPT_fno_sample_profile_use_profi, true)) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-sample-profile-use-profi");
}
diff --git a/clang/test/Driver/pgo-sample-use-profi.c b/clang/test/Driver/pgo-sample-use-profi.c
index a8c8e81f96dcb..a695cb16a2564 100644
--- a/clang/test/Driver/pgo-sample-use-profi.c
+++ b/clang/test/Driver/pgo-sample-use-profi.c
@@ -1,12 +1,12 @@
/// Test if profi flag is enabled/disabled correctly based on user-specified configuration.
-/// Ensure that profi flag is disabled by default
+/// Ensure that profi flag is enabled by default
// Target specific checks:
-// RUN: %clang --target=x86_64 -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
-// RUN: %clang --target=AArch64 -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
+// RUN: %clang --target=x86_64 -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
+// RUN: %clang --target=AArch64 -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
// Target agnostic checks:
-// RUN: %clang -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
+// RUN: %clang -c -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
// RUN: %clang -c -fsample-profile-use-profi -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
// RUN: %clang -c -fno-sample-profile-use-profi -fsample-profile-use-profi -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
|
nilanjana87
force-pushed
the
users/nbasu/enable-profi-for-sample-pgo-p2
branch
from
c28bdc7 to
3d2ca80
Compare
|
Heads up: I'll merge this patch tomorrow. Let me know if any of the downstream users need more time before this goes in.
|
MaskRay
approved these changes
Jul 9, 2025
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.
Since profile inference improves sample coverage, it can be turned on by default.
Ref. to part1 of this patch: #145957
Discourse conversation: https://discourse.llvm.org/t/turn-on-fsample-profile-use-profi-by-default/86508/2