From 9b20764f35094963ecbfc40a972ef54443630447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:30:13 +0100 Subject: [PATCH 1/8] features: --version now logs +/-pgo That is, whether the binary was compiled with PGO or not. --- client.c | 6 ++++-- foot-features.h | 9 +++++++++ main.c | 6 ++++-- meson.build | 3 +++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/client.c b/client.c index 975269b1..93dcc666 100644 --- a/client.c +++ b/client.c @@ -35,8 +35,10 @@ static const char * version_and_features(void) { static char buf[256]; - snprintf(buf, sizeof(buf), "version: %s %cime", - FOOT_VERSION, feature_ime() ? '+' : '-'); + snprintf(buf, sizeof(buf), "version: %s %cime %cpgo", + FOOT_VERSION, + feature_ime() ? '+' : '-', + feature_pgo() ? '+' : '-'); return buf; } diff --git a/foot-features.h b/foot-features.h index 8da22f6c..ae00c564 100644 --- a/foot-features.h +++ b/foot-features.h @@ -10,3 +10,12 @@ static inline bool feature_ime(void) return false; #endif } + +static inline bool feature_pgo(void) +{ +#if defined(FOOT_PGO_ENABLED) && FOOT_PGO_ENABLED + return true; +#else + return false; +#endif +} diff --git a/main.c b/main.c index 0941d3db..2c14d6d0 100644 --- a/main.c +++ b/main.c @@ -45,8 +45,10 @@ static const char * version_and_features(void) { static char buf[256]; - snprintf(buf, sizeof(buf), "version: %s %cime", - FOOT_VERSION, feature_ime() ? '+' : '-'); + snprintf(buf, sizeof(buf), "version: %s %cime %cpgo", + FOOT_VERSION, + feature_ime() ? '+' : '-', + feature_pgo() ? '+' : '-'); return buf; } diff --git a/meson.build b/meson.build index 01f27be9..e1b23102 100644 --- a/meson.build +++ b/meson.build @@ -24,6 +24,9 @@ add_project_arguments( (get_option('ime') ? ['-DFOOT_IME_ENABLED=1'] : []) + + (get_option('b_pgo') == 'use' + ? ['-DFOOT_PGO_ENABLED=1'] + : []) + cc.get_supported_arguments( ['-pedantic', '-fstrict-aliasing', From c6fb10863d0f985c1b9eb6a859086e761d393547 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:31:09 +0100 Subject: [PATCH 2/8] meson: build source files common to both foot and footclient as libraries Source files used by both foot and footclient are now compiled as static libraries, which foot and footclient links against. --- meson.build | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/meson.build b/meson.build index e1b23102..237d99c6 100644 --- a/meson.build +++ b/meson.build @@ -117,16 +117,18 @@ version = custom_target( output: 'version.h', command: [generate_version_sh, meson.project_version(), '@SOURCE_ROOT@', '@OUTPUT@']) +log = static_library('log', 'log.c', 'log.h') +debug = static_library('debug', 'debug.c', 'debug.h') + +xmalloc = static_library( + 'xmalloc', 'xmalloc.c', 'xmalloc.h', 'xsnprintf.c', 'xsnprintf.h') + misc = static_library( 'misc', - 'debug.c', 'debug.h', 'hsl.c', 'hsl.h', - 'log.c', 'log.h', 'macros.h', 'misc.c', 'misc.h', - 'uri.c', 'uri.h', - 'xmalloc.c', 'xmalloc.h', - 'xsnprintf.c', 'xsnprintf.h', + 'uri.c', 'uri.h' ) vtlib = static_library( @@ -140,7 +142,7 @@ vtlib = static_library( wl_proto_src + wl_proto_headers, version, dependencies: [libepoll, pixman, fcft, tllist, wayland_client], - link_with: misc, + link_with: [log, debug, misc, xmalloc], ) pgolib = static_library( @@ -195,14 +197,11 @@ executable( executable( 'footclient', 'client.c', 'client-protocol.h', - 'debug.c', 'debug.h', 'foot-features.h', - 'log.c', 'log.h', 'macros.h', 'util.h', - 'xmalloc.c', 'xmalloc.h', - 'xsnprintf.c', 'xsnprintf.h', version, + link_with: [log, debug, xmalloc], install: true) if tic.found() From ae6a656f4963b8cc5bde13fa347cfd4b76a4d4a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:34:18 +0100 Subject: [PATCH 3/8] meson: only build the pgo helper binary when -Db_pgo=generate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the only time it’s needed. In non-PGO builds, it is completely unnecessary, and we’re only wasting CPU cycles building it. In full PGO builds, with -Db_pgo=use, we get link warnings and/or failures, depending on compiler, since the pgo binary hasn’t been executed. --- meson.build | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/meson.build b/meson.build index 237d99c6..54e5b315 100644 --- a/meson.build +++ b/meson.build @@ -155,13 +155,15 @@ pgolib = static_library( link_with: vtlib, ) -executable( - 'pgo', - 'pgo/pgo.c', - wl_proto_src + wl_proto_headers, - dependencies: [math, threads, libepoll, pixman, wayland_client, fcft, tllist], - link_with: pgolib, -) +if get_option('b_pgo') == 'generate' + executable( + 'pgo', + 'pgo/pgo.c', + wl_proto_src + wl_proto_headers, + dependencies: [math, threads, libepoll, pixman, wayland_client, fcft, tllist], + link_with: pgolib, + ) +endif executable( 'foot', From 628382bc90f0bed2574752b2317ab25415a38213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:36:34 +0100 Subject: [PATCH 4/8] PKGBUILD: do PGO with either gcc or clang, but nothing else MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this, we assumed gcc was being used. The build would fail if clang (or something else) was used. Now, we check whether we’re compiling with gcc or clang, and disable PGO if it’s neither of them. Furthermore, we’re now adding the necessary flags needed to do PGO with clang. Note that ‘llvm-profdata’, from the ‘llvm’ package is needed for PGO:ing with clang. PGO is disabled if it isn’t available. It would be nice to have ‘llvm’ as an optional make dependency, but PKGBUILDs doesn’t appear to support such a thing. Finally, the -Wno-missing-profile flag, and its friends, have been removed; instead we execute “footclient --version” (and “foot --version”, in partial PGO builds) to ensure all generated binaries have been executed before we do the final build (with -Db_pgo=use). This fixes build failures with clang >= 11.x. --- PKGBUILD | 77 ++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 22 deletions(-) diff --git a/PKGBUILD b/PKGBUILD index d5a99516..67894b80 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -14,37 +14,70 @@ pkgver() { } build() { + local compiler=other + local do_pgo=no + # makepkg uses -O2 by default, but we *really* want -O3 - # -Wno-missing-profile since we're not exercising everything when doing PGO builds - export CFLAGS+=" -O3 -Wno-missing-profile" + CFLAGS+=" -O3" + + # Figure out which compiler we're using, and whether or not to do PGO + case $(${CC-cc} --version) in + *GCC*) + compiler=gcc + do_pgo=yes + ;; + + *clang*) + compiler=clang + + # We need llvm to be able to manage the profiling data + if command -v llvm-profdata > /dev/null; then + do_pgo=yes + + # Meson adds -fprofile-correction, which Clang doesn't + # understand + CFLAGS+=" -Wno-ignored-optimization-argument" + fi + ;; + esac meson --prefix=/usr --buildtype=release --wrap-mode=nofallback -Db_lto=true .. - find -name "*.gcda" -delete - meson configure -Db_pgo=generate - ninja + if [[ ${do_pgo} == yes ]]; then + find -name "*.gcda" -delete + meson configure -Db_pgo=generate + ninja - script_options="--scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline --sixel" + local script_options="--scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline --sixel" - tmp_file=$(mktemp) + tmp_file=$(mktemp) - if [[ -v WAYLAND_DISPLAY ]]; then - ./foot \ - --config /dev/null \ - --term=xterm \ - sh -c "../scripts/generate-alt-random-writes.py ${script_options} ${tmp_file} && cat ${tmp_file}" - else - ../scripts/generate-alt-random-writes.py \ - --rows=67 \ - --cols=135 \ - ${script_options} \ - ${tmp_file} - ./pgo ${tmp_file} ${tmp_file} ${tmp_file} + if [[ -v WAYLAND_DISPLAY ]]; then + ./footclient --version + ./foot \ + --config /dev/null \ + --term=xterm \ + sh -c "../scripts/generate-alt-random-writes.py ${script_options} ${tmp_file} && cat ${tmp_file}" + else + ./footclient --version + ./foot --version + ../scripts/generate-alt-random-writes.py \ + --rows=67 \ + --cols=135 \ + ${script_options} \ + ${tmp_file} + ./pgo ${tmp_file} ${tmp_file} ${tmp_file} + fi + + rm "${tmp_file}" + + if [[ ${compiler} == clang ]]; then + llvm-profdata merge default_*profraw --output=default.profdata + fi + + meson configure -Db_pgo=use fi - rm "${tmp_file}" - - meson configure -Db_pgo=use ninja } From 243578d308e2ce27fed24250dac5b363be1c05c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:43:03 +0100 Subject: [PATCH 5/8] =?UTF-8?q?install:=20add=20=E2=80=98llvm=E2=80=99=20a?= =?UTF-8?q?s=20an=20optional=20build=20dependency?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is needed for PGO builds with Clang. --- INSTALL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/INSTALL.md b/INSTALL.md index 1cb0d9a7..33f6aed7 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -65,6 +65,7 @@ In addition to the dev variant of the packages above, you need: * wayland protocols * ncurses (needed to generate terminfo) * scdoc (for man page generation) +* llvm (for PGO builds with Clang) * [tllist](https://codeberg.org/dnkl/tllist) [^1] A note on compilers; in general, foot runs **much** faster when From 3fd9256c0207cd7ff0b36b69282d70463be4dfb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 20:43:21 +0100 Subject: [PATCH 6/8] install: clang-11.x compatible PGO instructions LLVM/Clang 11.x throws errors for _files_ without any profiling data: error: ../../async.c: Function control flow change detected (hash mismatch) async_write Hash = 72057641800614222 [-Werror,-Wbackend-plugin] This caused multiple issues with the current PGO instructions: * The `pgo` binary failed to link with `meson configure -Db_pgo=use` when doing a full PGO build (since it isn't used in this case); * `footclient` fails to link for the same reason. * `foot` fails to link for the same reason in **partial** PGO builds. The solution is to make sure all binaries that are built in the final phase (`-Db_pgo=use`) have been executed in the _generate_ phase. Doing this also means we can drop `-Wno-missing-profile` and friends. Also add `--sixel` to the `generate-alt-random-writes` command line in the description for a full PGO build. Closes #418 --- INSTALL.md | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index 33f6aed7..9fa651e1 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -185,21 +185,15 @@ slower!) binary. First, configure the build directory: ```sh -export CFLAGS="$CFLAGS -O3 -Wno-missing-profile" +export CFLAGS="$CFLAGS -O3" meson --buildtype=release --prefix=/usr -Db_lto=true ../.. ``` It is **very** important `-O3` is being used here, as GCC-10.1.x and later have a regression where PGO with `-O2` is **much** slower. -If you are using Clang instead of GCC, use the following `CFLAGS` instead: - -```sh -export CFLAGS="$CFLAGS -O3 \ - -Wno-ignored-optimization-argument \ - -Wno-profile-instr-out-of-date \ - -Wno-profile-instr-unprofiled" -``` +Clang users **must** add `-Wno-ignored-optimization-argument` to +`CFLAGS`. Then, tell meson we want to _generate_ profiling data, and build: @@ -235,6 +229,8 @@ We will use the `pgo` binary along with input corpus generated by `scripts/generate-alt-random-writes.py`: ```sh +./footclient --version +./foot --version tmp_file=$(mktemp) ../../scripts/generate-alt-random-writes \ --rows=67 \ @@ -254,7 +250,12 @@ tmp_file=$(mktemp) rm ${tmp_file} ``` -The snippet above first creates an (empty) temporary file. Then, it +The first step, running `./foot --version` and `./footclient +--version` might seem unnecessary, but is needed to ensure we have +_some_ profiling data for functions not covered by the PGO helper +binary. Without this, the final link phase will fail. + +The snippet above then creates an (empty) temporary file. Then, it runs a script that generates random escape sequences (if you cat `${tmp_file}` in a terminal, you’ll see random colored characters all over the screen). Finally, we feed the randomly generated escape @@ -272,14 +273,19 @@ This method requires a running Wayland session. We will use the script `scripts/generate-alt-random-writes.py`: ```sh +./footclient --version foot_tmp_file=$(mktemp) -./foot --config=/dev/null --term=xterm sh -c " --scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline ${foot_tmp_file} && cat ${foot_tmp_file}" +./foot --config=/dev/null --term=xterm sh -c " --scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline --sixel ${foot_tmp_file} && cat ${foot_tmp_file}" rm ${foot_tmp_file} ``` You should see a foot window open up, with random colored text. The window should close after ~1-2s. +The first step, `./footclient --version` might seem unnecessary, but +is needed to ensure we have _some_ profiling data for +`footclient`. Without this, the final link phase will fail. + ##### Use the generated PGO data From 9786197d036e70ebe51c33a40f4bf2489f29e115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 26 Mar 2021 21:00:41 +0100 Subject: [PATCH 7/8] changelog: updated PGO build instructions --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53f95872..0b10e083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,11 @@ ## Unreleased ### Added ### Changed + +* Update PGO build instructions in `INSTALL.md` + (https://codeberg.org/dnkl/foot/issues/418). + + ### Deprecated ### Removed ### Fixed From b5ceed7b2bfa41584e17d156ecac231dde195798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 27 Mar 2021 13:17:43 +0100 Subject: [PATCH 8/8] =?UTF-8?q?meson:=20replace=20log+debug+xmalloc=20stat?= =?UTF-8?q?ic=20libraries=20with=20a=20single=20=E2=80=98common=E2=80=99?= =?UTF-8?q?=20library?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are cyclic dependencies between the log, debug and xmalloc libraries. While having them as separate static libraries work, as long as consumers of them link against all of them, having them in a single library feels slightly better. --- meson.build | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/meson.build b/meson.build index 54e5b315..9fbf99a6 100644 --- a/meson.build +++ b/meson.build @@ -117,11 +117,13 @@ version = custom_target( output: 'version.h', command: [generate_version_sh, meson.project_version(), '@SOURCE_ROOT@', '@OUTPUT@']) -log = static_library('log', 'log.c', 'log.h') -debug = static_library('debug', 'debug.c', 'debug.h') - -xmalloc = static_library( - 'xmalloc', 'xmalloc.c', 'xmalloc.h', 'xsnprintf.c', 'xsnprintf.h') +common = static_library( + 'common', + 'log.c', 'log.h', + 'debug.c', 'debug.h', + 'xmalloc.c', 'xmalloc.h', + 'xsnprintf.c', 'xsnprintf.h' +) misc = static_library( 'misc', @@ -142,7 +144,7 @@ vtlib = static_library( wl_proto_src + wl_proto_headers, version, dependencies: [libepoll, pixman, fcft, tllist, wayland_client], - link_with: [log, debug, misc, xmalloc], + link_with: [common, misc], ) pgolib = static_library( @@ -203,7 +205,7 @@ executable( 'macros.h', 'util.h', version, - link_with: [log, debug, xmalloc], + link_with: common, install: true) if tic.found()