[libc-commits] [libc] [libc] Use `${libc_opt_high_flag}` instead of `-O3` (PR #123233)

Petr Hosek via libc-commits libc-commits at lists.llvm.org
Thu Feb 6 01:09:43 PST 2025


https://github.com/petrhosek updated https://github.com/llvm/llvm-project/pull/123233

>From 150f6a58abbebf9ed4d5aa648406e67bea592322 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek at google.com>
Date: Thu, 16 Jan 2025 11:57:54 -0800
Subject: [PATCH 1/3] [libc] Use `${libc_opt_high_flag}` instead of `-O3`

This is preferable since `${libc_opt_high_flag}` will be set correctly
for the compiler used.
---
 libc/src/math/generic/CMakeLists.txt         |  2 +-
 libc/test/src/__support/CMakeLists.txt       |  2 +-
 libc/test/src/math/CMakeLists.txt            |  6 +++---
 libc/test/src/math/exhaustive/CMakeLists.txt |  2 +-
 libc/test/src/math/smoke/CMakeLists.txt      |  8 ++++----
 libc/test/src/stdfix/CMakeLists.txt          | 16 ++++++++--------
 libc/utils/MPFRWrapper/CMakeLists.txt        |  2 +-
 7 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/libc/src/math/generic/CMakeLists.txt b/libc/src/math/generic/CMakeLists.txt
index 0e57051807b3327..14e63d6cc139569 100644
--- a/libc/src/math/generic/CMakeLists.txt
+++ b/libc/src/math/generic/CMakeLists.txt
@@ -534,7 +534,7 @@ add_entrypoint_object(
     libc.src.__support.macros.optimization
     libc.src.__support.macros.properties.types
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_entrypoint_object(
diff --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index aeb8edf305d059a..8d175e857fcd119 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -234,7 +234,7 @@ add_libc_test(
     libc.src.stdlib.srand
     libc.src.string.memset
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   UNIT_TEST_ONLY
     # Aligned Allocation is not supported in hermetic builds.
 )
diff --git a/libc/test/src/math/CMakeLists.txt b/libc/test/src/math/CMakeLists.txt
index ae8518ee4b4cc18..5146b9624d4786b 100644
--- a/libc/test/src/math/CMakeLists.txt
+++ b/libc/test/src/math/CMakeLists.txt
@@ -1597,7 +1597,7 @@ add_fp_unittest(
     libc.src.math.sqrtf
     libc.src.__support.FPUtil.generic.sqrt
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_fp_unittest(
@@ -1613,7 +1613,7 @@ add_fp_unittest(
     libc.src.math.sqrt
     libc.src.__support.FPUtil.generic.sqrt
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_fp_unittest(
@@ -1629,7 +1629,7 @@ add_fp_unittest(
     libc.src.math.sqrtl
     libc.src.__support.FPUtil.generic.sqrt
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_fp_unittest(
diff --git a/libc/test/src/math/exhaustive/CMakeLists.txt b/libc/test/src/math/exhaustive/CMakeLists.txt
index 423c3b7a8bfd11b..b1927dbc19a3bdc 100644
--- a/libc/test/src/math/exhaustive/CMakeLists.txt
+++ b/libc/test/src/math/exhaustive/CMakeLists.txt
@@ -305,7 +305,7 @@ add_fp_unittest(
   SRCS
     hypotf_test.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     .exhaustive_test
     libc.src.math.hypotf
diff --git a/libc/test/src/math/smoke/CMakeLists.txt b/libc/test/src/math/smoke/CMakeLists.txt
index e23e7f41222d4ab..31f8ac40cc19fb2 100644
--- a/libc/test/src/math/smoke/CMakeLists.txt
+++ b/libc/test/src/math/smoke/CMakeLists.txt
@@ -2990,7 +2990,7 @@ add_fp_unittest(
   DEPENDS
     libc.src.__support.FPUtil.generic.sqrt
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_fp_unittest(
@@ -3004,7 +3004,7 @@ add_fp_unittest(
   DEPENDS
     libc.src.__support.FPUtil.generic.sqrt
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_fp_unittest(
@@ -3018,7 +3018,7 @@ add_fp_unittest(
   DEPENDS
     libc.src.__support.FPUtil.generic.sqrt
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_fp_unittest(
@@ -3035,7 +3035,7 @@ add_fp_unittest(
     libc.src.math.sqrtf128
     libc.src.__support.FPUtil.generic.sqrt
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_fp_unittest(
diff --git a/libc/test/src/stdfix/CMakeLists.txt b/libc/test/src/stdfix/CMakeLists.txt
index 60e38c9098c387f..1b3a1b965fc6c96 100644
--- a/libc/test/src/stdfix/CMakeLists.txt
+++ b/libc/test/src/stdfix/CMakeLists.txt
@@ -14,7 +14,7 @@ foreach(suffix IN ITEMS hr r lr hk k lk)
     SRCS
       abs${suffix}_test.cpp
     COMPILE_OPTIONS
-      -O3
+      ${libc_opt_high_flag}
     DEPENDS
       libc.src.stdfix.abs${suffix}
       libc.src.__support.fixed_point.fx_bits
@@ -31,7 +31,7 @@ foreach(suffix IN ITEMS uhr ur ulr uhk uk)
     SRCS
       sqrt${suffix}_test.cpp
     COMPILE_OPTIONS
-      -O3
+      ${libc_opt_high_flag}
     DEPENDS
       libc.src.stdfix.sqrt${suffix}
       libc.src.__support.CPP.bit
@@ -52,7 +52,7 @@ foreach(suffix IN ITEMS hr r lr hk k lk uhr ur ulr uhk uk ulk)
     SRCS
       round${suffix}_test.cpp
     COMPILE_OPTIONS
-      -O3
+      ${libc_opt_high_flag}
     DEPENDS
       libc.src.stdfix.round${suffix}
       libc.src.__support.fixed_point.fx_bits
@@ -67,7 +67,7 @@ foreach(suffix IN ITEMS hr r lr hk k lk uhr ur ulr uhk uk ulk)
     SRCS
       ${suffix}bits_test.cpp
     COMPILE_OPTIONS
-      -O3
+      ${libc_opt_high_flag}
     DEPENDS
       libc.src.stdfix.${suffix}bits
       libc.src.__support.CPP.bit
@@ -84,7 +84,7 @@ add_libc_test(
   SRCS
     uhksqrtus_test.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.stdfix.uhksqrtus
     libc.src.__support.CPP.bit
@@ -103,7 +103,7 @@ add_libc_test(
   SRCS
     uksqrtui_test.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.stdfix.uksqrtui
     libc.src.__support.CPP.bit
@@ -122,7 +122,7 @@ add_libc_test(
   SRCS
     exphk_test.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.stdfix.exphk
     libc.src.math.exp
@@ -140,7 +140,7 @@ add_libc_test(
   SRCS
     expk_test.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.stdfix.expk
     libc.src.math.exp
diff --git a/libc/utils/MPFRWrapper/CMakeLists.txt b/libc/utils/MPFRWrapper/CMakeLists.txt
index 0101c9f39908223..792ed5a9a2211af 100644
--- a/libc/utils/MPFRWrapper/CMakeLists.txt
+++ b/libc/utils/MPFRWrapper/CMakeLists.txt
@@ -7,7 +7,7 @@ if(LIBC_TESTS_CAN_USE_MPFR)
   _get_common_test_compile_options(compile_options "" "")
   # mpfr/gmp headers do not work with -ffreestanding flag.
   list(REMOVE_ITEM compile_options "-ffreestanding")
-  target_compile_options(libcMPFRWrapper PRIVATE -O3 ${compile_options})
+  target_compile_options(libcMPFRWrapper PRIVATE ${libc_opt_high_flag} ${compile_options})
   add_dependencies(
     libcMPFRWrapper
     libc.src.__support.CPP.array

>From 0ca085cb22c9873b870060b9a3a3f641f5a9dcb9 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek at google.com>
Date: Wed, 5 Feb 2025 08:27:14 +0000
Subject: [PATCH 2/3] Avoid applying compile options to the linker

Compile options aren't the same as link options and on some platforms
like Windows where the linker is invoked directly they're distinctly
different.
---
 libc/cmake/modules/LLVMLibCTestRules.cmake | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 4dbe5e046cc686f..ee8c59efdb2943e 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -272,7 +272,6 @@ function(create_libc_unittest fq_target_name)
   target_include_directories(${fq_build_target_name} SYSTEM PRIVATE ${LIBC_INCLUDE_DIR})
   target_include_directories(${fq_build_target_name} PRIVATE ${LIBC_SOURCE_DIR})
   target_compile_options(${fq_build_target_name} PRIVATE ${compile_options})
-  target_link_options(${fq_build_target_name} PRIVATE ${compile_options})
 
   if(NOT LIBC_UNITTEST_CXX_STANDARD)
     set(LIBC_UNITTEST_CXX_STANDARD ${CMAKE_CXX_STANDARD})

>From 5ea4d2adba1eee36d3717e79f60e20a9d2636897 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek at google.com>
Date: Thu, 6 Feb 2025 09:08:51 +0000
Subject: [PATCH 3/3] Pass common compile options to the linker

---
 libc/cmake/modules/LLVMLibCTestRules.cmake | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index ee8c59efdb2943e..c5e362c4657b8ca 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -218,6 +218,8 @@ function(create_libc_unittest fq_target_name)
 
   _get_common_test_compile_options(compile_options "${LIBC_UNITTEST_C_TEST}"
                                    "${LIBC_UNITTEST_FLAGS}")
+  # TODO: Perhaps we should should we have a separate function for link options.
+  set(link_options ${compile_options})
   list(APPEND compile_options ${LIBC_UNITTEST_COMPILE_OPTIONS})
 
   if(SHOW_INTERMEDIATE_OBJECTS)
@@ -272,6 +274,7 @@ function(create_libc_unittest fq_target_name)
   target_include_directories(${fq_build_target_name} SYSTEM PRIVATE ${LIBC_INCLUDE_DIR})
   target_include_directories(${fq_build_target_name} PRIVATE ${LIBC_SOURCE_DIR})
   target_compile_options(${fq_build_target_name} PRIVATE ${compile_options})
+  target_link_options(${fq_build_target_name} PRIVATE ${compile_options})
 
   if(NOT LIBC_UNITTEST_CXX_STANDARD)
     set(LIBC_UNITTEST_CXX_STANDARD ${CMAKE_CXX_STANDARD})



More information about the libc-commits mailing list