[Driver] Add -fsample-profile-use-profi

[HaoyuZhang]

Summary

This patch enable -sample-profile-use-profi in Clang frontend as user-facing
feature. By using this patch, we can use the cflag of -fsample-profile-use-profi
instead of -mllvm -sample-profile-use-profi.

[MaskRay]

[Driver] Enable profi flag as user-facing flag

Please avoid unclear term "profi" and use "-fsample-profile-use-profi" instead. The simplest subject is [Driver] Add -fsample-profile-use-profi

clang/test/CodeGen/pgo-sample-use-profi.cpp

1 ↗

(On Diff #471158)

Remove the test.

This change only needs a clang/test/Driver test showing that the option passes -sample-profile-use-profi to -cc1.

[wenlei]

I'm curious why do you need profi to be a driver flag? There're many similar tuning flags that don't have corresponding driver flag. profi is narrower than most tuning flags as it's specific to one component (profile inference) unique to sample PGO.

[HaoyuZhang]

Thank you for the comments.
The subject has been modified.

For wenlei's comment, I am working on a Chromium patch for enabling profi algorithm in Chrome OS. The reviewer Hans hope an user-facing flag can be used in Chromium BUILD.gn instead of "-mllvm" shape. With investigating that profi is not a user-facing feature yet, we choose to make it to be a driver flag. :)

clang/test/CodeGen/pgo-sample-use-profi.cpp

1 ↗

(On Diff #471158)

Thank you for the comments. So we only need to RUN the command for example:

//RUN: %clang_cc1 %s -fsample-profile-use-profi -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s

//CHECK: Running pass: VerifierPass

and without any test code.

[wenlei]

Did you see measurable perf boost with profi on autofdo? Or what motivated you to turn on profi? In most cases, profi helps when csspgo is used (instead of traditional autofdo).

[HaoyuZhang]

Sorry for not providing the enough information about our investigation. We have opened an issue to show that there are some perf boost with profi on autofdo base on Intel Alder Lake platform by experiments. Please take as reference.

[hans]

clang/include/clang/Driver/Options.td

1256

A user who reads the help message might reasonably ask "what's profi". Can you please add some documentation about it? It would also be nice with a DocBrief (see e.g. -fprofile-sample-accurate above).

clang/lib/Driver/ToolChains/Clang.cpp

5732

Perhaps consider skipping this comment, the code is clear enough.

clang/test/CodeGen/pgo-sample-use-profi.cpp

1 ↗

(On Diff #471158)

I think what MaskRay is saying is that you only need a test in clang/test/Driver/ that does "%clang -c -fsample-profile-use-profi -###" and FileCheck's that the cc1 invocation contains the right -mllvm flag.

(I assume you have tests for the actual functionality somewhere in llvm/test/)

[HaoyuZhang]

[Driver] Add -fsample-profile-use-profi

This patch enable -sample-profile-use-profi in Clang frontend as user-facing
feature. By using this patch, we can use the cflag of -fsample-profile-use-profi
instead of -mllvm -sample-profile-use-profi.

[HaoyuZhang]

Thanks for the comments. The modifications have been finished. PTAL~

clang/include/clang/Driver/Options.td

1256

Hi Hans, thank you for the comments. The DocBrief may be a good choice. I have added it in new commit.

clang/lib/Driver/ToolChains/Clang.cpp

5732

It has been deleted.

[HaoyuZhang]

Remove the prof file uploaded previous for test. No more need.

[hans]

clang/include/clang/Driver/Options.td

1257

I have to say, just reading this text I don't understand what it does.

I think a good description would start with "Infer block and edge counts " and then some kind of summary of how it does that.

I assume profile info is still needed for this (that's the input, right?) That should probably also be explained, and maybe we should warn when using -fsample-profile-use-profi without -fprofile-sample-use?

My main concern is that there's no documentation for this. How is a user supposed to learn about this feature and how it works? Why can't someone add something to https://clang.llvm.org/docs/UsersManual.html#profile-guided-optimization ? Once that is figured out, describing what the option does will probably be easy.

clang/test/CodeGen/pgo-sample-use-profi.c

1 ↗

(On Diff #471895)

This file should be under clang/test/Driver/ not CodeGen/ (since it's testing Driver functionality)

nit: There should typically be a space after the //

[HaoyuZhang]

1. Modify the DocBrief contents for fsample-profile-use-profi.
2. Add a checking for -fsample-profile-use-profi that this args only allowed with -fprofile-sample-use (has a profile).
3. Modify the test case and change the directory.

[HaoyuZhang]

clang/include/clang/Driver/Options.td

1257

Sorry for the unclear description of the DocBrief and I have do some modification.

A checking has been added for ensuring that -fsample-profile-use-profi is only allowed with fprofile-sample-use. Otherwise, there will be an error.

About the document in above link, do you want me to add some contents about using profi after the patch or invite the author of profi?

[hans]

clang/include/clang/Driver/Options.td

1257

It would ideally be added in this patch. Inviting the author of profi (I didn't realize it was a different person) sounds like a good idea.

1260

Thanks, this is much better!

(nit: s/converting/convert/)

clang/lib/Driver/ToolChains/Clang.cpp

5732

Could you use tools::getLastProfileSampleUseArg instead? It covers all the spellings of the option.

5737

I think if you did instead:

if (getLastProfileSampleUseArg(Args) && Args.hasArg(options::OPT_fsample_profile_use_profi)) {
  CmdArgs.push_back(...
}

then clang would warn about the option being unused if profile use is not enabled.

[HaoyuZhang]

1. Fix spelling grammar mistake in DocBrief.
2. Using tools::getLastProfileSampleUseArg instead of D.Diag(diag::err...)

[HaoyuZhang]

New version updated. PTAL~

clang/include/clang/Driver/Options.td

1260

Modified.

clang/lib/Driver/ToolChains/Clang.cpp

5737

Thank you for the guidance. Updated.

[hans]

lgtm

[HaoyuZhang]

1. add the documentation for this driver.
2. fix a small grammer mistake in Options.td.

[HaoyuZhang]

Hi @spupyrev , the using of profi algorithm is added as a user-facing flag in this patch. And I wrote the documentation in https://clang.llvm.org/docs/UsersManual.html#profile-guided-optimization. Do you have any comments about these?

Hi @MaskRay , some updates have been submitted. PTAL~

[HaoyuZhang]

clang/include/clang/Driver/Options.td

1257

Hi Hans, I have added the documentation about profi in this patch and also invited the author to take a look.

[spupyrev]

Thanks @HaoyuZhang for adding the flag and the documentation. I just edited the
message a bit; otherwise looks good.

clang/docs/UsersManual.rst

2244

[OPTIONAL] Sampling-based profiles can have inaccuracies or missing block/edge
counters. The profile inference algorithm (profi) can be used to infer missing
block and edge counts and improve the quality of profile data. To apply the
optimization, add an extra flag `-fsample-profile-use-profi` to the command
line.

[MaskRay]

clang/docs/UsersManual.rst

2246

This sentence is unnecessarily long. Just say Enable it with -fsample-profile-use-profi. (with correct backquotes)

clang/include/clang/Driver/Options.td

1256

Omit trailing . in HelpText.

clang/test/Driver/pgo-sample-use-profi.c

2

Optional: ///

Make non-RUN non-CHECK comments stand out and help tools finding unused/incorrect prefixes.

3

// => ``

[HaoyuZhang]

1. modified the documentation.
2. omit '.' in HelpText.
3. fix format in test case.

[HaoyuZhang]

Thank you for the updating and comments from @spupyrev and @MaskRay .

clang/docs/UsersManual.rst

2244

thank you for the updating.

2246

thank you for the comments.

clang/include/clang/Driver/Options.td

1256

Modified.

clang/test/Driver/pgo-sample-use-profi.c

2

thank you for the correction of format.

[MaskRay]

In D136846#3890699, @wenlei wrote:

Did you see measurable perf boost with profi on autofdo? Or what motivated you to turn on profi? In most cases, profi helps when csspgo is used (instead of traditional autofdo).

LG in my view, but @wenlei may have something to add.

clang/test/Driver/pgo-sample-use-profi.c

3

Such a cross-directory test file reference is generally not acceptable. Use a dummy file -fprofile-sample-use=/dev/null

[wenlei]

In D136846#3910272, @MaskRay wrote:

In D136846#3890699, @wenlei wrote:

Did you see measurable perf boost with profi on autofdo? Or what motivated you to turn on profi? In most cases, profi helps when csspgo is used (instead of traditional autofdo).

LG in my view, but @wenlei may have something to add.

lgtm.

[HaoyuZhang]

Use a dummy file instead of cross-directory test file reference.

[HaoyuZhang]

Thanks @MaskRay and @wenlei for the reviewing.

New version for using dummy file has been updated.

clang/test/Driver/pgo-sample-use-profi.c

3

Thank you for the comments. Modified.

clang/docs/UsersManual.rst

4. Build the code again using the collected profile. This step feeds
    required to build the code with the exact same arguments that you
    used in the first step. The only requirement is that you build the code
    with ``-gline-tables-only`` and ``-fprofile-sample-use``.
 
    .. code-block:: console
 
      $ clang++ -O2 -gline-tables-only -fprofile-sample-use=code.prof code.cc -o code
 
+  [OPTIONAL] Sampling-based profiles can have inaccuracies or missing block/
+  edge counters. The profile inference algorithm (profi) can be used to infer
+  missing blocks and edge counts, and improve the quality of profile data.
+  Enable it with ``-fsample-profile-use-profi``.
+
+  .. code-block:: console
+
+    $ clang++ -O2 -gline-tables-only -fprofile-sample-use=code.prof \
+      -fsample-profile-use-profi code.cc -o code
 
 Sample Profile Formats
 """"""""""""""""""""""
 
 Since external profilers generate profile data in a variety of custom formats,
 the data generated by the profiler must be converted into a format that can be
 read by the backend. LLVM supports three different sample profile formats:
 

clang/include/clang/Driver/Options.td

 def fprofile_sample_accurate : Flag<["-"], "fprofile-sample-accurate">,
     Group<f_Group>, Flags<[NoXarchOption, CC1Option]>,
     HelpText<"Specifies that the sample profile is accurate">,
     DocBrief<[{Specifies that the sample profile is accurate. If the sample
                profile is accurate, callsites without profile samples are marked
                as cold. Otherwise, treat callsites without profile samples as if
                we have no profile}]>,
    MarshallingInfoFlag<CodeGenOpts<"ProfileSampleAccurate">>;
+def fsample_profile_use_profi : Flag<["-"], "fsample-profile-use-profi">,
+    Flags<[NoXarchOption, CC1Option]>, Group<f_Group>,
+    HelpText<"Use profi to infer block and edge counts">,
+    DocBrief<[{Infer block and edge counts. If the profiles have errors or missing
+               blocks caused by sampling, profile inference (profi) can convert
+               basic block counts to branch probabilites 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>, Flags<[NoXarchOption]>;
 def fauto_profile : Flag<["-"], "fauto-profile">, Group<f_Group>,
     Alias<fprofile_sample_use>;
 def fno_auto_profile : Flag<["-"], "fno-auto-profile">, Group<f_Group>,
     Alias<fno_profile_sample_use>;
 def fauto_profile_EQ : Joined<["-"], "fauto-profile=">,
     Alias<fprofile_sample_use_EQ>;

clang/lib/Driver/ToolChains/Clang.cpp

#endif
   // NVPTX/AMDGCN doesn't support PGO or coverage. There's no runtime support
   // for sampling, overhead of call arc collection is way too high and there's
   // no way to collect the output.
   if (!Triple.isNVPTX() && !Triple.isAMDGCN())
     addPGOAndCoverageFlags(TC, C, D, Output, Args, SanitizeArgs, CmdArgs);
 
   Args.AddLastArg(CmdArgs, options::OPT_fclang_abi_compat_EQ);
 
+  if (getLastProfileSampleUseArg(Args) &&
+      Args.hasArg(options::OPT_fsample_profile_use_profi)) {
+    CmdArgs.push_back("-mllvm");
+    CmdArgs.push_back("-sample-profile-use-profi");
+  }
+
   // Add runtime flag for PS4/PS5 when PGO, coverage, or sanitizers are enabled.
   if (RawTriple.isPS() &&
       !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
     PScpu::addProfileRTArgs(TC, Args, CmdArgs);
     PScpu::addSanitizerArgs(TC, Args, CmdArgs);
   }
 
   // Pass options for controlling the default header search paths.

clang/test/Driver/pgo-sample-use-profi.c

+/// Test if profi flat is enabled in frontend as user-facing feature.
+// RUN: %clang -c -fsample-profile-use-profi -fprofile-sample-use=/dev/null -### %s 2>&1 | FileCheck %s
+
+// CHECK: "-mllvm" "-sample-profile-use-profi"