https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/224192
>From 7c260aec21b37e71e8070a84ad8278fdfc430324 Mon Sep 17 00:00:00 2001 From: Louis Dionne <[email protected]> Date: Wed, 16 Sep 2026 10:31:15 -0700 Subject: [PATCH 1/2] [libc++] Build GoogleBenchmark directly from Lit Previously, we would build GoogleBenchmark against the just-built library, not against the library being tested. When testing historical versions of libc++ or other standard libraries, this breaks. So instead of building Google Benchmark against the just-built library in CMake, do it from Lit as part of the test suite's configuration. I'm not a huge fan of using Lit as a poor man's build system and we should make the CMake test suite self-contained, however this is a step in the right direction and it removes a major coupling between the test suite and the regular libc++ build. --- libcxx/test/CMakeLists.txt | 1 - libcxx/test/benchmarks/CMakeLists.txt | 48 ----- libcxx/test/benchmarks/lit.local.cfg | 11 + .../test/configs/harness-configuration.cfg.in | 1 - libcxx/utils/libcxx/test/config.py | 2 +- libcxx/utils/libcxx/test/format.py | 13 +- libcxx/utils/libcxx/test/googlebenchmark.py | 192 ++++++++++++++++++ libcxxabi/test/configs/cmake-bridge.cfg.in | 1 - libunwind/test/configs/cmake-bridge.cfg.in | 1 - 9 files changed, 212 insertions(+), 58 deletions(-) delete mode 100644 libcxx/test/benchmarks/CMakeLists.txt create mode 100644 libcxx/test/benchmarks/lit.local.cfg create mode 100644 libcxx/utils/libcxx/test/googlebenchmark.py diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt index 8fd34c086a9928..85fa6aacc7796f 100644 --- a/libcxx/test/CMakeLists.txt +++ b/libcxx/test/CMakeLists.txt @@ -26,7 +26,6 @@ set(SERIALIZED_LIT_PARAMS "# Lit parameters serialized here for llvm-lit to pick serialize_lit_string_param(SERIALIZED_LIT_PARAMS compiler "${CMAKE_CXX_COMPILER}") if (LIBCXX_INCLUDE_BENCHMARKS) - add_subdirectory(benchmarks) set(_libcxx_benchmark_mode "dry-run") else() serialize_lit_string_param(SERIALIZED_LIT_PARAMS enable_benchmarks "no") diff --git a/libcxx/test/benchmarks/CMakeLists.txt b/libcxx/test/benchmarks/CMakeLists.txt deleted file mode 100644 index b3f881a008b808..00000000000000 --- a/libcxx/test/benchmarks/CMakeLists.txt +++ /dev/null @@ -1,48 +0,0 @@ -#============================================================================== -# Build Google Benchmark -#============================================================================== - -include(ExternalProject) -set(BENCHMARK_COMPILE_FLAGS - -Wno-unused-command-line-argument - -nostdinc++ - -isystem "${LIBCXX_GENERATED_INCLUDE_DIR}" - -L${LIBCXX_LIBRARY_DIR} - -Wl,-rpath,${LIBCXX_LIBRARY_DIR} - ${SANITIZER_FLAGS} - ) -if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR) - list(APPEND BENCHMARK_COMPILE_FLAGS - -isystem "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}") -endif() -if (DEFINED LIBCXX_CXX_ABI_LIBRARY_PATH) - list(APPEND BENCHMARK_COMPILE_FLAGS - -L${LIBCXX_CXX_ABI_LIBRARY_PATH} - -Wl,-rpath,${LIBCXX_CXX_ABI_LIBRARY_PATH}) -endif() -split_list(BENCHMARK_COMPILE_FLAGS) - -set(BENCHMARK_CXX_LIBRARIES) -list(APPEND BENCHMARK_CXX_LIBRARIES c++) -if (NOT LIBCXX_ENABLE_SHARED) - list(APPEND BENCHMARK_CXX_LIBRARIES c++abi) -endif() - -ExternalProject_Add(google-benchmark - EXCLUDE_FROM_ALL ON - DEPENDS cxx cxx-headers - PREFIX google-benchmark - SOURCE_DIR ${LLVM_THIRD_PARTY_DIR}/benchmark - INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/google-benchmark - CMAKE_CACHE_ARGS - -DCMAKE_C_COMPILER:FILEPATH=${CMAKE_C_COMPILER} - -DCMAKE_CXX_COMPILER:FILEPATH=${CMAKE_CXX_COMPILER} - -DCMAKE_MAKE_PROGRAM:FILEPATH=${CMAKE_MAKE_PROGRAM} - -DCMAKE_BUILD_TYPE:STRING=RELEASE - -DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR> - -DCMAKE_CXX_FLAGS:STRING=${BENCHMARK_COMPILE_FLAGS} - -DBENCHMARK_USE_LIBCXX:BOOL=ON - -DBENCHMARK_ENABLE_TESTING:BOOL=OFF - -DBENCHMARK_CXX_LIBRARIES:STRING=${BENCHMARK_CXX_LIBRARIES}) - -add_dependencies(cxx-test-depends google-benchmark) diff --git a/libcxx/test/benchmarks/lit.local.cfg b/libcxx/test/benchmarks/lit.local.cfg new file mode 100644 index 00000000000000..07a15e4ae5df95 --- /dev/null +++ b/libcxx/test/benchmarks/lit.local.cfg @@ -0,0 +1,11 @@ +# Build the GoogleBenchmark library using the current Lit configuration so +# that benchmarks can link against it. + +import libcxx.test.googlebenchmark + +if "enable-benchmarks=no" in config.available_features: + config.substitutions.append(("%{benchmark_flags}", "")) +else: + config.substitutions.append( + ("%{benchmark_flags}", libcxx.test.googlebenchmark.prepare(config, lit_config)) + ) diff --git a/libcxx/test/configs/harness-configuration.cfg.in b/libcxx/test/configs/harness-configuration.cfg.in index c527d60c64af5d..fab0ee69255014 100644 --- a/libcxx/test/configs/harness-configuration.cfg.in +++ b/libcxx/test/configs/harness-configuration.cfg.in @@ -26,5 +26,4 @@ config.test_exec_root = os.path.join('@LIBCXX_BINARY_DIR@', 'test') # Add substitutions for bootstrapping the test suite configuration config.substitutions.append(('%{libcxx-dir}', '@LIBCXX_SOURCE_DIR@')) -config.substitutions.append(('%{benchmark_flags}', '-I @LIBCXX_BINARY_DIR@/test/benchmarks/google-benchmark/include -L @LIBCXX_BINARY_DIR@/test/benchmarks/google-benchmark/lib -L @LIBCXX_BINARY_DIR@/test/benchmarks/google-benchmark/lib64 -l benchmark')) config.substitutions.append(("%{python}", shlex.quote(sys.executable))) diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py index fd6f1bf5accd67..c71e81cf9573ac 100644 --- a/libcxx/utils/libcxx/test/config.py +++ b/libcxx/utils/libcxx/test/config.py @@ -51,7 +51,7 @@ def configure(parameters, features, config, lit_config): ) # Print the basic substitutions - for sub in ("%{cxx}", "%{flags}", "%{compile_flags}", "%{link_flags}", "%{benchmark_flags}", "%{exec}"): + for sub in ("%{cxx}", "%{flags}", "%{compile_flags}", "%{link_flags}", "%{exec}"): debug("Using {} substitution: '{}'".format(sub, _getSubstitution(sub, config.substitutions))) # Print all available features diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py index 2f449e234e6d5e..3e24e435e7c07b 100644 --- a/libcxx/utils/libcxx/test/format.py +++ b/libcxx/utils/libcxx/test/format.py @@ -31,7 +31,7 @@ def _getTempPaths(test): def _checkBaseSubstitutions(substitutions): substitutions = [s for (s, _) in substitutions] - for s in ["%{cxx}", "%{compile_flags}", "%{link_flags}", "%{benchmark_flags}", "%{flags}", "%{exec}"]: + for s in ["%{cxx}", "%{compile_flags}", "%{link_flags}", "%{flags}", "%{exec}"]: assert s in substitutions, "Required substitution {} was not provided".format(s) def _executeScriptInternal(test, litConfig, commands): @@ -235,10 +235,6 @@ class CxxStandardLibraryTest(lit.formats.FileBasedTest): %{compile_flags} - Flags to use when compiling a test case %{link_flags} - Flags to use when linking a test case %{flags} - Flags to use either when compiling or linking a test case - %{benchmark_flags} - Flags to use when compiling benchmarks. These flags should provide access to - GoogleBenchmark but shouldn't hardcode any optimization level or other settings, - since the benchmarks should be run under the same configuration as the rest of - the test suite. %{exec} - A command to prefix the execution of executables Note that when building an executable (as opposed to only compiling a source @@ -359,6 +355,13 @@ def execute(self, test, litConfig): test.getFullName() ), ) + substitutions = [s for (s, _) in test.config.substitutions] + if "%{benchmark_flags}" not in substitutions: + return lit.Test.Result( + lit.Test.UNRESOLVED, + "Test {} is a benchmark, but the %{{benchmark_flags}} substitution " + "isn't provided by the configuration.".format(test.getFullName()), + ) steps = [ "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{benchmark_flags} %{link_flags} -o %t.exe", ] diff --git a/libcxx/utils/libcxx/test/googlebenchmark.py b/libcxx/utils/libcxx/test/googlebenchmark.py new file mode 100644 index 00000000000000..ec61b7fd5751a3 --- /dev/null +++ b/libcxx/utils/libcxx/test/googlebenchmark.py @@ -0,0 +1,192 @@ +# ===----------------------------------------------------------------------===## +# +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +# +# ===----------------------------------------------------------------------===## + +""" +Support for building GoogleBenchmark from the Lit configuration. + +The benchmarks in the test suite are linked against GoogleBenchmark, which must be +built with the same Standard Library (and more generally with the same ABI-affecting +flags) as the benchmarks themselves. This allows building GoogleBenchmark on-demand +from Lit using the flags of the configuration being tested. + +The result is cached inside the build directory so that subsequent invocations are +cheap. +""" + +import hashlib +import os +import shlex +import subprocess + +import lit.TestRunner + +import libcxx.test.config +import libcxx.test.dsl + +THIS_FILE = os.path.abspath(__file__) +LIBCXX_UTILS = os.path.dirname(os.path.dirname(os.path.dirname(THIS_FILE))) +MONOREPO_ROOT = os.path.dirname(os.path.dirname(LIBCXX_UTILS)) +SOURCE_DIR = os.path.join(MONOREPO_ROOT, "third-party", "benchmark") + +# Flags used by the test suite that must not be used when building GoogleBenchmark. +# Anything that isn't listed here is forwarded verbatim. +# +# -Werror +# Avoid failing GoogleBenchmark's build due to warnings. +# -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER +# Only relevant when testing libc++ itself. +# -fmodules, -fcxx-modules, -fmodules-cache-path= +# Modules are irrelevant when building a third-party static library, and sharing a +# module cache with the test suite is undesirable. +# -std= +# GoogleBenchmark sets CMAKE_CXX_STANDARD itself and requires C++17. +_DROPPED_FLAGS = { + "-Werror", + "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER", + "-fmodules", + "-fcxx-modules", +} + +_DROPPED_FLAG_PREFIXES = ( + "-std=", + "-fmodules-cache-path=", +) + +# Flags that are dropped along with the '-Xclang' that introduces them. +_DROPPED_XCLANG_FLAGS = { + "-fmodules-local-submodule-visibility", +} + + +def _expand(config, string): + """ + Expand the Lit substitutions in the given string, recursively. + """ + (expanded,) = lit.TestRunner.applySubstitutions( + [string], + config.substitutions, + recursion_limit=config.recursiveExpansionLimit, + ) + return expanded + + +def _filterFlags(flags): + """ + Remove the flags that must not be used when building GoogleBenchmark. + """ + result = [] + flags = iter(flags) + for flag in flags: + if flag == "-Xclang": + arg = next(flags, None) + if arg is None: + result.append(flag) + elif arg not in _DROPPED_XCLANG_FLAGS: + result += [flag, arg] + elif flag not in _DROPPED_FLAGS and not flag.startswith(_DROPPED_FLAG_PREFIXES): + result.append(flag) + return result + + +def _getFlags(config): + """ + Return the flags to build GoogleBenchmark with, based on the flags used by the + configuration under test. + """ + flags = [] + for substitution in ("%{flags}", "%{compile_flags}", "%{link_flags}"): + expanded = _expand(config, _getSubstitution(substitution, config)) + flags += shlex.split(expanded) + return _filterFlags(flags) + + +def _getSubstitution(substitution, config): + return libcxx.test.config._getSubstitution(substitution, config.substitutions) + + +def _fingerprint(config, flags): + """ + Return an opaque value identifying this GoogleBenchmark build. + + This changes whenever the compiler is rebuilt or whenever the flags used to build + GoogleBenchmark change. + """ + compiler = libcxx.test.dsl._compilerFingerprint(config) + return hashlib.sha256(repr((compiler, flags)).encode()).hexdigest()[:16] + + +def _run(litConfig, what, command, cwd): + result = subprocess.run( + command, + cwd=cwd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + universal_newlines=True, + ) + if result.returncode != 0: + pretty = " ".join(shlex.quote(arg) for arg in command) + litConfig.fatal( + "Failed to {} GoogleBenchmark.\n" + "Command was:\n{}\n\n" + "Output was:\n{}".format(what, pretty, result.stdout) + ) + + +def prepare(config, litConfig): + """ + Make GoogleBenchmark available to the test suite and return the flags required to + build the benchmarks against it. + + GoogleBenchmark is built using the same flags as the rest of the test suite, and the + result is cached inside the build directory. The cache is keyed on the flags being + used, so different Lit configurations do not interfere with each other. + """ + flags = _getFlags(config) + root = os.path.join(config.test_exec_root, "__gbench__") + prefix = os.path.join(root, _fingerprint(config, flags)) + buildDir = os.path.join(prefix, "build") + installDir = os.path.join(prefix, "install") + os.makedirs(root, exist_ok=True) + + cmake = os.environ.get("CMAKE", "cmake") + + if not os.path.exists(os.path.join(buildDir, "CMakeCache.txt")): + litConfig.note("Configuring GoogleBenchmark in {}".format(buildDir)) + compiler = _expand(config, _getSubstitution("%{cxx}", config)) + _run( + litConfig, + "configure", + [ + cmake, + "-S", SOURCE_DIR, + "-B", buildDir, + "-DCMAKE_BUILD_TYPE=Release", + "-DCMAKE_CXX_COMPILER={}".format(compiler), + "-DCMAKE_CXX_FLAGS={}".format(" ".join(flags)), + "-DCMAKE_INSTALL_PREFIX={}".format(installDir), + "-DCMAKE_INSTALL_LIBDIR=lib", + "-DBENCHMARK_ENABLE_TESTING=OFF", + "-DBENCHMARK_ENABLE_WERROR=OFF", + "-DBENCHMARK_INSTALL_DOCS=OFF", + ], + cwd=root, + ) + + # Always build: GoogleBenchmark is compiled against the headers of the library + # under test, so it must be rebuilt when those change. This is a no-op when + # nothing changed. + _run( + litConfig, + "build", + [cmake, "--build", buildDir, "--target", "install", "--parallel"], + cwd=root, + ) + + include = os.path.join(installDir, "include") + lib = os.path.join(installDir, "lib") + return "-isystem {} -L {} -l benchmark".format(include, lib) diff --git a/libcxxabi/test/configs/cmake-bridge.cfg.in b/libcxxabi/test/configs/cmake-bridge.cfg.in index f81dd8afb10915..a2fa847f2d6c66 100644 --- a/libcxxabi/test/configs/cmake-bridge.cfg.in +++ b/libcxxabi/test/configs/cmake-bridge.cfg.in @@ -34,7 +34,6 @@ config.substitutions.append(('%{include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/i config.substitutions.append(('%{cxx-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_INCLUDE_DIR@')) config.substitutions.append(('%{cxx-target-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_INCLUDE_TARGET_DIR@')) config.substitutions.append(('%{lib}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_LIBRARY_DIR@')) -config.substitutions.append(('%{benchmark_flags}', '')) if @LIBCXXABI_USE_LLVM_UNWINDER@: config.substitutions.append(('%{maybe-include-libunwind}', '-I "@LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL@"')) diff --git a/libunwind/test/configs/cmake-bridge.cfg.in b/libunwind/test/configs/cmake-bridge.cfg.in index ed0b2afbc0c0d2..98e7d8d653f5dc 100644 --- a/libunwind/test/configs/cmake-bridge.cfg.in +++ b/libunwind/test/configs/cmake-bridge.cfg.in @@ -45,7 +45,6 @@ config.substitutions.append(('%{libcxx}', '@LIBUNWIND_LIBCXX_PATH@')) config.substitutions.append(('%{install-prefix}', '@LIBUNWIND_TESTING_INSTALL_PREFIX@')) config.substitutions.append(('%{include}', '@LIBUNWIND_TESTING_INSTALL_PREFIX@/include')) config.substitutions.append(('%{lib}', '@LIBUNWIND_TESTING_INSTALL_PREFIX@/@LIBUNWIND_INSTALL_LIBRARY_DIR@')) -config.substitutions.append(('%{benchmark_flags}', '')) # Check for objcopy tools objcopy_path = which('llvm-objcopy', '@LLVM_BUILD_BINARY_DIR@/bin') >From 45ea531f7e3280b9d0332145bda6940c2da9957a Mon Sep 17 00:00:00 2001 From: Louis Dionne <[email protected]> Date: Thu, 17 Sep 2026 11:49:38 -0700 Subject: [PATCH 2/2] We have to pass the libraries separately to GoogleBenchmark --- libcxx/utils/libcxx/test/googlebenchmark.py | 48 +++++++++++++++++---- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/libcxx/utils/libcxx/test/googlebenchmark.py b/libcxx/utils/libcxx/test/googlebenchmark.py index ec61b7fd5751a3..58e39f1fd47a32 100644 --- a/libcxx/utils/libcxx/test/googlebenchmark.py +++ b/libcxx/utils/libcxx/test/googlebenchmark.py @@ -93,23 +93,48 @@ def _filterFlags(flags): return result -def _getFlags(config): +def _getFlags(config, substitutions): """ - Return the flags to build GoogleBenchmark with, based on the flags used by the + Return the flags contained in the given substitutions, based on the flags used by the configuration under test. """ flags = [] - for substitution in ("%{flags}", "%{compile_flags}", "%{link_flags}"): + for substitution in substitutions: expanded = _expand(config, _getSubstitution(substitution, config)) flags += shlex.split(expanded) return _filterFlags(flags) +def _splitLibraries(flags): + """ + Split the given link flags into (flags, libraries), where libraries contains the + name of the libraries that were being linked against. + + We can't simply hand the libraries over to CMake as part of CMAKE_CXX_FLAGS, since + CMake puts those flags before the object files on the link line. + """ + result = [] + libraries = [] + flags = iter(flags) + for flag in flags: + if flag == "-l": + library = next(flags, None) + if library is None: + result.append(flag) + else: + libraries.append(library) + elif flag.startswith("-l"): + libraries.append(flag[len("-l"):]) + else: + result.append(flag) + return (result, libraries) + + def _getSubstitution(substitution, config): return libcxx.test.config._getSubstitution(substitution, config.substitutions) -def _fingerprint(config, flags): +def _fingerprint(config, flags, libraries): """ Return an opaque value identifying this GoogleBenchmark build. @@ -117,7 +142,7 @@ def _fingerprint(config, flags): GoogleBenchmark change. """ compiler = libcxx.test.dsl._compilerFingerprint(config) - return hashlib.sha256(repr((compiler, flags)).encode()).hexdigest()[:16] + return hashlib.sha256(repr((compiler, flags, libraries)).encode()).hexdigest()[:16] def _run(litConfig, what, command, cwd): @@ -146,9 +171,11 @@ def prepare(config, litConfig): result is cached inside the build directory. The cache is keyed on the flags being used, so different Lit configurations do not interfere with each other. """ - flags = _getFlags(config) + flags, libraries = _splitLibraries( + _getFlags(config, ("%{flags}", "%{compile_flags}", "%{link_flags}")) + ) root = os.path.join(config.test_exec_root, "__gbench__") - prefix = os.path.join(root, _fingerprint(config, flags)) + prefix = os.path.join(root, _fingerprint(config, flags, libraries)) buildDir = os.path.join(prefix, "build") installDir = os.path.join(prefix, "install") os.makedirs(root, exist_ok=True) @@ -163,13 +190,16 @@ def prepare(config, litConfig): "configure", [ cmake, - "-S", SOURCE_DIR, - "-B", buildDir, + "-S", + SOURCE_DIR, + "-B", + buildDir, "-DCMAKE_BUILD_TYPE=Release", "-DCMAKE_CXX_COMPILER={}".format(compiler), "-DCMAKE_CXX_FLAGS={}".format(" ".join(flags)), "-DCMAKE_INSTALL_PREFIX={}".format(installDir), "-DCMAKE_INSTALL_LIBDIR=lib", + "-DBENCHMARK_CXX_LIBRARIES={}".format(";".join(libraries)), "-DBENCHMARK_ENABLE_TESTING=OFF", "-DBENCHMARK_ENABLE_WERROR=OFF", "-DBENCHMARK_INSTALL_DOCS=OFF", _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
