Refactor WIDE_READ to allow finer control over high-performance function selection (#165613)

Sterling-Augustine · web-flow · commit 8701c2a9106b · 2025-12-04T10:21:53.000-08:00
[This is more of a straw-proposal than a ready-for-merging PR. I got
started thinking about what this might look like, and ended up just
implementing something as a proof-of-concept. Totally open to other
methods an ideas.]

As we implement more high-performance string-related functions, we have
found a need for better control over their selection than the big-hammer
LIBC_CONF_STRING_LENGTH_WIDE_READ. For example, I have a memchr
implementation coming, and unless I implement it in every variant, a
simple binary value doesn't work.

This PR makes gives finer-grained control over high-performance
functions than the generic LIBC_CONF_UNSAFE_WIDE_READ option. For any
function they like, the user can now select one of four implementations
at build time:

1. element, which reads byte-by-byte (or wchar by wchar)
2. wide, which reads by unsigned long
3. generic, which uses standard clang vector implemenations, if
available
4. arch, which uses an architecture-specific implemenation

(Reading the code carefully, you may note that a user can actually
specify any namespace they want, so we aren't technically limited to
those 4.)

We may also want to switch from command-line #defines as it is currently
done, to something more like
llvm-project/llvm/include/llvm/Config/llvm-config.h.cmake, and
#including the resulting file, which would move quite a bit of
complexity out of the command-line. But that's a future problem.
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -81,9 +81,8 @@ function(_get_compile_options_from_config output_var)
     list(APPEND config_options "-DLIBC_QSORT_IMPL=${LIBC_CONF_QSORT_IMPL}")
   endif()
 
-  if(LIBC_CONF_STRING_UNSAFE_WIDE_READ)
-    list(APPEND config_options "-DLIBC_COPT_STRING_UNSAFE_WIDE_READ")
-  endif()
+  list(APPEND config_options "-DLIBC_COPT_STRING_LENGTH_IMPL=${LIBC_CONF_STRING_LENGTH_IMPL}")
+  list(APPEND config_options "-DLIBC_COPT_FIND_FIRST_CHARACTER_IMPL=${LIBC_CONF_FIND_FIRST_CHARACTER_IMPL}")
 
   if(LIBC_CONF_MEMSET_X86_USE_SOFTWARE_PREFETCHING)
     list(APPEND config_options "-DLIBC_COPT_MEMSET_X86_USE_SOFTWARE_PREFETCHING")
diff --git a/libc/config/config.json b/libc/config/config.json
@@ -40,6 +40,7 @@
       "value": false,
       "doc": "Use an alternative printf float implementation based on 320-bit floats"
     },
+
     "LIBC_CONF_PRINTF_DISABLE_FIXED_POINT": {
       "value": false,
       "doc": "Disable printing fixed point values in printf and friends."
@@ -64,9 +65,13 @@
     }
   },
   "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."
+    "LIBC_CONF_STRING_LENGTH_IMPL": {
+      "value": "element",
+      "doc": "Selects the implementation for string-length: 'element', 'word', 'clang_vector', or 'arch_vector'."
+    },
+    "LIBC_CONF_FIND_FIRST_CHARACTER_IMPL": {
+      "value": "element",
+      "doc": "Selects the implementation for find-first-character-related functions: 'element', 'word', 'clang_vector', or 'arch_vector'."
     },
     "LIBC_CONF_MEMSET_X86_USE_SOFTWARE_PREFETCHING": {
       "value": false,
diff --git a/libc/config/linux/arm/config.json b/libc/config/linux/arm/config.json
@@ -1,7 +1,10 @@
 {
   "string": {
-    "LIBC_CONF_STRING_UNSAFE_WIDE_READ": {
-      "value": false
+    "LIBC_CONF_STRING_LENGTH_IMPL": {
+      "value": "element"
+    }
+    "LIBC_CONF_FIND_FIRST_CHARACTER_IMPL": {
+      "value": "element"
     }
   }
 }
diff --git a/libc/config/linux/config.json b/libc/config/linux/config.json
@@ -1,7 +1,10 @@
 {
   "string": {
-    "LIBC_CONF_STRING_UNSAFE_WIDE_READ": {
-      "value": true
+    "LIBC_CONF_STRING_LENGTH_IMPL": {
+      "value": "clang_vector",
+    },
+    "LIBC_CONF_FIND_FIRST_CHARACTER_IMPL": {
+      "value": "word",
     }
   }
 }
diff --git a/libc/config/linux/riscv/config.json b/libc/config/linux/riscv/config.json
@@ -1,7 +1,10 @@
 {
   "string": {
-    "LIBC_CONF_STRING_UNSAFE_WIDE_READ": {
-      "value": false
+    "LIBC_CONF_STRING_LENGTH_IMPL": {
+      "value": "element"
+    }
+    "LIBC_CONF_FIND_FIRST_CHARACTER_IMPL": {
+      "value": "element"
     }
   }
 }
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
@@ -58,8 +58,9 @@ to learn about the defaults for your platform and target.
 * **"setjmp" options**
     - ``LIBC_CONF_SETJMP_AARCH64_RESTORE_PLATFORM_REGISTER``: Make setjmp save the value of x18, and longjmp restore it. The AArch64 ABI delegates this register to platform ABIs, which can choose whether to make it caller-saved.
 * **"string" options**
+    - ``LIBC_CONF_FIND_FIRST_CHARACTER_IMPL``: Selects the implementation for find-first-character-related functions: 'element', 'word', 'clang_vector', or 'arch_vector'.
     - ``LIBC_CONF_MEMSET_X86_USE_SOFTWARE_PREFETCHING``: Inserts prefetch for write instructions (PREFETCHW) for memset on x86 to recover performance when hardware prefetcher is disabled.
-    - ``LIBC_CONF_STRING_UNSAFE_WIDE_READ``: Read more than a byte at a time to perform byte-string operations like strlen.
+    - ``LIBC_CONF_STRING_LENGTH_IMPL``: Selects the implementation for string-length: 'element', 'word', 'clang_vector', or 'arch_vector'.
 * **"threads" options**
     - ``LIBC_CONF_THREAD_MODE``: The implementation used for Mutex, acceptable values are LIBC_THREAD_MODE_PLATFORM, LIBC_THREAD_MODE_SINGLE, and LIBC_THREAD_MODE_EXTERNAL.
 * **"time" options**
diff --git a/libc/src/string/memory_utils/aarch64/inline_strlen.h b/libc/src/string/memory_utils/aarch64/inline_strlen.h
@@ -15,7 +15,7 @@
 #include <arm_neon.h>
 #include <stddef.h> // size_t
 namespace LIBC_NAMESPACE_DECL {
-namespace neon {
+namespace internal::neon {
 [[maybe_unused]] LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE static size_t
 string_length(const char *src) {
   using Vector __attribute__((may_alias)) = uint8x8_t;
@@ -43,15 +43,15 @@ string_length(const char *src) {
                                  (cpp::countr_zero(cmp) >> 3));
   }
 }
-} // namespace neon
+} // namespace internal::neon
 } // namespace LIBC_NAMESPACE_DECL
 #endif // __ARM_NEON
 
 #ifdef LIBC_TARGET_CPU_HAS_SVE
 #include "src/__support/macros/optimization.h"
 #include <arm_sve.h>
 namespace LIBC_NAMESPACE_DECL {
-namespace sve {
+namespace internal::sve {
 [[maybe_unused]] LIBC_INLINE static size_t string_length(const char *src) {
   const uint8_t *ptr = reinterpret_cast<const uint8_t *>(src);
   // Initialize the first-fault register to all true
@@ -92,15 +92,19 @@ namespace sve {
   len += svcntp_b8(all_true, before_zero);
   return len;
 }
-} // namespace sve
+} // namespace internal::sve
 } // namespace LIBC_NAMESPACE_DECL
 #endif // LIBC_TARGET_CPU_HAS_SVE
 
 namespace LIBC_NAMESPACE_DECL {
+namespace internal::arch_vector {
+[[maybe_unused]] LIBC_INLINE size_t string_length(const char *src) {
 #ifdef LIBC_TARGET_CPU_HAS_SVE
-namespace string_length_impl = sve;
+  return sve::string_length(src);
 #elif defined(__ARM_NEON)
-namespace string_length_impl = neon;
+  return neon::string_length(src);
 #endif
+}
+} // namespace internal::arch_vector
 } // namespace LIBC_NAMESPACE_DECL
 #endif // LLVM_LIBC_SRC_STRING_MEMORY_UTILS_AARCH64_INLINE_STRLEN_H
diff --git a/libc/src/string/memory_utils/generic/inline_strlen.h b/libc/src/string/memory_utils/generic/inline_strlen.h
@@ -14,7 +14,7 @@
 #include "src/__support/common.h"
 
 namespace LIBC_NAMESPACE_DECL {
-namespace internal {
+namespace clang_vector {
 
 // Exploit the underlying integer representation to do a variable shift.
 LIBC_INLINE constexpr cpp::simd_mask<char> shift_mask(cpp::simd_mask<char> m,
@@ -46,9 +46,8 @@ LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE size_t string_length(const char *src) {
              cpp::find_first_set(mask);
   }
 }
-} // namespace internal
+} // namespace clang_vector
 
-namespace string_length_impl = internal;
 } // namespace LIBC_NAMESPACE_DECL
 
 #endif // LLVM_LIBC_SRC_STRING_MEMORY_UTILS_GENERIC_INLINE_STRLEN_H
diff --git a/libc/src/string/memory_utils/x86_64/inline_strlen.h b/libc/src/string/memory_utils/x86_64/inline_strlen.h
@@ -15,7 +15,8 @@
 
 namespace LIBC_NAMESPACE_DECL {
 
-namespace string_length_internal {
+namespace internal::arch_vector {
+
 // Return a bit-mask with the nth bit set if the nth-byte in block_ptr is zero.
 template <typename Vector, typename Mask>
 LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE static Mask
@@ -92,15 +93,18 @@ namespace avx512 {
 }
 } // namespace avx512
 #endif
-} // namespace string_length_internal
 
+[[maybe_unused]] LIBC_INLINE size_t string_length(const char *src) {
 #if defined(__AVX512F__)
-namespace string_length_impl = string_length_internal::avx512;
+  return avx512::string_length(src);
 #elif defined(__AVX2__)
-namespace string_length_impl = string_length_internal::avx2;
+  return avx2::string_length(src);
 #else
-namespace string_length_impl = string_length_internal::sse2;
+  return sse2::string_length(src);
 #endif
+}
+
+} // namespace internal::arch_vector
 
 } // namespace LIBC_NAMESPACE_DECL
 
diff --git a/libc/src/string/string_length.h b/libc/src/string/string_length.h
diff --git a/libc/src/string/string_utils.h b/libc/src/string/string_utils.h
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
diff --git a/utils/bazel/llvm-project-overlay/libc/libc_configure_options.bzl b/utils/bazel/llvm-project-overlay/libc/libc_configure_options.bzl

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,10 @@`
`1`	`1`	`{`
`2`	`2`	`"string": {`
`3`		`- "LIBC_CONF_STRING_UNSAFE_WIDE_READ": {`
`4`		`- "value": false`
	`3`	`+ "LIBC_CONF_STRING_LENGTH_IMPL": {`
	`4`	`+ "value": "element"`
	`5`	`+ }`
	`6`	`+ "LIBC_CONF_FIND_FIRST_CHARACTER_IMPL": {`
	`7`	`+ "value": "element"`
`5`	`8`	`}`
`6`	`9`	`}`
`7`	`10`	`}`