[libcxx-commits] [libcxx] [libc++] Remove special handling of the native C++ library in benchmarks (PR #98529)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jul 15 12:01:59 PDT 2024


https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/98529

>From f9c4f12e5be8bbfe9e2f5c212f33cf92934281c4 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Sat, 11 May 2024 10:38:30 -0400
Subject: [PATCH 1/3] [libc++] Remove special handling of the native C++
 library in benchmarks

There were some ad-hoc settings that allowed running the benchmarks against
the native C++ Standard Library. While this ability is very useful, it
was done before the test suite was quite independent of libc++ itself.
Instead, it is better to streamline running the benchmarks on the native
standard library by using a custom Lit configuration like we do with the
test suite.

A follow-up patch will rework the integration of benchmarks with the
Lit configuration used for the test suite so that we can reuse the same
mechanism for both, making it easy to benchmark the native standard library.

It will also make benchmarks way more user-friendly to run since we will
be able to run them like we run individual tests, which is a pain point
right now.
---
 libcxx/CMakeLists.txt            |  14 -----
 libcxx/benchmarks/CMakeLists.txt | 100 ++++++-------------------------
 libcxx/benchmarks/lit.cfg.py     |   2 +-
 libcxx/docs/BuildingLibcxx.rst   |  16 -----
 libcxx/docs/ReleaseNotes/19.rst  |   4 ++
 libcxx/docs/TestingLibcxx.rst    |  15 ++---
 6 files changed, 28 insertions(+), 123 deletions(-)

diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index e098bd574eec7..6e37174240ae9 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -159,20 +159,6 @@ set(LIBCXX_BENCHMARK_TEST_ARGS_DEFAULT --benchmark_min_time=0.01)
 set(LIBCXX_BENCHMARK_TEST_ARGS "${LIBCXX_BENCHMARK_TEST_ARGS_DEFAULT}" CACHE STRING
     "Arguments to pass when running the benchmarks using check-cxx-benchmarks")
 
-set(LIBCXX_BENCHMARK_NATIVE_STDLIB "" CACHE STRING
-        "Build the benchmarks against the specified native STL.
-         The value must be one of libc++/libstdc++")
-set(LIBCXX_BENCHMARK_NATIVE_GCC_TOOLCHAIN "" CACHE STRING
-    "Use alternate GCC toolchain when building the native benchmarks")
-
-if (LIBCXX_BENCHMARK_NATIVE_STDLIB)
-  if (NOT (LIBCXX_BENCHMARK_NATIVE_STDLIB STREQUAL "libc++"
-        OR LIBCXX_BENCHMARK_NATIVE_STDLIB STREQUAL "libstdc++"))
-    message(FATAL_ERROR "Invalid value for LIBCXX_BENCHMARK_NATIVE_STDLIB: "
-            "'${LIBCXX_BENCHMARK_NATIVE_STDLIB}'")
-  endif()
-endif()
-
 option(LIBCXX_INCLUDE_DOCS "Build the libc++ documentation." ${LLVM_INCLUDE_DOCS})
 set(LIBCXX_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}" CACHE STRING
     "Define suffix of library directory name (32/64)")
diff --git a/libcxx/benchmarks/CMakeLists.txt b/libcxx/benchmarks/CMakeLists.txt
index 2101f9c71788c..fa55e77d0825a 100644
--- a/libcxx/benchmarks/CMakeLists.txt
+++ b/libcxx/benchmarks/CMakeLists.txt
@@ -2,12 +2,12 @@ include(ExternalProject)
 include(CheckCXXCompilerFlag)
 
 #==============================================================================
-# Build Google Benchmark for libc++
+# Build Google Benchmark
 #==============================================================================
 
 set(CMAKE_FOLDER "${CMAKE_FOLDER}/Benchmarks")
 
-set(BENCHMARK_LIBCXX_COMPILE_FLAGS
+set(BENCHMARK_COMPILE_FLAGS
     -Wno-unused-command-line-argument
     -nostdinc++
     -isystem "${LIBCXX_GENERATED_INCLUDE_DIR}"
@@ -16,64 +16,37 @@ set(BENCHMARK_LIBCXX_COMPILE_FLAGS
     ${SANITIZER_FLAGS}
     )
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  list(APPEND BENCHMARK_LIBCXX_COMPILE_FLAGS
+  list(APPEND BENCHMARK_COMPILE_FLAGS
     -isystem "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}")
 endif()
 if (DEFINED LIBCXX_CXX_ABI_LIBRARY_PATH)
-  list(APPEND BENCHMARK_LIBCXX_COMPILE_FLAGS
+  list(APPEND BENCHMARK_COMPILE_FLAGS
           -L${LIBCXX_CXX_ABI_LIBRARY_PATH}
           -Wl,-rpath,${LIBCXX_CXX_ABI_LIBRARY_PATH})
 endif()
-split_list(BENCHMARK_LIBCXX_COMPILE_FLAGS)
+split_list(BENCHMARK_COMPILE_FLAGS)
 
-ExternalProject_Add(google-benchmark-libcxx
+ExternalProject_Add(google-benchmark
         EXCLUDE_FROM_ALL ON
         DEPENDS cxx cxx-headers
-        PREFIX benchmark-libcxx
+        PREFIX google-benchmark
         SOURCE_DIR ${LLVM_THIRD_PARTY_DIR}/benchmark
-        INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/benchmark-libcxx
+        INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/google-benchmark
         CMAKE_CACHE_ARGS
           -DCMAKE_C_COMPILER:STRING=${CMAKE_C_COMPILER}
           -DCMAKE_CXX_COMPILER:STRING=${CMAKE_CXX_COMPILER}
           -DCMAKE_BUILD_TYPE:STRING=RELEASE
           -DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR>
-          -DCMAKE_CXX_FLAGS:STRING=${BENCHMARK_LIBCXX_COMPILE_FLAGS}
+          -DCMAKE_CXX_FLAGS:STRING=${BENCHMARK_COMPILE_FLAGS}
           -DBENCHMARK_USE_LIBCXX:BOOL=ON
           -DBENCHMARK_ENABLE_TESTING:BOOL=OFF)
 
-#==============================================================================
-# Build Google Benchmark for the native stdlib
-#==============================================================================
-set(BENCHMARK_NATIVE_TARGET_FLAGS)
-if (LIBCXX_BENCHMARK_NATIVE_GCC_TOOLCHAIN)
-  set(BENCHMARK_NATIVE_TARGET_FLAGS
-      --gcc-toolchain=${LIBCXX_BENCHMARK_NATIVE_GCC_TOOLCHAIN})
-endif()
-split_list(BENCHMARK_NATIVE_TARGET_FLAGS)
-
-if (LIBCXX_BENCHMARK_NATIVE_STDLIB)
-  ExternalProject_Add(google-benchmark-native
-        EXCLUDE_FROM_ALL ON
-        PREFIX benchmark-native
-        SOURCE_DIR ${LLVM_THIRD_PARTY_DIR}/benchmark
-        INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/benchmark-native
-        CMAKE_CACHE_ARGS
-          -DCMAKE_C_COMPILER:STRING=${CMAKE_C_COMPILER}
-          -DCMAKE_CXX_COMPILER:STRING=${CMAKE_CXX_COMPILER}
-          -DCMAKE_CXX_FLAGS:STRING=${BENCHMARK_NATIVE_TARGET_FLAGS}
-          -DCMAKE_BUILD_TYPE:STRING=RELEASE
-          -DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR>
-          -DBENCHMARK_ENABLE_TESTING:BOOL=OFF)
-endif()
-
-
 #==============================================================================
 # Benchmark tests configuration
 #==============================================================================
 add_custom_target(cxx-benchmarks)
 set(BENCHMARK_OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR})
-set(BENCHMARK_LIBCXX_INSTALL ${CMAKE_CURRENT_BINARY_DIR}/benchmark-libcxx)
-set(BENCHMARK_NATIVE_INSTALL ${CMAKE_CURRENT_BINARY_DIR}/benchmark-native)
+set(BENCHMARK_INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/google-benchmark)
 
 add_library(               cxx-benchmarks-flags INTERFACE)
 
@@ -97,32 +70,14 @@ else()
   target_compile_features( cxx-benchmarks-flags INTERFACE cxx_std_23)
 endif()
 
-target_compile_options(    cxx-benchmarks-flags INTERFACE -fsized-deallocation -nostdinc++)
+target_compile_options(cxx-benchmarks-flags INTERFACE -fsized-deallocation -nostdinc++
+                                                      ${SANITIZER_FLAGS} -Wno-user-defined-literals -Wno-suggest-override)
 target_include_directories(cxx-benchmarks-flags INTERFACE "${LIBCXX_GENERATED_INCLUDE_DIR}"
-                                                INTERFACE "${BENCHMARK_LIBCXX_INSTALL}/include"
+                                                INTERFACE "${BENCHMARK_INSTALL_DIR}/include"
                                                 INTERFACE "${LIBCXX_SOURCE_DIR}/test/support")
-
-add_library(           cxx-benchmarks-flags-native INTERFACE)
-target_link_libraries( cxx-benchmarks-flags-native INTERFACE cxx-benchmarks-flags)
-target_compile_options(cxx-benchmarks-flags-native INTERFACE ${BENCHMARK_NATIVE_TARGET_FLAGS})
-target_link_options(   cxx-benchmarks-flags-native INTERFACE ${BENCHMARK_NATIVE_TARGET_FLAGS} "-L${BENCHMARK_NATIVE_INSTALL}/lib")
-if (LIBCXX_BENCHMARK_NATIVE_STDLIB STREQUAL "libstdc++")
-  find_library(LIBSTDCXX_FILESYSTEM_TEST stdc++fs
-        PATHS ${LIBCXX_BENCHMARK_NATIVE_GCC_TOOLCHAIN}
-        PATH_SUFFIXES lib lib64
-        DOC "The libstdc++ filesystem library used by the benchmarks"
-    )
-  if (LIBSTDCXX_FILESYSTEM_TEST)
-    target_link_libraries(cxx-benchmarks-flags-native INTERFACE -lstdc++fs)
-  endif()
-else()
-  target_link_libraries(cxx-benchmarks-flags-native INTERFACE -lc++fs -lc++experimental)
-endif()
-
-add_library(           cxx-benchmarks-flags-libcxx INTERFACE)
-target_link_libraries( cxx-benchmarks-flags-libcxx INTERFACE cxx-benchmarks-flags)
-target_compile_options(cxx-benchmarks-flags-libcxx INTERFACE ${SANITIZER_FLAGS} -Wno-user-defined-literals -Wno-suggest-override)
-target_link_options(   cxx-benchmarks-flags-libcxx INTERFACE -lm -nostdlib++ "-L${BENCHMARK_LIBCXX_INSTALL}/lib" "-L${BENCHMARK_LIBCXX_INSTALL}/lib64" ${SANITIZER_FLAGS})
+target_link_options(cxx-benchmarks-flags INTERFACE -lm -nostdlib++
+                                                   "-L${BENCHMARK_INSTALL_DIR}/lib" "-L${BENCHMARK_INSTALL_DIR}/lib64"
+                                                   ${SANITIZER_FLAGS})
 
 set(libcxx_benchmark_targets)
 
@@ -130,8 +85,8 @@ function(add_benchmark_test name source_file)
   set(libcxx_target ${name}_libcxx)
   list(APPEND libcxx_benchmark_targets ${libcxx_target})
   add_executable(${libcxx_target} EXCLUDE_FROM_ALL ${source_file})
-  target_link_libraries(${libcxx_target} PRIVATE cxx-benchmarks-flags-libcxx)
-  add_dependencies(${libcxx_target} cxx google-benchmark-libcxx)
+  target_link_libraries(${libcxx_target} PRIVATE cxx-benchmarks-flags)
+  add_dependencies(${libcxx_target} cxx google-benchmark)
   add_dependencies(cxx-benchmarks ${libcxx_target})
   if (LIBCXX_ENABLE_SHARED)
     target_link_libraries(${libcxx_target} PRIVATE cxx_shared)
@@ -144,27 +99,10 @@ function(add_benchmark_test name source_file)
   endif()
   set_target_properties(${libcxx_target}
     PROPERTIES
-          OUTPUT_NAME "${name}.libcxx.out"
+          OUTPUT_NAME "${name}.out"
           RUNTIME_OUTPUT_DIRECTORY "${BENCHMARK_OUTPUT_DIR}"
           CXX_EXTENSIONS NO)
   cxx_link_system_libraries(${libcxx_target})
-  if (LIBCXX_BENCHMARK_NATIVE_STDLIB)
-    set(native_target ${name}_native)
-    add_executable(${native_target} EXCLUDE_FROM_ALL ${source_file})
-    target_link_libraries(${native_target} PRIVATE cxx-benchmarks-flags-native)
-    add_dependencies(${native_target} google-benchmark-native
-                                      google-benchmark-libcxx)
-    target_link_libraries(${native_target} PRIVATE -lbenchmark)
-    if (LIBCXX_HAS_PTHREAD_LIB)
-      target_link_libraries(${native_target} PRIVATE -pthread)
-    endif()
-    add_dependencies(cxx-benchmarks ${native_target})
-    set_target_properties(${native_target}
-      PROPERTIES
-          OUTPUT_NAME "${name}.native.out"
-          RUNTIME_OUTPUT_DIRECTORY "${BENCHMARK_OUTPUT_DIR}"
-          CXX_EXTENSIONS NO)
-  endif()
 endfunction()
 
 
diff --git a/libcxx/benchmarks/lit.cfg.py b/libcxx/benchmarks/lit.cfg.py
index 7d222ddf9284e..a8209a90110cc 100644
--- a/libcxx/benchmarks/lit.cfg.py
+++ b/libcxx/benchmarks/lit.cfg.py
@@ -19,5 +19,5 @@
 config.test_source_root = config.test_exec_root
 
 config.test_format = GoogleBenchmark(
-    test_sub_dirs=".", test_suffix=".libcxx.out", benchmark_args=config.benchmark_args
+    test_sub_dirs=".", test_suffix=".out", benchmark_args=config.benchmark_args
 )
diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst
index e425b9dadfe7d..66bb19bb5b2cd 100644
--- a/libcxx/docs/BuildingLibcxx.rst
+++ b/libcxx/docs/BuildingLibcxx.rst
@@ -399,22 +399,6 @@ libc++ Feature Options
   since the primary use of ``check-cxx-benchmarks`` is to get test and sanitizer coverage, not to
   get accurate measurements.
 
-.. option:: LIBCXX_BENCHMARK_NATIVE_STDLIB:STRING
-
-  **Default**:: ``""``
-
-  **Values**:: ``libc++``, ``libstdc++``
-
-  Build the libc++ benchmark tests and Google Benchmark library against the
-  specified standard library on the platform. On Linux this can be used to
-  compare libc++ to libstdc++ by building the benchmark tests against both
-  standard libraries.
-
-.. option:: LIBCXX_BENCHMARK_NATIVE_GCC_TOOLCHAIN:STRING
-
-  Use the specified GCC toolchain and standard library when building the native
-  stdlib benchmark tests.
-
 .. option:: LIBCXX_ASSERTION_HANDLER_FILE:PATH
 
   **Default**:: ``"${CMAKE_CURRENT_SOURCE_DIR}/vendor/llvm/default_assertion_handler.in"``
diff --git a/libcxx/docs/ReleaseNotes/19.rst b/libcxx/docs/ReleaseNotes/19.rst
index b9c6bc84892c5..47f792066e925 100644
--- a/libcxx/docs/ReleaseNotes/19.rst
+++ b/libcxx/docs/ReleaseNotes/19.rst
@@ -169,3 +169,7 @@ Build System Changes
   to automatically detect the presence of ``clang-tidy`` and the required ``Clang`` libraries.
 
 - The CMake options ``LIBCXX_INSTALL_MODULES`` now defaults to ``ON``.
+
+- The CMake options ``LIBCXX_BENCHMARK_NATIVE_STDLIB`` and ``LIBCXX_BENCHMARK_NATIVE_GCC_TOOLCHAIN`` have
+  been removed. To benchmark the native standard library, configure the test suite against the native
+  standard library directly instead.
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index d9f4fe467fe36..071ed388e0a91 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -351,7 +351,7 @@ Test Filenames`_ when determining the names for new test files.
      - Same as ``FOO.pass.cpp``, but for Objective-C++.
 
    * - ``FOO.compile.pass.cpp``
-     - Checks whether the C++ code in the file compiles successfully. In general, prefer ``compile`` tests over ``verify`` tests, 
+     - Checks whether the C++ code in the file compiles successfully. In general, prefer ``compile`` tests over ``verify`` tests,
        subject to the specific recommendations, below, for when to write ``verify`` tests.
    * - ``FOO.compile.pass.mm``
      - Same as ``FOO.compile.pass.cpp``, but for Objective-C++.
@@ -447,19 +447,12 @@ An example build would look like:
 
 .. code-block:: bash
 
-  $ cd build
-  $ ninja cxx-benchmarks
+  $ ninja -C build cxx-benchmarks
 
 This will build all of the benchmarks under ``<libcxx-src>/benchmarks`` to be
 built against the just-built libc++. The compiled tests are output into
 ``build/projects/libcxx/benchmarks``.
 
-The benchmarks can also be built against the platforms native standard library
-using the ``-DLIBCXX_BUILD_BENCHMARKS_NATIVE_STDLIB=ON`` CMake option. This
-is useful for comparing the performance of libc++ to other standard libraries.
-The compiled benchmarks are named ``<test>.libcxx.out`` if they test libc++ and
-``<test>.native.out`` otherwise.
-
 Also See:
 
   * :ref:`Building Libc++ <build instructions>`
@@ -476,8 +469,8 @@ For example:
 .. code-block:: bash
 
   $ cd build/projects/libcxx/benchmarks
-  $ ./algorithms.libcxx.out # Runs all the benchmarks
-  $ ./algorithms.libcxx.out --benchmark_filter=BM_Sort.* # Only runs the sort benchmarks
+  $ ./algorithms.out # Runs all the benchmarks
+  $ ./algorithms.out --benchmark_filter=BM_Sort.* # Only runs the sort benchmarks
 
 For more information about running benchmarks see `Google Benchmark`_.
 

>From c6a36e69b0bcd18fc91d2c16d7af0c263fb0d8b4 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 12 Jul 2024 14:40:24 -0400
Subject: [PATCH 2/3] Avoid conflicting with random executables

---
 libcxx/benchmarks/CMakeLists.txt | 2 +-
 libcxx/benchmarks/lit.cfg.py     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/benchmarks/CMakeLists.txt b/libcxx/benchmarks/CMakeLists.txt
index fa55e77d0825a..110672600213a 100644
--- a/libcxx/benchmarks/CMakeLists.txt
+++ b/libcxx/benchmarks/CMakeLists.txt
@@ -99,7 +99,7 @@ function(add_benchmark_test name source_file)
   endif()
   set_target_properties(${libcxx_target}
     PROPERTIES
-          OUTPUT_NAME "${name}.out"
+          OUTPUT_NAME "${name}.bench.out"
           RUNTIME_OUTPUT_DIRECTORY "${BENCHMARK_OUTPUT_DIR}"
           CXX_EXTENSIONS NO)
   cxx_link_system_libraries(${libcxx_target})
diff --git a/libcxx/benchmarks/lit.cfg.py b/libcxx/benchmarks/lit.cfg.py
index a8209a90110cc..0d08966c26cc1 100644
--- a/libcxx/benchmarks/lit.cfg.py
+++ b/libcxx/benchmarks/lit.cfg.py
@@ -19,5 +19,5 @@
 config.test_source_root = config.test_exec_root
 
 config.test_format = GoogleBenchmark(
-    test_sub_dirs=".", test_suffix=".out", benchmark_args=config.benchmark_args
+    test_sub_dirs=".", test_suffix=".bench.out", benchmark_args=config.benchmark_args
 )

>From 996fdd8d36a0f6d6dd9a19fa5474ab8fcf326834 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 15 Jul 2024 15:01:48 -0400
Subject: [PATCH 3/3] Update documentation after changes

---
 libcxx/docs/TestingLibcxx.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index 071ed388e0a91..6d3417cabfd61 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -469,8 +469,8 @@ For example:
 .. code-block:: bash
 
   $ cd build/projects/libcxx/benchmarks
-  $ ./algorithms.out # Runs all the benchmarks
-  $ ./algorithms.out --benchmark_filter=BM_Sort.* # Only runs the sort benchmarks
+  $ ./algorithms.bench.out # Runs all the benchmarks
+  $ ./algorithms.bench.out --benchmark_filter=BM_Sort.* # Only runs the sort benchmarks
 
 For more information about running benchmarks see `Google Benchmark`_.
 



More information about the libcxx-commits mailing list