Skip to content

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 23, 2025

Using -Wall and -Wextra when building SPEC leads to extremely large log files. Since we don't actually care about warnings in these benchmarks, this patches tones down the number of warnings a bit.

Using -Wall and -Wextra when building SPEC leads to extremely large
log files. Since we don't actually care about warnings in these
benchmarks, this patches tones down the number of warnings a bit.
@ldionne ldionne requested a review from a team as a code owner September 23, 2025 18:50
@ldionne ldionne requested a review from philnik777 September 23, 2025 18:50
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2025

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

Using -Wall and -Wextra when building SPEC leads to extremely large log files. Since we don't actually care about warnings in these benchmarks, this patches tones down the number of warnings a bit.


Full diff: https://github.com/llvm/llvm-project/pull/160366.diff

1 Files Affected:

  • (modified) libcxx/test/benchmarks/spec.gen.py (+4-1)
diff --git a/libcxx/test/benchmarks/spec.gen.py b/libcxx/test/benchmarks/spec.gen.py
index ea7b75b3d2085..de7c14db5ad8e 100644
--- a/libcxx/test/benchmarks/spec.gen.py
+++ b/libcxx/test/benchmarks/spec.gen.py
@@ -28,6 +28,9 @@
 link_flags = (test_dir / 'link_flags.subs').open().read().strip()
 spec_dir = pathlib.Path((test_dir / 'spec_dir.subs').open().read().strip())
 
+# Remove -Wall, -Wextra and -Werror from the flags since we don't care about enabling all warnings inside SPEC
+compile_flags = compile_flags.replace('-Wall', '').replace('-Wextra', '').replace('-Werror', '')
+
 # Setup the configuration file
 test_dir.mkdir(parents=True, exist_ok=True)
 spec_config = test_dir / 'spec-config.cfg'
@@ -46,7 +49,7 @@
     copies               = 1
     threads              = 1
     CC                   = cc -O3
-    CXX                  = {cxx} {compile_flags} {flags} {link_flags} -Wno-error
+    CXX                  = {cxx} {compile_flags} {flags} {link_flags}
     CC_VERSION_OPTION    = --version
     CXX_VERSION_OPTION   = --version
     EXTRA_PORTABILITY    = -DSPEC_NO_CXX17_SPECIAL_MATH_FUNCTIONS # because libc++ doesn't implement the special math functions yet

Copy link

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r origin/main...HEAD libcxx/test/benchmarks/spec.gen.py

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from darker here.
--- spec.gen.py	2025-09-23 18:48:17.000000 +0000
+++ spec.gen.py	2025-09-23 18:52:17.006291 +0000
@@ -27,16 +27,19 @@
 flags = (test_dir / 'flags.subs').open().read().strip()
 link_flags = (test_dir / 'link_flags.subs').open().read().strip()
 spec_dir = pathlib.Path((test_dir / 'spec_dir.subs').open().read().strip())
 
 # Remove -Wall, -Wextra and -Werror from the flags since we don't care about enabling all warnings inside SPEC
-compile_flags = compile_flags.replace('-Wall', '').replace('-Wextra', '').replace('-Werror', '')
+compile_flags = (
+    compile_flags.replace("-Wall", "").replace("-Wextra", "").replace("-Werror", "")
+)
 
 # Setup the configuration file
 test_dir.mkdir(parents=True, exist_ok=True)
-spec_config = test_dir / 'spec-config.cfg'
-spec_config.write_text(f"""
+spec_config = test_dir / "spec-config.cfg"
+spec_config.write_text(
+    f"""
 default:
     ignore_errors        = 1
     iterations           = 1
     label                = spec-stdlib
     log_line_width       = 4096
@@ -51,11 +54,12 @@
     CC                   = cc -O3
     CXX                  = {cxx} {compile_flags} {flags} {link_flags}
     CC_VERSION_OPTION    = --version
     CXX_VERSION_OPTION   = --version
     EXTRA_PORTABILITY    = -DSPEC_NO_CXX17_SPECIAL_MATH_FUNCTIONS # because libc++ doesn't implement the special math functions yet
-""")
+"""
+)
 
 # Build the list of benchmarks. We take all intrate and fprate benchmarks that contain C++ and
 # discard the ones that contain Fortran, since this test suite isn't set up to build Fortran code.
 spec_benchmarks = set()
 no_fortran = set()

Comment on lines +31 to +32
# Remove -Wall, -Wextra and -Werror from the flags since we don't care about enabling all warnings inside SPEC
compile_flags = compile_flags.replace('-Wall', '').replace('-Wextra', '').replace('-Werror', '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we ever care about the warnings, do we? In that case we might as well just pass -w.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants