[libcxx-commits] [libcxx] [RFC][libc++][benchmark] Enable benchmark optimizations. (PR #132445)

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Fri Mar 21 11:22:01 PDT 2025


https://github.com/mordante created https://github.com/llvm/llvm-project/pull/132445

Before we moved our benchmark suite from a stand-alone suite to the current lit based suite the default was to run benchmarks optimized. With the change the default is to run not optimized. I've been bitten several times by this issue and need to spend time to look how to run these test optimized.

I feel this is a regression and also makes it harder for other people to do the right thing. I feel the default should be to do the right thing. I agree this somewhat differs from the idea that the test suite should be run with the same flags. Still I feel having the right default makes it easier to onboard in the project and not to remember these oddities.

This patch adds a new default optimization (bikeshedding is welcome) that keeps the main tests as if specified optimization=none and the benchmarks as if specified optimization=speed.
The other values keep their exisiting behaviour. (When the test are optimized for speed or size the benchmarks will inherit this flag without any changes.) The benchmarks can still be not opimized by using optimization=none.

This patch will need more cleanups and documentation.

>From 6c2051c9e50981953f9ece65de58be92cb42e769 Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Fri, 21 Mar 2025 19:11:34 +0100
Subject: [PATCH] [RFC][libc++][benchmark] Enable benchmark optimizations.

Before we moved our benchmark suite from a stand-alone suite to the
current lit based suite the default was to run benchmarks optimized.
With the change the default is to run not optimized. I've been bitten
several times by this issue and need to spend time to look how to run
these test optimized.

I feel this is a regression and also makes it harder for other people to
do the right thing. I feel the default should be to do the right thing.
I agree this somewhat differs from the idea that the test suite should
be run with the same flags. Still I feel having the right default makes
it easier to onboard in the project and not to remember these oddities.

This patch adds a new default optimization (bikeshedding is welcome)
that keeps the main tests as if specified optimization=none and the
benchmarks as if specified optimization=speed.
The other values keep their exisiting behaviour. (When the test are
optimized for speed or size the benchmarks will inherit this flag
without any changes.) The benchmarks can still be not opimized by using
optimization=none.

This patch will need more cleanups and documentation.
---
 libcxx/utils/libcxx/test/dsl.py    | 22 ++++++++++++++++++++++
 libcxx/utils/libcxx/test/params.py |  5 +++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index 9a97e61efbe7d..63fb580923467 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -511,6 +511,28 @@ def pretty(self, config, litParams):
         return "add {} to %{{compile_flags}}".format(self._getFlag(config))
 
 
+class AddBenchmarkCompileFlag(ConfigAction):
+    """
+    This action adds the given flag to the %{benchmark_flags} substitution.
+
+    The flag can be a string or a callable, in which case it is called with the
+    configuration to produce the actual flag (as a string).
+    """
+
+    def __init__(self, flag):
+        self._getFlag = lambda config: flag(config) if callable(flag) else flag
+
+    def applyTo(self, config):
+        flag = self._getFlag(config)
+        _ensureFlagIsSupported(config, flag)
+        config.substitutions = _appendToSubstitution(
+            config.substitutions, "%{benchmark_flags}", flag
+        )
+
+    def pretty(self, config, litParams):
+        return "add {} to %{{benchmark_flags}}".format(self._getFlag(config))
+
+
 class AddLinkFlag(ConfigAction):
     """
     This action appends the given flag to the %{link_flags} substitution.
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index 93dc3a8be3f08..512531f59f343 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -195,11 +195,12 @@ def getSuitableClangTidy(cfg):
     ),
     Parameter(
         name="optimization",
-        choices=["none", "speed", "size"],
+        choices=["default", "none", "speed", "size"],
         type=str,
         help="The optimization level to use when compiling the test suite.",
-        default="none",
+        default="default", # defaults, test -> none, benchmarks -> speed
         actions=lambda opt: filter(None, [
+            AddBenchmarkCompileFlag(lambda cfg: getSpeedOptimizationFlag(cfg)) if opt == "default" else None,
             AddCompileFlag(lambda cfg: getSpeedOptimizationFlag(cfg)) if opt == "speed" else None,
             AddCompileFlag(lambda cfg: getSizeOptimizationFlag(cfg)) if opt == "size" else None,
             AddFeature(f'optimization={opt}'),



More information about the libcxx-commits mailing list