[PATCH v2 1/6] Add AutoFDO support for Clang build

Rong Xu posted 6 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Rong Xu 1 month, 3 weeks ago
Add the build support for using Clang's AutoFDO. Building the kernel
with AutoFDO does not reduce the optimization level from the
compiler. AutoFDO uses hardware sampling to gather information about
the frequency of execution of different code paths within a binary.
This information is then used to guide the compiler's optimization
decisions, resulting in a more efficient binary. Experiments
showed that the kernel can improve up to 10% in latency.

The support requires a Clang compiler after LLVM 17. This submission
is limited to x86 platforms that support PMU features like LBR on
Intel machines and AMD Zen3 BRS. Support for SPE on ARM 1,
 and BRBE on ARM 1 is part of planned future work.

Here is an example workflow for AutoFDO kernel:

1) Build the kernel on the HOST machine with LLVM enabled, for example,
       $ make menuconfig LLVM=1
    Turn on AutoFDO build config:
      CONFIG_AUTOFDO_CLANG=y
    With a configuration that has LLVM enabled, use the following
    command:
       scripts/config -e AUTOFDO_CLANG
    After getting the config, build with
      $ make LLVM=1

2) Install the kernel on the TEST machine.

3) Run the load tests. The '-c' option in perf specifies the sample
   event period. We suggest     using a suitable prime number,
   like 500009, for this purpose.
   For Intel platforms:
      $ perf record -e BR_INST_RETIRED.NEAR_TAKEN:k -a -N -b -c <count> \
        -o <perf_file> -- <loadtest>
   For AMD platforms:
      The supported system are: Zen3 with BRS, or Zen4 with amd_lbr_v2
     For Zen3:
      $ cat proc/cpuinfo | grep " brs"
      For Zen4:
      $ cat proc/cpuinfo | grep amd_lbr_v2
      $ perf record --pfm-events RETIRED_TAKEN_BRANCH_INSTRUCTIONS:k -a \
        -N -b -c <count> -o <perf_file> -- <loadtest>

4) (Optional) Download the raw perf file to the HOST machine.

5) To generate an AutoFDO profile, two offline tools are available:
   create_llvm_prof and llvm_profgen. The create_llvm_prof tool is part
   of the AutoFDO project and can be found on GitHub
   (https://github.com/google/autofdo), version v0.30.1 or later. The
   llvm_profgen tool is included in the LLVM compiler itself. It's
   important to note that the version of llvm_profgen doesn't need to
   match the version of Clang. It needs to be the LLVM 19 release or
   later, or from the LLVM trunk.
      $ llvm-profgen --kernel --binary=<vmlinux> --perfdata=<perf_file> \
        -o <profile_file>
   or
      $ create_llvm_prof --binary=<vmlinux> --profile=<perf_file> \
        --format=extbinary -o <profile_file>

   Note that multiple AutoFDO profile files can be merged into one via:
      $ llvm-profdata merge -o <profile_file>  <profile_1> ... <profile_n>

6) Rebuild the kernel using the AutoFDO profile file with the same config
   as step 1, (Note CONFIG_AUTOFDO_CLANG needs to be enabled):
      $ make LLVM=1 CLANG_AUTOFDO_PROFILE=<profile_file>

Co-developed-by: Han Shen <shenhan@google.com>
Signed-off-by: Han Shen <shenhan@google.com>
Signed-off-by: Rong Xu <xur@google.com>
Suggested-by: Sriraman Tallam <tmsriram@google.com>
Suggested-by: Krzysztof Pszeniczny <kpszeniczny@google.com>
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Suggested-by: Stephane Eranian <eranian@google.com>
---
 Documentation/dev-tools/autofdo.rst | 203 ++++++++++++++++++++++++++++
 Documentation/dev-tools/index.rst   |   1 +
 MAINTAINERS                         |   7 +
 Makefile                            |   1 +
 arch/Kconfig                        |  20 +++
 arch/x86/Kconfig                    |   1 +
 scripts/Makefile.autofdo            |  23 ++++
 scripts/Makefile.lib                |  10 ++
 tools/objtool/check.c               |   1 +
 9 files changed, 267 insertions(+)
 create mode 100644 Documentation/dev-tools/autofdo.rst
 create mode 100644 scripts/Makefile.autofdo

diff --git a/Documentation/dev-tools/autofdo.rst b/Documentation/dev-tools/autofdo.rst
new file mode 100644
index 000000000000..e3f7ee8ee6bb
--- /dev/null
+++ b/Documentation/dev-tools/autofdo.rst
@@ -0,0 +1,203 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================================
+Using AutoFDO with the Linux kernel
+===================================
+
+This enables AutoFDO build support for the kernel when using
+the Clang compiler. AutoFDO (Auto-Feedback-Directed Optimization)
+is a type of profile-guided optimization (PGO) used to enhance the
+performance of binary executables. It gathers information about the
+frequency of execution of various code paths within a binary using
+hardware sampling. This data is then used to guide the compiler's
+optimization decisions, resulting in a more efficient binary. AutoFDO
+is a powerful optimization technique, and data indicates that it can
+significantly improve kernel performance. It's especially beneficial
+for workloads affected by front-end stalls.
+
+For AutoFDO builds, unlike non-FDO builds, the user must supply a
+profile. Acquiring an AutoFDO profile can be done in several ways.
+AutoFDO profiles are created by converting hardware sampling using
+the "perf" tool. It is crucial that the workload used to create these
+perf files is representative; they must exhibit runtime
+characteristics similar to the workloads that are intended to be
+optimized. Failure to do so will result in the compiler optimizing
+for the wrong objective.
+
+The AutoFDO profile often encapsulates the program's behavior. If the
+performance-critical codes are architecture-independent, the profile
+can be applied across platforms to achieve performance gains. For
+instance, using the profile generated on Intel architecture to build
+a kernel for AMD architecture can also yield performance improvements.
+
+There are two methods for acquiring a representative profile:
+(1) Sample real workloads using a production environment.
+(2) Generate the profile using a representative load test.
+When enabling the AutoFDO build configuration without providing an
+AutoFDO profile, the compiler only modifies the dwarf information in
+the kernel without impacting runtime performance. It's advisable to
+use a kernel binary built with the same AutoFDO configuration to
+collect the perf profile. While it's possible to use a kernel built
+with different options, it may result in inferior performance.
+
+One can collect profiles using AutoFDO build for the previous kernel.
+AutoFDO employs relative line numbers to match the profiles, offering
+some tolerance for source changes. This mode is commonly used in a
+production environment for profile collection.
+
+In a profile collection based on a load test, the AutoFDO collection
+process consists of the following steps:
+
+#. Initial build: The kernel is built with AutoFDO options
+   without a profile.
+
+#. Profiling: The above kernel is then run with a representative
+   workload to gather execution frequency data. This data is
+   collected using hardware sampling, via perf. AutoFDO is most
+   effective on platforms supporting advanced PMU features like
+   LBR on Intel machines.
+
+#. AutoFDO profile generation: Perf output file is converted to
+   the AutoFDO profile via offline tools.
+
+The support requires a Clang compiler LLVM 17 or later.
+
+Preparation
+===========
+
+Configure the kernel with:
+
+   .. code-block:: make
+
+      CONFIG_AUTOFDO_CLANG=y
+
+Customization
+=============
+
+You can enable or disable AutoFDO build for individual file and directories by
+adding a line similar to the following to the respective kernel Makefile:
+
+- For enabling a single file (e.g. foo.o)
+
+     .. code-block:: make
+
+        AUTOFDO_PROFILE_foo.o := y
+
+- For enabling all files in one directory
+
+     .. code-block:: make
+
+        AUTOFDO_PROFILE := y
+
+- For disabling one file
+
+     .. code-block:: make
+
+        AUTOFDO_PROFILE_foo.o := n
+
+- For disabling all files in one directory
+
+     .. code-block:: make
+
+        AUTOFDO_PROFILE := n
+
+
+Workflow
+========
+
+Here is an example workflow for AutoFDO kernel:
+
+
+
+1)  Build the kernel on the HOST machine with LLVM enabled, for example,
+
+      .. code-block:: make
+
+         $ make menuconfig LLVM=1
+
+
+    Turn on AutoFDO build config:
+
+      .. code-block:: make
+
+         CONFIG_AUTOFDO_CLANG=y
+
+    With a configuration that with LLVM enabled, use the following command:
+
+      .. code-block:: sh
+
+         $ scripts/config -e AUTOFDO_CLANG
+
+    After getting the config, build with
+
+      .. code-block:: make
+
+         $ make LLVM=1
+
+2) Install the kernel on the TEST machine.
+
+3) Run the load tests. The '-c' option in perf specifies the sample
+   event period. We suggest using a suitable prime number, like 500009,
+   for this purpose.
+
+   - For Intel platforms:
+
+      .. code-block:: sh
+
+         $ perf record -e BR_INST_RETIRED.NEAR_TAKEN:k -a -N -b -c <count> -o <perf_file> -- <loadtest>
+
+   - For AMD platforms: For Intel platforms:
+     The supported systems are: Zen3 with BRS, or Zen4 with amd_lbr_v2. To check,
+     For Zen3:
+
+      .. code-block:: sh
+
+         $ cat proc/cpuinfo | grep " brs"
+
+      For Zen4:
+
+      .. code-block:: sh
+
+         $ cat proc/cpuinfo | grep amd_lbr_v2
+
+      The following command generated the perf data file:
+
+      .. code-block:: sh
+
+         $ perf record --pfm-events RETIRED_TAKEN_BRANCH_INSTRUCTIONS:k -a -N -b \
+           -c <count> -o <perf_file> -- <loadtest>
+
+4) (Optional) Download the raw perf file to the HOST machine.
+
+5) To generate an AutoFDO profile, two offline tools are available:
+   create_llvm_prof and llvm_profgen. The create_llvm_prof tool is part
+   of the AutoFDO project and can be found on GitHub
+   (https://github.com/google/autofdo),  version v0.30.1 or later.
+   The llvm_profgen tool is included in the LLVM compiler itself. It's
+   important to note that the version of llvm_profgen doesn't need to match
+   the version of Clang. It needs to be the LLVM 19 release of Clang
+   or later, or just from the LLVM trunk.
+
+      .. code-block:: sh
+
+         $ llvm-profgen --kernel --binary=<vmlinux> --perfdata=<perf_file> -o <profile_file>
+
+   or
+      .. code-block:: sh
+
+         $ create_llvm_prof --binary=<vmlinux> --profile=<perf_file> --format=extbinary -o <profile_file>
+
+   Note that multiple AutoFDO profile files can be merged into one via:
+
+      .. code-block:: sh
+
+         $ llvm-profdata merge -o <profile_file>  <profile_1> <profile_2> ... <profile_n>
+
+
+6) Rebuild the kernel using the AutoFDO profile file with the same config as step 1,
+    (Note CONFIG_AUTOFDO_CLANG needs to be enabled):
+
+      .. code-block:: sh
+
+         $ make LLVM=1 CLANG_AUTOFDO_PROFILE=<profile_file
+
diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 53d4d124f9c5..6945644f7008 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -34,6 +34,7 @@ Documentation/dev-tools/testing-overview.rst
    ktap
    checkuapi
    gpio-sloppy-logic-analyzer
+   autofdo
 
 
 .. only::  subproject and html
diff --git a/MAINTAINERS b/MAINTAINERS
index c27f3190737f..62b798c20128 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3678,6 +3678,13 @@ F:	kernel/audit*
 F:	lib/*audit.c
 K:	\baudit_[a-z_0-9]\+\b
 
+AUTOFDO BUILD
+M:	Rong Xu <xur@google.com>
+M:	Han Shen <shenhan@google.com>
+S:	Supported
+F:	Documentation/dev-tools/autofdo.rst
+F:	scripts/Makefile.autofdo
+
 AUXILIARY BUS DRIVER
 M:	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 R:	Dave Ertman <david.m.ertman@intel.com>
diff --git a/Makefile b/Makefile
index 187a4ce2728e..55d19c81b382 100644
--- a/Makefile
+++ b/Makefile
@@ -1018,6 +1018,7 @@ include-$(CONFIG_KMSAN)		+= scripts/Makefile.kmsan
 include-$(CONFIG_UBSAN)		+= scripts/Makefile.ubsan
 include-$(CONFIG_KCOV)		+= scripts/Makefile.kcov
 include-$(CONFIG_RANDSTRUCT)	+= scripts/Makefile.randstruct
+include-$(CONFIG_AUTOFDO_CLANG)	+= scripts/Makefile.autofdo
 include-$(CONFIG_GCC_PLUGINS)	+= scripts/Makefile.gcc-plugins
 
 include $(addprefix $(srctree)/, $(include-y))
diff --git a/arch/Kconfig b/arch/Kconfig
index 98157b38f5cf..106d09fc42ce 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -811,6 +811,26 @@ config LTO_CLANG_THIN
 	  If unsure, say Y.
 endchoice
 
+config ARCH_SUPPORTS_AUTOFDO_CLANG
+	bool
+
+config AUTOFDO_CLANG
+	bool "Enable Clang's AutoFDO build (EXPERIMENTAL)"
+	depends on ARCH_SUPPORTS_AUTOFDO_CLANG
+	depends on CC_IS_CLANG && CLANG_VERSION >= 170000
+	help
+	  This option enables Clang’s AutoFDO build. When
+	  an AutoFDO profile is specified in variable
+	  CLANG_AUTOFDO_PROFILE during the build process,
+	  Clang uses the profile to optimize the kernel.
+
+	  If no profile is specified, AutoFDO options are
+	  still passed to Clang to facilitate the collection
+	  of perf data for creating an AutoFDO profile in
+	  subsequent builds.
+
+	  If unsure, say N.
+
 config ARCH_SUPPORTS_CFI_CLANG
 	bool
 	help
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2852fcd82cbd..503a0268155a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -126,6 +126,7 @@ config X86
 	select ARCH_SUPPORTS_LTO_CLANG
 	select ARCH_SUPPORTS_LTO_CLANG_THIN
 	select ARCH_SUPPORTS_RT
+	select ARCH_SUPPORTS_AUTOFDO_CLANG
 	select ARCH_USE_BUILTIN_BSWAP
 	select ARCH_USE_CMPXCHG_LOCKREF		if X86_CMPXCHG64
 	select ARCH_USE_MEMTEST
diff --git a/scripts/Makefile.autofdo b/scripts/Makefile.autofdo
new file mode 100644
index 000000000000..1c9f224bc221
--- /dev/null
+++ b/scripts/Makefile.autofdo
@@ -0,0 +1,23 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# Enable available and selected Clang AutoFDO features.
+
+CFLAGS_AUTOFDO_CLANG := -fdebug-info-for-profiling -mllvm -enable-fs-discriminator=true -mllvm -improved-fs-discriminator=true
+
+# If CONFIG_DEBUG_INFO is not enabled, set -gmlt option.
+ifndef CONFIG_DEBUG_INFO
+  CFLAGS_AUTOFDO_CLANG += -gmlt
+endif
+
+ifdef CLANG_AUTOFDO_PROFILE
+  CFLAGS_AUTOFDO_CLANG += -fprofile-sample-use=$(CLANG_AUTOFDO_PROFILE)
+endif
+
+ifdef CONFIG_LTO_CLANG_THIN
+  ifdef CLANG_AUTOFDO_PROFILE
+    KBUILD_LDFLAGS += --lto-sample-profile=$(CLANG_AUTOFDO_PROFILE)
+  endif
+  KBUILD_LDFLAGS += --mllvm=-enable-fs-discriminator=true --mllvm=-improved-fs-discriminator=true -plugin-opt=thinlto
+endif
+
+export CFLAGS_AUTOFDO_CLANG
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 01a9f567d5af..e85d6ac31bd9 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -191,6 +191,16 @@ _c_flags += $(if $(patsubst n%,, \
 	-D__KCSAN_INSTRUMENT_BARRIERS__)
 endif
 
+#
+# Enable Clang's AutoFDO build flags for a file or directory depending on
+# variables AUTOFDO_PROFILE_obj.o and AUTOFDO_PROFILE.
+#
+ifeq ($(CONFIG_AUTOFDO_CLANG),y)
+_c_flags += $(if $(patsubst n%,, \
+	$(AUTOFDO_PROFILE_$(target-stem).o)$(AUTOFDO_PROFILE)$(is-kernel-object)), \
+	$(CFLAGS_AUTOFDO_CLANG))
+endif
+
 # $(src) for including checkin headers from generated source files
 # $(obj) for including generated headers from checkin source files
 ifeq ($(KBUILD_EXTMOD),)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 6604f5d038aa..4c5229991e1e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4557,6 +4557,7 @@ static int validate_ibt(struct objtool_file *file)
 		    !strcmp(sec->name, "__jump_table")			||
 		    !strcmp(sec->name, "__mcount_loc")			||
 		    !strcmp(sec->name, ".kcfi_traps")			||
+		    !strcmp(sec->name, ".llvm.call-graph-profile")	||
 		    strstr(sec->name, "__patchable_function_entries"))
 			continue;
 
-- 
2.46.1.824.gd892dcdcdd-goog
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Peter Zijlstra 1 month, 3 weeks ago
On Wed, Oct 02, 2024 at 04:34:00PM -0700, Rong Xu wrote:
> +Preparation
> +===========
> +
> +Configure the kernel with:
> +
> +   .. code-block:: make
> +
> +      CONFIG_AUTOFDO_CLANG=y
> +
> +Customization
> +=============
> +
> +You can enable or disable AutoFDO build for individual file and directories by
> +adding a line similar to the following to the respective kernel Makefile:
> +
> +- For enabling a single file (e.g. foo.o)
> +
> +     .. code-block:: make
> +
> +        AUTOFDO_PROFILE_foo.o := y
> +
> +- For enabling all files in one directory
> +
> +     .. code-block:: make
> +
> +        AUTOFDO_PROFILE := y
> +
> +- For disabling one file
> +
> +     .. code-block:: make
> +
> +        AUTOFDO_PROFILE_foo.o := n
> +
> +- For disabling all files in one directory
> +
> +     .. code-block:: make
> +
> +        AUTOFDO_PROFILE := n
> +
> +
> +Workflow
> +========
> +
> +Here is an example workflow for AutoFDO kernel:
> +
> +
> +
> +1)  Build the kernel on the HOST machine with LLVM enabled, for example,
> +
> +      .. code-block:: make
> +
> +         $ make menuconfig LLVM=1
> +
> +
> +    Turn on AutoFDO build config:
> +
> +      .. code-block:: make
> +
> +         CONFIG_AUTOFDO_CLANG=y
> +
> +    With a configuration that with LLVM enabled, use the following command:
> +
> +      .. code-block:: sh
> +
> +         $ scripts/config -e AUTOFDO_CLANG
> +
> +    After getting the config, build with
> +
> +      .. code-block:: make
> +
> +         $ make LLVM=1
> +
> +2) Install the kernel on the TEST machine.
> +
> +3) Run the load tests. The '-c' option in perf specifies the sample
> +   event period. We suggest using a suitable prime number, like 500009,
> +   for this purpose.
> +
> +   - For Intel platforms:
> +
> +      .. code-block:: sh
> +
> +         $ perf record -e BR_INST_RETIRED.NEAR_TAKEN:k -a -N -b -c <count> -o <perf_file> -- <loadtest>
> +
> +   - For AMD platforms: For Intel platforms:
> +     The supported systems are: Zen3 with BRS, or Zen4 with amd_lbr_v2. To check,
> +     For Zen3:
> +
> +      .. code-block:: sh
> +
> +         $ cat proc/cpuinfo | grep " brs"
> +
> +      For Zen4:
> +
> +      .. code-block:: sh
> +
> +         $ cat proc/cpuinfo | grep amd_lbr_v2
> +
> +      The following command generated the perf data file:
> +
> +      .. code-block:: sh
> +
> +         $ perf record --pfm-events RETIRED_TAKEN_BRANCH_INSTRUCTIONS:k -a -N -b \
> +           -c <count> -o <perf_file> -- <loadtest>
> +
> +4) (Optional) Download the raw perf file to the HOST machine.
> +
> +5) To generate an AutoFDO profile, two offline tools are available:
> +   create_llvm_prof and llvm_profgen. The create_llvm_prof tool is part
> +   of the AutoFDO project and can be found on GitHub
> +   (https://github.com/google/autofdo),  version v0.30.1 or later.
> +   The llvm_profgen tool is included in the LLVM compiler itself. It's
> +   important to note that the version of llvm_profgen doesn't need to match
> +   the version of Clang. It needs to be the LLVM 19 release of Clang
> +   or later, or just from the LLVM trunk.
> +
> +      .. code-block:: sh
> +
> +         $ llvm-profgen --kernel --binary=<vmlinux> --perfdata=<perf_file> -o <profile_file>
> +
> +   or
> +      .. code-block:: sh
> +
> +         $ create_llvm_prof --binary=<vmlinux> --profile=<perf_file> --format=extbinary -o <profile_file>
> +
> +   Note that multiple AutoFDO profile files can be merged into one via:
> +
> +      .. code-block:: sh
> +
> +         $ llvm-profdata merge -o <profile_file>  <profile_1> <profile_2> ... <profile_n>
> +
> +
> +6) Rebuild the kernel using the AutoFDO profile file with the same config as step 1,
> +    (Note CONFIG_AUTOFDO_CLANG needs to be enabled):
> +
> +      .. code-block:: sh
> +
> +         $ make LLVM=1 CLANG_AUTOFDO_PROFILE=<profile_file
> +


Can this be done without the endless ... code-block nonsense?
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Kees Cook 1 month, 3 weeks ago
On Thu, Oct 03, 2024 at 05:41:43PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 02, 2024 at 04:34:00PM -0700, Rong Xu wrote:
> > +Preparation
> > +===========
> > +
> > +Configure the kernel with:
> > +
> > +   .. code-block:: make
> > +
> > +      CONFIG_AUTOFDO_CLANG=y
> > +
> > +
> [...]
> > +    With a configuration that with LLVM enabled, use the following command:
> > +
> > +      .. code-block:: sh
> > +
> > +         $ scripts/config -e AUTOFDO_CLANG
> [...]
> 
> Can this be done without the endless ... code-block nonsense?

The tradition in kernel .rst is to do this with the trailing "::", e.g.:

+Configure the kernel with::
+
+     CONFIG_AUTOFDO_CLANG=y

This loses the language-specific highlighting when rendered. Perhaps the
"::" extension can be further extended?

+Configure the kernel with::(make)
+
+     CONFIG_AUTOFDO_CLANG=y

Then we could avoid the extra 2 lines but still gain the rendered language
highlights?

-- 
Kees Cook
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Jonathan Corbet 1 month, 3 weeks ago
Kees Cook <kees@kernel.org> writes:

> The tradition in kernel .rst is to do this with the trailing "::", e.g.:
>
> +Configure the kernel with::
> +
> +     CONFIG_AUTOFDO_CLANG=y
>
> This loses the language-specific highlighting when rendered. Perhaps the
> "::" extension can be further extended?
>
> +Configure the kernel with::(make)
> +
> +     CONFIG_AUTOFDO_CLANG=y
>
> Then we could avoid the extra 2 lines but still gain the rendered language
> highlights?

The :: notation is standard Sphinx, not an extension we have done.  So
the proposed syntax would have to be done from the beginning, I'm not
sure how easy or hard that would be, or whether it would be worth it.
But then, I've always seen relatively little value in the highlighting;
others clearly differ.

Thanks,

jon
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Rong Xu 1 month, 3 weeks ago
I removed the "code-block" directives from the rst files,
and used "::" suggested by Jonathan. The rst files themselves are now
easier to read in vi.  However, the rendered HTML output has some differences:
(1) The text that was previously in code-block no longer indents. It aligns
      with the preceding text, regardless of how many spaces I add.
(2) Previously, "code-block" removed '\' and combined the text into a
single line.
     This is no longer happening -- the '\' is not expanded.

These differences do not seem to be a blocker. I'm attaching the html files to
this email. If there is no objection, I'll change to using the new
method of "::".

-Rong

On Sat, Oct 5, 2024 at 7:43 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> Kees Cook <kees@kernel.org> writes:
>
> > The tradition in kernel .rst is to do this with the trailing "::", e.g.:
> >
> > +Configure the kernel with::
> > +
> > +     CONFIG_AUTOFDO_CLANG=y
> >
> > This loses the language-specific highlighting when rendered. Perhaps the
> > "::" extension can be further extended?
> >
> > +Configure the kernel with::(make)
> > +
> > +     CONFIG_AUTOFDO_CLANG=y
> >
> > Then we could avoid the extra 2 lines but still gain the rendered language
> > highlights?
>
> The :: notation is standard Sphinx, not an extension we have done.  So
> the proposed syntax would have to be done from the beginning, I'm not
> sure how easy or hard that would be, or whether it would be worth it.
> But then, I've always seen relatively little value in the highlighting;
> others clearly differ.
>
> Thanks,
>
> jon
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Miguel Ojeda 1 month, 3 weeks ago
On Mon, Oct 7, 2024 at 8:04 PM Rong Xu <xur@google.com> wrote:
>
> I removed the "code-block" directives from the rst files,
> and used "::" suggested by Jonathan. The rst files themselves are now

I think it was Mike.

> (1) The text that was previously in code-block no longer indents. It aligns
>       with the preceding text, regardless of how many spaces I add.

Did you try with a tab? At least in Doc/rust/ all those three ways
(i.e. `::`, `.. code-block::` and a single `:` for indented non-code)
seem to work fine, e.g. the following document uses all of them:

    https://docs.kernel.org/rust/coding-guidelines.html

I hope that helps!

Cheers,
Miguel
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Rong Xu 1 month, 3 weeks ago
On Mon, Oct 7, 2024 at 11:33 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Mon, Oct 7, 2024 at 8:04 PM Rong Xu <xur@google.com> wrote:
> >
> > I removed the "code-block" directives from the rst files,
> > and used "::" suggested by Jonathan. The rst files themselves are now
>
> I think it was Mike.

Sorry for mixing up the names.

>
> > (1) The text that was previously in code-block no longer indents. It aligns
> >       with the preceding text, regardless of how many spaces I add.
>
> Did you try with a tab? At least in Doc/rust/ all those three ways
> (i.e. `::`, `.. code-block::` and a single `:` for indented non-code)
> seem to work fine, e.g. the following document uses all of them:
>
>     https://docs.kernel.org/rust/coding-guidelines.html

I looked at run/coding-guidelines.html generated by "make htmldocs".
It has the same issue
of indentation for "::" text. I also tried tab, nothing changed for me.

-Rong

>
> I hope that helps!
>
> Cheers,
> Miguel
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Peter Zijlstra 1 month, 3 weeks ago
On Fri, Oct 04, 2024 at 11:10:04AM -0700, Kees Cook wrote:

> +Configure the kernel with::(make)
> +
> +     CONFIG_AUTOFDO_CLANG=y
> 
> Then we could avoid the extra 2 lines but still gain the rendered language
> highlights?

The whole double-colon thing is already a pain to read; you're making it
worse again.
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Justin Stitt 1 month, 3 weeks ago
On Fri, Oct 4, 2024 at 11:29 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Oct 04, 2024 at 11:10:04AM -0700, Kees Cook wrote:
>
> > +Configure the kernel with::(make)
> > +
> > +     CONFIG_AUTOFDO_CLANG=y
> >
> > Then we could avoid the extra 2 lines but still gain the rendered language
> > highlights?
>
> The whole double-colon thing is already a pain to read; you're making it
> worse again.

Lots of people read docs on the web and having code blocks with
monospaced fonts (+syntax highlighting) makes them easier to read
there.

Configure the kernel with:

     CONFIG_AUTOFDO_CLANG=y

--versus--

Configure the kernel with::

     CONFIG_AUTOFDO_CLANG=y

This renders better for html through Sphinx and really can't be that
bad to read in vim, can it?

two cents and such,
Justin
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Peter Zijlstra 1 month, 3 weeks ago
On Fri, Oct 04, 2024 at 02:23:45PM -0700, Justin Stitt wrote:
> On Fri, Oct 4, 2024 at 11:29 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Fri, Oct 04, 2024 at 11:10:04AM -0700, Kees Cook wrote:
> >
> > > +Configure the kernel with::(make)
> > > +
> > > +     CONFIG_AUTOFDO_CLANG=y
> > >
> > > Then we could avoid the extra 2 lines but still gain the rendered language
> > > highlights?
> >
> > The whole double-colon thing is already a pain to read; you're making it
> > worse again.
> 
> Lots of people read docs on the web and having code blocks with
> monospaced fonts (+syntax highlighting) makes them easier to read
> there.
> 
> Configure the kernel with:
> 
>      CONFIG_AUTOFDO_CLANG=y
> 
> --versus--
> 
> Configure the kernel with::
> 
>      CONFIG_AUTOFDO_CLANG=y
> 
> This renders better for html through Sphinx and really can't be that
> bad to read in vim, can it?

It's weird; but I've gotten sorta used to it. I was more or less
commenting on Kees' proposal of making it:

  Configure the kernel with::(make)

Which is definitely harder to read since it insta triggers a syntax
error exception in my head.
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Nick Desaulniers 1 month, 3 weeks ago
On Thu, Oct 3, 2024 at 8:42 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Oct 02, 2024 at 04:34:00PM -0700, Rong Xu wrote:
> > +6) Rebuild the kernel using the AutoFDO profile file with the same config as step 1,
> > +    (Note CONFIG_AUTOFDO_CLANG needs to be enabled):
> > +
> > +      .. code-block:: sh
> > +
> > +         $ make LLVM=1 CLANG_AUTOFDO_PROFILE=<profile_file
> > +
>
>
> Can this be done without the endless ... code-block nonsense?

Dunno, I think it looks pretty nice once rendered. Makes it
straightforward for a user to copy+paste. I asked Rong explicitly to
make sure the documentation made it so that non-googler or folks not
working on autofdo or propellor could reproduce (since we'll probably
end up standing up CI for these newer configs, and BOLT).
-- 
Thanks,
~Nick Desaulniers
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Peter Zijlstra 1 month, 3 weeks ago
On Thu, Oct 03, 2024 at 08:51:51AM -0700, Nick Desaulniers wrote:
> On Thu, Oct 3, 2024 at 8:42 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Oct 02, 2024 at 04:34:00PM -0700, Rong Xu wrote:
> > > +6) Rebuild the kernel using the AutoFDO profile file with the same config as step 1,
> > > +    (Note CONFIG_AUTOFDO_CLANG needs to be enabled):
> > > +
> > > +      .. code-block:: sh
> > > +
> > > +         $ make LLVM=1 CLANG_AUTOFDO_PROFILE=<profile_file
> > > +
> >
> >
> > Can this be done without the endless ... code-block nonsense?
> 
> Dunno, I think it looks pretty nice once rendered. Makes it

It makes it absolute crap for all of us who 'render' text documents
using less or vi.
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Nick Desaulniers 1 month, 3 weeks ago
On Thu, Oct 3, 2024 at 9:03 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Oct 03, 2024 at 08:51:51AM -0700, Nick Desaulniers wrote:
> > On Thu, Oct 3, 2024 at 8:42 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Wed, Oct 02, 2024 at 04:34:00PM -0700, Rong Xu wrote:
> > > > +6) Rebuild the kernel using the AutoFDO profile file with the same config as step 1,
> > > > +    (Note CONFIG_AUTOFDO_CLANG needs to be enabled):
> > > > +
> > > > +      .. code-block:: sh
> > > > +
> > > > +         $ make LLVM=1 CLANG_AUTOFDO_PROFILE=<profile_file
> > > > +
> > >
> > >
> > > Can this be done without the endless ... code-block nonsense?
> >
> > Dunno, I think it looks pretty nice once rendered. Makes it
>
> It makes it absolute crap for all of us who 'render' text documents
> using less or vi.

"It hurts when I punch myself in the face."

https://docs.kernel.org/ is where we point people to, and what we
should be optimizing for (IMO).  Jonathan, do we have a policy about
using code-block consistently throughout the kernel docs or not?
Otherwise, this complaint smells like "non-technical non-sense" to me.

-- 
Thanks,
~Nick Desaulniers
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Peter Zijlstra 1 month, 3 weeks ago
On Thu, Oct 03, 2024 at 09:11:34AM -0700, Nick Desaulniers wrote:

> > It makes it absolute crap for all of us who 'render' text documents
> > using less or vi.
> 
> "It hurts when I punch myself in the face."

Weirdly enough I have a job that entails staring at text documents in
text editors all day every day :-) sorry for thinking that's a sane
thing to do.
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Rong Xu 1 month, 3 weeks ago
Writing the doc with all these code-blocks was not fun either.
We are happy to change if there is a better way for this.

-Rong

On Thu, Oct 3, 2024 at 9:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Oct 03, 2024 at 09:11:34AM -0700, Nick Desaulniers wrote:
>
> > > It makes it absolute crap for all of us who 'render' text documents
> > > using less or vi.
> >
> > "It hurts when I punch myself in the face."
>
> Weirdly enough I have a job that entails staring at text documents in
> text editors all day every day :-) sorry for thinking that's a sane
> thing to do.
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Mike Rapoport 1 month, 3 weeks ago
On Thu, Oct 03, 2024 at 11:20:17AM -0700, Rong Xu wrote:
> Writing the doc with all these code-blocks was not fun either.
> We are happy to change if there is a better way for this.
> 
> -Rong
> 
> On Thu, Oct 3, 2024 at 9:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Thu, Oct 03, 2024 at 09:11:34AM -0700, Nick Desaulniers wrote:
> >
> > > > It makes it absolute crap for all of us who 'render' text documents
> > > > using less or vi.
> > >
> > > "It hurts when I punch myself in the face."
> >
> > Weirdly enough I have a job that entails staring at text documents in
> > text editors all day every day :-) sorry for thinking that's a sane
> > thing to do.

Something like this should do:

> +- For enabling a single file (e.g. foo.o)::
> +
> +        AUTOFDO_PROFILE_foo.o := y


-- 
Sincerely yours,
Mike.
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Rong Xu 1 month, 3 weeks ago
On Thu, Oct 3, 2024 at 11:09 PM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Thu, Oct 03, 2024 at 11:20:17AM -0700, Rong Xu wrote:
> > Writing the doc with all these code-blocks was not fun either.
> > We are happy to change if there is a better way for this.
> >
> > -Rong
> >
> > On Thu, Oct 3, 2024 at 9:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Thu, Oct 03, 2024 at 09:11:34AM -0700, Nick Desaulniers wrote:
> > >
> > > > > It makes it absolute crap for all of us who 'render' text documents
> > > > > using less or vi.
> > > >
> > > > "It hurts when I punch myself in the face."
> > >
> > > Weirdly enough I have a job that entails staring at text documents in
> > > text editors all day every day :-) sorry for thinking that's a sane
> > > thing to do.
>
> Something like this should do:
>
> > +- For enabling a single file (e.g. foo.o)::
> > +
> > +        AUTOFDO_PROFILE_foo.o := y

Do you mean we don't use ".. code-block:: " here and just use indented
text separated with blank lines?

-Rong

>
>
> --
> Sincerely yours,
> Mike.
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Mike Rapoport 1 month, 3 weeks ago
On Fri, Oct 04, 2024 at 09:28:36AM -0700, Rong Xu wrote:
> On Thu, Oct 3, 2024 at 11:09 PM Mike Rapoport <rppt@kernel.org> wrote:
> >
> > On Thu, Oct 03, 2024 at 11:20:17AM -0700, Rong Xu wrote:
> > > Writing the doc with all these code-blocks was not fun either.
> > > We are happy to change if there is a better way for this.
> > >
> > > -Rong
> > >
> > > On Thu, Oct 3, 2024 at 9:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > On Thu, Oct 03, 2024 at 09:11:34AM -0700, Nick Desaulniers wrote:
> > > >
> > > > > > It makes it absolute crap for all of us who 'render' text documents
> > > > > > using less or vi.
> > > > >
> > > > > "It hurts when I punch myself in the face."
> > > >
> > > > Weirdly enough I have a job that entails staring at text documents in
> > > > text editors all day every day :-) sorry for thinking that's a sane
> > > > thing to do.
> >
> > Something like this should do:
> >
> > > +- For enabling a single file (e.g. foo.o)::
> > > +
> > > +        AUTOFDO_PROFILE_foo.o := y
> 
> Do you mean we don't use ".. code-block:: " here and just use indented
> text separated with blank lines?

The double column (::) in the end of a paragraph means that the next paragraph
is a literal block:

https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks

You'd loose the coloring, but other than that it won't be different from
the ".. code-block::" and easier to read in a text form.
 
> -Rong

-- 
Sincerely yours,
Mike.
Re: [PATCH v2 1/6] Add AutoFDO support for Clang build
Posted by Rong Xu 1 month, 3 weeks ago
On Fri, Oct 4, 2024 at 9:38 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Fri, Oct 04, 2024 at 09:28:36AM -0700, Rong Xu wrote:
> > On Thu, Oct 3, 2024 at 11:09 PM Mike Rapoport <rppt@kernel.org> wrote:
> > >
> > > On Thu, Oct 03, 2024 at 11:20:17AM -0700, Rong Xu wrote:
> > > > Writing the doc with all these code-blocks was not fun either.
> > > > We are happy to change if there is a better way for this.
> > > >
> > > > -Rong
> > > >
> > > > On Thu, Oct 3, 2024 at 9:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > > > >
> > > > > On Thu, Oct 03, 2024 at 09:11:34AM -0700, Nick Desaulniers wrote:
> > > > >
> > > > > > > It makes it absolute crap for all of us who 'render' text documents
> > > > > > > using less or vi.
> > > > > >
> > > > > > "It hurts when I punch myself in the face."
> > > > >
> > > > > Weirdly enough I have a job that entails staring at text documents in
> > > > > text editors all day every day :-) sorry for thinking that's a sane
> > > > > thing to do.
> > >
> > > Something like this should do:
> > >
> > > > +- For enabling a single file (e.g. foo.o)::
> > > > +
> > > > +        AUTOFDO_PROFILE_foo.o := y
> >
> > Do you mean we don't use ".. code-block:: " here and just use indented
> > text separated with blank lines?
>
> The double column (::) in the end of a paragraph means that the next paragraph
> is a literal block:
>
> https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks
>
> You'd loose the coloring, but other than that it won't be different from
> the ".. code-block::" and easier to read in a text form.

Sorry that I did not notice the (::) in your suggestion.

This is great. I'll change the docs in this patch to use this method.

-Rong
>
> > -Rong
>
> --
> Sincerely yours,
> Mike.