From f43155962241d4614f3b5ca3555a2533b81a37f7 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 11 Sep 2025 23:57:28 -0400 Subject: [PATCH] [libc++][WIP] Build Google Benchmark directly from Lit This removes a weird interconnection between libc++'s own build and the configuration of the test suite, which should be disjoint. After this patch, Google Benchmark gets built using the same flags used for the rest of the test suite, which provides additional flexibility. --- libcxx/test/CMakeLists.txt | 1 - libcxx/test/benchmarks/CMakeLists.txt | 48 ---------------------- libcxx/test/configs/cmake-bridge.cfg.in | 1 - libcxx/utils/libcxx/test/config.py | 2 +- libcxx/utils/libcxx/test/format.py | 23 ++++++++--- libcxxabi/test/configs/cmake-bridge.cfg.in | 1 - libunwind/test/configs/cmake-bridge.cfg.in | 1 - third-party/benchmark/CMakeLists.txt | 4 +- 8 files changed, 20 insertions(+), 61 deletions(-) delete mode 100644 libcxx/test/benchmarks/CMakeLists.txt diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt index f4e577aed57de..4107793be281e 100644 --- a/libcxx/test/CMakeLists.txt +++ b/libcxx/test/CMakeLists.txt @@ -63,7 +63,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 b0fe600623d96..0000000000000 --- 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 AND NOT APPLE) - 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= - -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/configs/cmake-bridge.cfg.in b/libcxx/test/configs/cmake-bridge.cfg.in index 20b7c1e9bc357..1207c25cd7c9c 100644 --- a/libcxx/test/configs/cmake-bridge.cfg.in +++ b/libcxx/test/configs/cmake-bridge.cfg.in @@ -33,5 +33,4 @@ config.substitutions.append(('%{target-include-dir}', '@LIBCXX_TESTING_INSTALL_P config.substitutions.append(('%{lib-dir}', '@LIBCXX_TESTING_INSTALL_PREFIX@/@LIBCXX_INSTALL_LIBRARY_DIR@')) config.substitutions.append(('%{module-dir}', '@LIBCXX_TESTING_INSTALL_PREFIX@/@LIBCXX_INSTALL_MODULES_DIR@')) config.substitutions.append(('%{test-tools-dir}', '@LIBCXX_TEST_TOOLS_PATH@')) -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 0840c46d7bfae..9a71c494030d7 100644 --- a/libcxx/utils/libcxx/test/config.py +++ b/libcxx/utils/libcxx/test/config.py @@ -52,7 +52,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}"): note("Using {} substitution: '{}'".format(sub, _getSubstitution(sub, config))) # Print all available features diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py index 5765afec399cf..1b3268a0549e0 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): @@ -230,10 +230,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 @@ -350,8 +346,23 @@ def execute(self, test, litConfig): test.getFullName() ), ) + # TODO: %{libcxx-dir} is not a base substitution + # TODO: We're building Google Benchmark for every test, which is super wasteful + configure = ['cmake', '-S', '%{libcxx-dir}/../third-party/benchmark', '-B', '%T/gbench-build'] + configure += ['-D', 'CMAKE_INSTALL_PREFIX=%T/gbench'] + configure += ['-D', 'CMAKE_CXX_COMPILER=%{cxx}'] + configure += ['-D', 'CMAKE_BUILD_TYPE=Release'] + configure += ['-D', 'CMAKE_CXX_FLAGS="%{flags} %{compile_flags} %{link_flags} -Wno-error"'] + configure += ['-D', 'BENCHMARK_ENABLE_TESTING=OFF'] + configure += ['-D', 'BENCHMARK_ENABLE_WERROR=OFF'] + configure += ['-D', 'BENCHMARK_INSTALL_DOCS=OFF'] + build = ['cmake', '--build', '%T/gbench-build', '--target', 'install'] + steps = [ - "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{benchmark_flags} %{link_flags} -o %t.exe", + "rm -rf %T/gbench %T/gbench-build", + "%dbg(CONFIGURING GBENCH) " + " ".join(configure), + "%dbg(BUILDING GBENCH) " + " ".join(build), + "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{link_flags} -I %T/gbench/include -L %T/gbench/lib -l benchmark -o %t.exe", ] if "enable-benchmarks=run" in test.config.available_features: steps += ["%dbg(EXECUTED AS) %{exec} %t.exe --benchmark_out=%T/benchmark-result.json --benchmark_out_format=json"] diff --git a/libcxxabi/test/configs/cmake-bridge.cfg.in b/libcxxabi/test/configs/cmake-bridge.cfg.in index f81dd8afb1091..a2fa847f2d6c6 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 b804c210f0bbc..20b61e788ab18 100644 --- a/libunwind/test/configs/cmake-bridge.cfg.in +++ b/libunwind/test/configs/cmake-bridge.cfg.in @@ -32,4 +32,3 @@ if not @LIBUNWIND_ENABLE_THREADS@: 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}', '')) diff --git a/third-party/benchmark/CMakeLists.txt b/third-party/benchmark/CMakeLists.txt index d9bcc6a4939be..b4ecd2cea02e4 100644 --- a/third-party/benchmark/CMakeLists.txt +++ b/third-party/benchmark/CMakeLists.txt @@ -197,8 +197,8 @@ else() # Disable warning when compiling tests as gtest does not use 'override'. add_cxx_compiler_flag(-Wsuggest-override) endif() - add_cxx_compiler_flag(-pedantic) - add_cxx_compiler_flag(-pedantic-errors) + # add_cxx_compiler_flag(-pedantic) + # add_cxx_compiler_flag(-pedantic-errors) add_cxx_compiler_flag(-Wshorten-64-to-32) add_cxx_compiler_flag(-fstrict-aliasing) # Disable warnings regarding deprecated parts of the library while building