Fix documentation on PGO/coverage related options. #73845

david-xl · 2023-11-29T19:58:36Z

Update the user manual to provide guidance on the usage for different flavors of instrumentations.

llvmbot · 2023-11-29T19:59:05Z

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

@llvm/pr-subscribers-llvm-transforms

Author: David Li (david-xl)

Changes

Update the user manual to provide guidance on the usage for different flavors of instrumentations.

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

2 Files Affected:

(modified) clang/docs/UsersManual.rst (+26-11)
(modified) llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp (+1-1)

diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 2e658557b0e310c..1d2165157b8be8a 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -2607,11 +2607,24 @@ overhead during the profiling, but it provides more detailed results than a
 sampling profiler. It also provides reproducible results, at least to the
 extent that the code behaves consistently across runs.
 
+There are two types of instrumentation available in Clang: frontend based and
+IR based. The frontend based instrumentation can be turned on with option
+``-fprofile-instr-generate`` and the IR based instrumentation can be turned
+on with ``-fprofile-generate`` option. For best performance with PGO, the IR
+based instrumentation should be used. It has the benefits of lower instrumentation
+overhead, smaller raw profile size, and better runtime performance. Frontend
+based instrumnetaition, on the other hand, has better source correlation so should
+be used with source line based coverage testing.
+
+Flag ``-fcs-profile-generate`` also instruments programs using the same
+instrumentation method as ``-fprofile-generate``. It does a post-inline late
+instrumentation and can produce context sensientive profile.
+
 Here are the steps for using profile guided optimization with
 instrumentation:
 
 1. Build an instrumented version of the code by compiling and linking with the
-   ``-fprofile-instr-generate`` option.
+   ``-fprofile-generate`` or ``-fprofile-instr-generate`` option.
 
    .. code-block:: console
 
@@ -2674,8 +2687,8 @@ instrumentation:
    Note that this step is necessary even when there is only one "raw" profile,
    since the merge operation also changes the file format.
 
-4. Build the code again using the ``-fprofile-instr-use`` option to specify the
-   collected profile data.
+4. Build the code again using the ``-fprofile-use`` or ``-fprofile-instr-use``
+   option to specify the collected profile data.
 
    .. code-block:: console
 
@@ -2685,13 +2698,10 @@ instrumentation:
    profile. As you make changes to your code, clang may no longer be able to
    use the profile data. It will warn you when this happens.
 
-Profile generation using an alternative instrumentation method can be
-controlled by the GCC-compatible flags ``-fprofile-generate`` and
-``-fprofile-use``. Although these flags are semantically equivalent to
-their GCC counterparts, they *do not* handle GCC-compatible profiles.
-They are only meant to implement GCC's semantics with respect to
-profile creation and use. Flag ``-fcs-profile-generate`` also instruments
-programs using the same instrumentation method as ``-fprofile-generate``.
+Note that ``-fprofile-use`` option is semantically equivalent to
+its GCC counterpart, it *does not* handle profile formats produced by GCC.
+Both ``-fprofile-use`` and ``-fprofile-instr-use`` accept profiles in the
+indexed format, regardeless whether it is produced by frontend or the IR pass.
 
 .. option:: -fprofile-generate[=<dirname>]
 
@@ -4401,6 +4411,9 @@ Execute ``clang-cl /?`` to see a list of supported options:
                               Instrument only functions from files where names don't match all the regexes separated by a semi-colon
       -fprofile-filter-files=<value>
                               Instrument only functions from files where names match any regex separated by a semi-colon
+      -fprofile-generate[=<dirname>]
+                              Generate instrumented code to collect execution counts into a raw profile file in <dirname>
+                              (overridden by LLVM_PROFILE_FILE env var)
       -fprofile-instr-generate=<file>
                               Generate instrumented code to collect execution counts into <file>
                               (overridden by LLVM_PROFILE_FILE env var)
@@ -4408,6 +4421,8 @@ Execute ``clang-cl /?`` to see a list of supported options:
                               Generate instrumented code to collect execution counts into default.profraw file
                               (overridden by '=' form of option or LLVM_PROFILE_FILE env var)
       -fprofile-instr-use=<value>
+                              Use instrumentation data for coverage testing or profile-guided optimization
+      -fprofile--use=<value>
                               Use instrumentation data for profile-guided optimization
       -fprofile-remapping-file=<file>
                               Use the remappings described in <file> to match the profile data against names in the program
@@ -4569,7 +4584,7 @@ clang-cl supports several features that require runtime library support:
 - Address Sanitizer (ASan): ``-fsanitize=address``
 - Undefined Behavior Sanitizer (UBSan): ``-fsanitize=undefined``
 - Code coverage: ``-fprofile-instr-generate -fcoverage-mapping``
-- Profile Guided Optimization (PGO): ``-fprofile-instr-generate``
+- Profile Guided Optimization (PGO): ``-fprofile-generate``
 - Certain math operations (int128 division) require the builtins library
 
 In order to use these features, the user must link the right runtime libraries
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index 601903c29f799a2..73a7116f74e1180 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This pass lowers instrprof_* intrinsics emitted by a frontend for profiling.
+// This pass lowers instrprof_* intrinsics emitted by an instrumentor.
 // It also builds the data structures and initialization code needed for
 // updating execution counts and emitting the profile at runtime.
 //

petrhosek · 2023-11-30T21:37:23Z

clang/docs/UsersManual.rst

@@ -4401,13 +4413,18 @@ Execute ``clang-cl /?`` to see a list of supported options:
                              Instrument only functions from files where names don't match all the regexes separated by a semi-colon
      -fprofile-filter-files=<value>
                              Instrument only functions from files where names match any regex separated by a semi-colon
+      -fprofile-generate[=<dirname>]


Rather than <dirname>, I think it'd be better to say <pattern> and point to https://clang.llvm.org/docs/UsersManual.html#profiling-with-instrumentation for more details.

Updated description. It is actually a dirname, not pattern.

This is really surprising but I just tested this and you're correct. Do you know why that's the case, that is why -fprofile-generate=<dirname> and -fprofile-instr-generate=<filename_pattern> have a different behavior?

-fprofile-generate= in GCC specifies a path which is a directory name (the reason is that GCC's profile data is dumped per module, so there is no single profile name).

When we introduced this option in Clang/LLVM, we just matched that behavior.

petrhosek · 2023-11-30T21:37:28Z

clang/docs/UsersManual.rst

@@ -4401,13 +4413,18 @@ Execute ``clang-cl /?`` to see a list of supported options:
                              Instrument only functions from files where names don't match all the regexes separated by a semi-colon
      -fprofile-filter-files=<value>
                              Instrument only functions from files where names match any regex separated by a semi-colon
+      -fprofile-generate[=<dirname>]
+                              Generate instrumented code to collect execution counts into a raw profile file in <dirname>
+                              (overridden by LLVM_PROFILE_FILE env var)
      -fprofile-instr-generate=<file>


Can we also update this line to match -fprofile-generate?

petrhosek · 2023-11-30T21:38:04Z

clang/docs/UsersManual.rst

      -fprofile-instr-generate=<file>
                              Generate instrumented code to collect execution counts into <file>
                              (overridden by LLVM_PROFILE_FILE env var)
      -fprofile-instr-generate
                              Generate instrumented code to collect execution counts into default.profraw file
                              (overridden by '=' form of option or LLVM_PROFILE_FILE env var)
      -fprofile-instr-use=<value>
+                              Use instrumentation data for coverage testing or profile-guided optimization
+      -fprofile--use=<value>


This a nit, but there should be only a single -, so -fprofile-use not -fprofile--use.

petrhosek · 2023-12-02T06:03:38Z

clang/docs/UsersManual.rst

-      -fprofile-instr-generate=<file>
-                              Generate instrumented code to collect execution counts into <file>
+      -fprofile-generate=<dirname>
+                              Generate instrumented code to collect execution counts into a raw profile file in the directory specified by the argument. The filename uses the %m format. See :ref:`Profiling With Instrumentation <prof_instr>`  section for details.


Should the second sentence say "The filename uses the default_%m.profraw."?

petrhosek · 2023-12-02T06:04:10Z

clang/docs/UsersManual.rst

+      -fprofile-generate
+                              Generate instrumented code to collect execution counts into default_%m.profraw file
+                              (overridden by '=' form of option or LLVM_PROFILE_FILE env var)
+      -fprofile-instr-generate=<file_name_pattern>


Can you also add -fprofile-instr-generate case below for consistency?

done (-fprofile-instr-generate was there before. I added -fprofile-generate).

llvmbot added clang Clang issues not falling into any other category PGO Profile Guided Optimizations llvm:transforms labels Nov 29, 2023

david-xl requested review from petrhosek and xur-llvm November 29, 2023 19:59

david-xl force-pushed the docfix branch 2 times, most recently from dfd0ae0 to 627d664 Compare November 30, 2023 19:15

petrhosek reviewed Nov 30, 2023

View reviewed changes

david-xl force-pushed the docfix branch 4 times, most recently from 15dd202 to 4cf62b1 Compare December 2, 2023 05:58

petrhosek reviewed Dec 2, 2023

View reviewed changes

petrhosek mentioned this pull request Dec 2, 2023

Rework Clang PGO documentation #74022

Open

david-xl force-pushed the docfix branch from 4cf62b1 to 29b5b28 Compare December 2, 2023 06:29

petrhosek approved these changes Dec 2, 2023

View reviewed changes

Fix PGO documentation in user manual

4c0f907

david-xl force-pushed the docfix branch from 29b5b28 to 4c0f907 Compare December 2, 2023 22:47

david-xl merged commit 7882f38 into llvm:main Dec 3, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix documentation on PGO/coverage related options. #73845

Fix documentation on PGO/coverage related options. #73845

david-xl commented Nov 29, 2023

llvmbot commented Nov 29, 2023 •

edited

petrhosek Nov 30, 2023

david-xl Nov 30, 2023

petrhosek Dec 2, 2023

david-xl Dec 2, 2023

petrhosek Nov 30, 2023

david-xl Nov 30, 2023

petrhosek Nov 30, 2023

david-xl Nov 30, 2023

petrhosek Dec 2, 2023

david-xl Dec 2, 2023

petrhosek Dec 2, 2023

david-xl Dec 2, 2023

Fix documentation on PGO/coverage related options. #73845

Fix documentation on PGO/coverage related options. #73845

Conversation

david-xl commented Nov 29, 2023

llvmbot commented Nov 29, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

llvmbot commented Nov 29, 2023 •

edited