Skip to content

[Clang][Driver][SamplePGO] Enable -fsample-profile-use-profi for SamplePGO #145957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nilanjana87
Copy link
Contributor

@nilanjana87 nilanjana87 commented Jun 26, 2025

  • Enable -fsample-profile-use-profi flag by default for SamplePGO
  • Add -fno_sample_profile_use_profi flag for opting out

Discourse conversation: https://discourse.llvm.org/t/turn-on-fsample-profile-use-profi-by-default/86508/2

@nilanjana87 nilanjana87 marked this pull request as ready for review June 26, 2025 19:49
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jun 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-pgo
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Nilanjana Basu (nilanjana87)

Changes
  • Enable -fsample-profile-use-profi flag by default for SamplePGO
  • Add -fno_sample_profile_use_profi flag for opting out

Discourse conversation: https://discourse.llvm.org/t/turn-on-fsample-profile-use-profi-by-default/86508/2


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

3 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+2)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+2-1)
  • (modified) clang/test/Driver/pgo-sample-use-profi.c (+17-2)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0822df2640d23..ef6bcfe296a80 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1698,6 +1698,8 @@ def fsample_profile_use_profi : Flag<["-"], "fsample-profile-use-profi">,
                basic block counts to branch probabilities to fix them by extended
                and re-engineered classic MCMF (min-cost max-flow) approach.}]>;
 def fno_profile_sample_accurate : Flag<["-"], "fno-profile-sample-accurate">, Group<f_Group>;
+def fno_sample_profile_use_profi : Flag<["-"], "fno-sample-profile-use-profi">,
+                                   Group<f_Group>;
 def fno_auto_profile : Flag<["-"], "fno-auto-profile">, Group<f_Group>,
     Alias<fno_profile_sample_use>;
 def fauto_profile_EQ : Joined<["-"], "fauto-profile=">,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 8a18865b899d2..7898c0a6a899c 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6283,7 +6283,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   Args.AddLastArg(CmdArgs, options::OPT_fclang_abi_compat_EQ);
 
   if (getLastProfileSampleUseArg(Args) &&
-      Args.hasArg(options::OPT_fsample_profile_use_profi)) {
+      Args.hasFlag(options::OPT_fsample_profile_use_profi,
+                   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 0370d947ce590..9d7c7af983508 100644
--- a/clang/test/Driver/pgo-sample-use-profi.c
+++ b/clang/test/Driver/pgo-sample-use-profi.c
@@ -1,4 +1,19 @@
-/// Test if profi flat is enabled in frontend as user-facing feature.
-// RUN: %clang --target=x86_64 -c -fsample-profile-use-profi -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
+/// Ensure that profi flag is enabled by default in frontend for SamplePGO
+
+// Target specific checks:
+// 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
+// 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
+
+// Cases where profi flag is disabled:
+// RUN: %clang -c -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
+// RUN: %clang -c -fno-sample-profile-use-profi -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
+// RUN: %clang -c -fsample-profile-use-profi -fno-sample-profile-use-profi -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-PROFI
+
 
 // CHECK: "-mllvm" "-sample-profile-use-profi"
+// CHECK-NO-PROFI-NOT: "-sample-profile-use-profi"

@nilanjana87 nilanjana87 added the PGO Profile Guided Optimizations label Jun 27, 2025
@teresajohnson teresajohnson requested a review from snehasish June 27, 2025 14:49
@snehasish
Copy link
Contributor

We evaluated profi internally (at Google) last year. Our configuration uses AutoFDO with flow-sensitive discriminators (FS-AFDO). We found slight regressions with this configuration and didn't enable it for our workloads. With FS-AFDO we see performance improvements of ~1%. Can you share your configuration and performance numbers?

@teresajohnson teresajohnson requested a review from amharc June 27, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants