[libc-commits] [libc] remove common libc tuners (PR #66136)

Siva Chandra via libc-commits libc-commits at lists.llvm.org
Wed Sep 13 09:09:38 PDT 2023


https://github.com/sivachandra updated https://github.com/llvm/llvm-project/pull/66136:

>From d1a9fda20a2be37385b44dcbf2f73b1890fa7d78 Mon Sep 17 00:00:00 2001
From: Siva Chandra Reddy <sivachandra at google.com>
Date: Tue, 12 Sep 2023 05:42:37 +0000
Subject: [PATCH 1/3] [libc][NFC] Make entrypoint alias targets real library
 targets.

This is part of a libc wide CMake cleanup which aims to eliminate
certain explicitly duplicated logic which is available in CMake-3.20.
This change in particular makes the entrypoint aliases real library
targets so that they can be treated as normal library targets by other
libc build rules.
---
 libc/cmake/modules/LLVMLibCObjectRules.cmake | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index 709acd9ad614fbe..006b8949b6fd0a1 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -559,6 +559,8 @@ function(create_entrypoint_object fq_target_name)
     return()
   endif()
 
+  set(internal_target_name ${fq_target_name}.__internal__)
+
   if(ADD_ENTRYPOINT_OBJ_ALIAS)
     # Alias targets help one add aliases to other entrypoint object targets.
     # One can use alias targets setup OS/machine independent entrypoint targets.
@@ -586,10 +588,23 @@ function(create_entrypoint_object fq_target_name)
       message(FATAL_ERROR "The aliasee of an entrypoint alias should be an entrypoint.")
     endif()
 
-    add_custom_target(${fq_target_name})
-    add_dependencies(${fq_target_name} ${fq_dep_name})
+    # add_custom_target(${fq_target_name})
     get_target_property(object_file ${fq_dep_name} "OBJECT_FILE")
     get_target_property(object_file_raw ${fq_dep_name} "OBJECT_FILE_RAW")
+    add_library(
+      ${internal_target_name}
+      EXCLUDE_FROM_ALL
+      OBJECT
+      ${object_file_raw}
+    )
+    add_dependencies(${internal_target_name} ${fq_dep_name})
+    add_library(
+      ${fq_target_name}
+      EXCLUDE_FROM_ALL
+      OBJECT
+      ${object_file}
+    )
+    add_dependencies(${fq_target_name} ${fq_dep_name} ${internal_target_name})
     set_target_properties(
       ${fq_target_name}
       PROPERTIES
@@ -619,7 +634,6 @@ function(create_entrypoint_object fq_target_name)
     "${ADD_ENTRYPOINT_OBJ_FLAGS}"
     ${ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS}
   )
-  set(internal_target_name ${fq_target_name}.__internal__)
   set(include_dirs ${LIBC_SOURCE_DIR} ${LIBC_INCLUDE_DIR})
   get_fq_deps_list(fq_deps_list ${ADD_ENTRYPOINT_OBJ_DEPENDS})
   set(full_deps_list ${fq_deps_list} libc.src.__support.common)

>From f0475c5918f4f645e45f05f8e07f472e9fd94559 Mon Sep 17 00:00:00 2001
From: Siva Chandra Reddy <sivachandra at google.com>
Date: Tue, 12 Sep 2023 16:30:50 +0000
Subject: [PATCH 2/3] [libc][NFC] Remove stray comment.

---
 libc/cmake/modules/LLVMLibCObjectRules.cmake | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index 006b8949b6fd0a1..4d5835cc6b4d809 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -588,7 +588,6 @@ function(create_entrypoint_object fq_target_name)
       message(FATAL_ERROR "The aliasee of an entrypoint alias should be an entrypoint.")
     endif()
 
-    # add_custom_target(${fq_target_name})
     get_target_property(object_file ${fq_dep_name} "OBJECT_FILE")
     get_target_property(object_file_raw ${fq_dep_name} "OBJECT_FILE_RAW")
     add_library(

>From 1eeb5e0eda3bdcf6d45555eec6e1afac5c6c3f45 Mon Sep 17 00:00:00 2001
From: Siva Chandra Reddy <sivachandra at google.com>
Date: Mon, 11 Sep 2023 22:17:32 +0000
Subject: [PATCH 3/3] [libc] Remove common_libc_tuners.cmake and move options
 into config.json.

The name has been changed to adhere to the config option naming format.
The necessary build changes to use the new option have also been made.
---
 libc/CMakeLists.txt                          |  2 --
 libc/cmake/modules/LLVMLibCObjectRules.cmake |  2 ++
 libc/common_libc_tuners.cmake                | 14 --------------
 libc/config/config.json                      |  6 ++++++
 libc/docs/configure.rst                      |  2 ++
 libc/src/string/CMakeLists.txt               |  9 +++++++++
 libc/src/string/string_utils.h               |  4 ++--
 7 files changed, 21 insertions(+), 18 deletions(-)
 delete mode 100644 libc/common_libc_tuners.cmake

diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 12fff25c197e2d9..cf90919c20fd969 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -64,8 +64,6 @@ add_compile_definitions(LIBC_NAMESPACE=${LIBC_NAMESPACE})
 # Flags to pass down to the compiler while building the libc functions.
 set(LIBC_COMPILE_OPTIONS_DEFAULT "" CACHE STRING "Architecture to tell clang to optimize for (e.g. -march=... or -mcpu=...)")
 
-include(common_libc_tuners.cmake)
-
 list(APPEND LIBC_COMPILE_OPTIONS_DEFAULT ${LIBC_COMMON_TUNE_OPTIONS})
 
 # Check --print-resource-dir to find the compiler resource dir if this flag
diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index 4d5835cc6b4d809..891c298855e6131 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -672,6 +672,7 @@ function(create_entrypoint_object fq_target_name)
     target_compile_options(${internal_target_name} BEFORE PRIVATE ${common_compile_options})
     target_include_directories(${internal_target_name} PRIVATE ${include_dirs})
     add_dependencies(${internal_target_name} ${full_deps_list})
+    target_link_libraries(${internal_target_name} ${full_deps_list})
 
     add_library(
       ${fq_target_name}
@@ -685,6 +686,7 @@ function(create_entrypoint_object fq_target_name)
     target_compile_options(${fq_target_name} BEFORE PRIVATE ${common_compile_options} -DLIBC_COPT_PUBLIC_PACKAGING)
     target_include_directories(${fq_target_name} PRIVATE ${include_dirs})
     add_dependencies(${fq_target_name} ${full_deps_list})
+    target_link_libraries(${fq_target_name} ${full_deps_list})
   endif()
 
   set_target_properties(
diff --git a/libc/common_libc_tuners.cmake b/libc/common_libc_tuners.cmake
deleted file mode 100644
index 20aee4899623490..000000000000000
--- a/libc/common_libc_tuners.cmake
+++ /dev/null
@@ -1,14 +0,0 @@
-# ------------------------------------------------------------------------------
-# Common tuning option definitions.
-# ------------------------------------------------------------------------------
-
-set(LIBC_COMMON_TUNE_OPTIONS "")
-
-option(LIBC_UNSAFE_STRING_WIDE_READ "Functions searching for the first character in a string such as strlen will read the string as int sized blocks instead of bytes. This relies on undefined behavior and may fail on some systems, but improves performance on long strings." OFF)
-if(LIBC_UNSAFE_STRING_WIDE_READ)
-  if(LLVM_USE_SANITIZER)
-    message(FATAL_ERROR "LIBC_UNSAFE_STRING_WIDE_READ is set at the same time as a sanitizer. LIBC_UNSAFE_STRING_WIDE_READ causes strlen and memchr to read beyond the end of their target strings, which is undefined behavior caught by sanitizers.")
-  else()
-    list(APPEND LIBC_COMMON_TUNE_OPTIONS "-DLIBC_COPT_UNSAFE_STRING_WIDE_READ")
-    endif()
-endif()
diff --git a/libc/config/config.json b/libc/config/config.json
index cd68b81028bff7f..4ec096c36103ec4 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -12,5 +12,11 @@
       "value": false,
       "doc": "Disable handling of %n in printf format string."
     }
+  },
+  "string": {
+    "LIBC_CONF_STRING_UNSAFE_WIDE_READ": {
+      "value": false,
+      "doc": "Read more than a byte at a time to perform byte-string operations like strlen."
+    }
   }
 }
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 9b4d6034129351d..e8a602896e333a6 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -29,3 +29,5 @@ to learn about the defaults for your platform and target.
     - ``LIBC_CONF_PRINTF_DISABLE_FLOAT``: Disable printing floating point values in printf and friends.
     - ``LIBC_CONF_PRINTF_DISABLE_INDEX_MODE``: Disable index mode in the printf format string.
     - ``LIBC_CONF_PRINTF_DISABLE_WRITE_INT``: Disable handling of %n in printf format string.
+* **"string" options**
+    - ``LIBC_CONF_STRING_UNSAFE_WIDE_READ``: Read more than a byte at a time to perform byte-string operations like strlen.
diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt
index b073a62f957a734..77824b1f0e5408e 100644
--- a/libc/src/string/CMakeLists.txt
+++ b/libc/src/string/CMakeLists.txt
@@ -1,5 +1,13 @@
 add_subdirectory(memory_utils)
 
+if(LIBC_CONF_STRING_UNSAFE_WIDE_READ)
+  message(">>> Adding -DLIBC_COPT_STRING_UNSAFE_WIDE_READ")
+  list(APPEND string_config_options "-DLIBC_COPT_STRING_UNSAFE_WIDE_READ")
+endif()
+if(string_config_options)
+  list(PREPEND string_config_options "COMPILE_OPTIONS")
+endif()
+
 add_header_library(
   string_utils
   HDRS
@@ -10,6 +18,7 @@ add_header_library(
     libc.include.stdlib
     libc.src.__support.common
     libc.src.__support.CPP.bitset
+  ${string_config_options}
 )
 
 add_header_library(
diff --git a/libc/src/string/string_utils.h b/libc/src/string/string_utils.h
index e2e7722e392a7fd..87dc4c6cd2b4c58 100644
--- a/libc/src/string/string_utils.h
+++ b/libc/src/string/string_utils.h
@@ -88,7 +88,7 @@ LIBC_INLINE size_t string_length_byte_read(const char *src) {
 // Returns the length of a string, denoted by the first occurrence
 // of a null terminator.
 LIBC_INLINE size_t string_length(const char *src) {
-#ifdef LIBC_COPT_UNSAFE_STRING_WIDE_READ
+#ifdef LIBC_COPT_STRING_UNSAFE_WIDE_READ
   // Unsigned int is the default size for most processors, and on x86-64 it
   // performs better than larger sizes when the src pointer can't be assumed to
   // be aligned to a word boundary, so it's the size we use for reading the
@@ -143,7 +143,7 @@ LIBC_INLINE void *find_first_character_byte_read(const unsigned char *src,
 // 'src'. If 'ch' is not found, returns nullptr.
 LIBC_INLINE void *find_first_character(const unsigned char *src,
                                        unsigned char ch, size_t max_strlen) {
-#ifdef LIBC_COPT_UNSAFE_STRING_WIDE_READ
+#ifdef LIBC_COPT_STRING_UNSAFE_WIDE_READ
   // If the maximum size of the string is small, the overhead of aligning to a
   // word boundary and generating a bitmask of the appropriate size may be
   // greater than the gains from reading larger chunks. Based on some testing,



More information about the libc-commits mailing list