[libc] Use global preprocessor definitions for libc tuning

[gchatelet]

No description provided.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

---

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

1 Files Affected:

• (modified) libc/src/string/CMakeLists.txt (+2-6)

diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt
index 6daaf1998ea7bc2..dd1bdffa9d0a37f 100644
--- a/libc/src/string/CMakeLists.txt
+++ b/libc/src/string/CMakeLists.txt
@@ -1,13 +1,10 @@
 add_subdirectory(memory_utils)
 
 if(LIBC_CONF_STRING_UNSAFE_WIDE_READ)
-  list(APPEND string_config_options "-DLIBC_COPT_STRING_UNSAFE_WIDE_READ")
+  add_compile_definitions("-DLIBC_COPT_STRING_UNSAFE_WIDE_READ")
 endif()
 if(LIBC_CONF_MEMSET_X86_USE_SOFTWARE_PREFETCHING)
-  list(APPEND string_config_options "-DLIBC_COPT_MEMSET_X86_USE_SOFTWARE_PREFETCHING")
-endif()
-if(string_config_options)
-  list(PREPEND string_config_options "COMPILE_OPTIONS")
+  add_compile_definitions("-DLIBC_COPT_MEMSET_X86_USE_SOFTWARE_PREFETCHING")
 endif()
 
 add_header_library(
@@ -20,7 +17,6 @@ add_header_library(
     libc.include.stdlib
     libc.src.__support.common
     libc.src.__support.CPP.bitset
-  ${string_config_options}
 )
 
 add_header_library(

[gchatelet]

One caveat is that the configuration is global and leaks in the tests.

For instance, setting the flag in a test instance is useless if the configuration for the build bot already has the option set to true. Conversely, we cannot really test disabling the option (TBH I'm not sure how add_compile_definitions takes precedence over options passed through add_entrypoint_object::COMPILE_OPTIONS).

[michaelrj-google]

I'm not necessarily against setting flags globally, but we should do it in a way that means we don't need extra cmake logic for each flag. We should see about creating a list of config options and setting them as compile options directly.

[gchatelet]

I'm not necessarily against setting flags globally, but we should do it in a way that means we don't need extra cmake logic for each flag. We should see about creating a list of config options and setting them as compile options directly.

Good point.

We discussed option exploration with @lntue a few days ago. I think it would be profitable to have a consistent way to explore all flag combinations in a principled way for testing purposes. Let's create an RFC to discuss the details.