Reland Refactor WIDE_READ to allow finer control over high-performance function selection (#165613) #170738
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e function selection (llvm#165613) [Previous commit had an incorrect default case when FIND_FIRST_CHARACTER_WIDE_READ_IMPL was not specified in config.json.] 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 complexity out of the command-line. But that's a future problem.
Sterling-Augustine
requested review from
aaronmondal,
keith and
rupprecht
as code owners
Sterling-Augustine
added
the
skip-precommit-approval
llvmbot
added
backend:RISC-V
libc
bazel
Member
|
@llvm/pr-subscribers-libc Author: None (Sterling-Augustine) Changes[Previous commit had an incorrect default case when FIND_FIRST_CHARACTER_WIDE_READ_IMPL was not specified in config.json. This PR is identical to that one with one line fixed.] 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:
(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 Patch is 25.78 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170738.diff 13 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 4e9a9b66a63a7..f4e2a62d14b31 100644
--- 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
index a7844e4fe2dd1..f0ab3b9cce2e9 100644
--- 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
index e7ad4544b104d..caa16744d389f 100644
--- 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
index 30e8b2cdadabe..8e7db248dc1bd 100644
--- 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
index e7ad4544b104d..caa16744d389f 100644
--- 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
index 362e293a4b714..43d3c0ec06d3b 100644
--- 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
index eafaca9776a42..87f6cb8cf9bd5 100644
--- 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,7 +43,7 @@ string_length(const char *src) {
(cpp::countr_zero(cmp) >> 3));
}
}
-} // namespace neon
+} // namespace internal::neon
} // namespace LIBC_NAMESPACE_DECL
#endif // __ARM_NEON
@@ -51,7 +51,7 @@ string_length(const char *src) {
#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
index 69700e801bcea..7a565b36617ed 100644
--- 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
index 9e10d58363393..07b4a470f0d77 100644
--- 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
new file mode 100644
index 0000000000000..c828c85c16a17
--- /dev/null
+++ b/libc/src/string/string_length.h
@@ -0,0 +1,213 @@
+//===-- String Length -------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Basic implementation and dispatch mechanism for performance-sensitive string-
+// related code.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STRING_STRING_LENGTH_H
+#define LLVM_LIBC_SRC_STRING_STRING_LENGTH_H
+
+#include "hdr/limits_macros.h"
+#include "hdr/stdint_proxy.h" // uintptr_t
+#include "hdr/types/size_t.h"
+#include "src/__support/CPP/type_traits.h" // cpp::is_same_v
+
+#if LIBC_HAS_VECTOR_TYPE
+#include "src/string/memory_utils/generic/inline_strlen.h"
+#endif
+#if defined(LIBC_TARGET_ARCH_IS_X86)
+#include "src/string/memory_utils/x86_64/inline_strlen.h"
+#elif defined(LIBC_TARGET_ARCH_IS_AARCH64)
+#include "src/string/memory_utils/aarch64/inline_strlen.h"
+#endif
+
+// Set sensible defaults
+#ifndef LIBC_COPT_STRING_LENGTH_IMPL
+#define LIBC_COPT_STRING_LENGTH_IMPL element
+#endif
+#ifndef LIBC_COPT_FIND_FIRST_CHARACTER_IMPL
+#define LIBC_COPT_FIND_FIRST_CHARACTER_IMPL element
+#endif
+
+namespace LIBC_NAMESPACE_DECL {
+namespace internal {
+
+#if !LIBC_HAS_VECTOR_TYPE
+// Forward any clang vector impls to architecture specific ones
+namespace arch_vector {}
+namespace clang_vector = arch_vector;
+#endif
+
+namespace element {
+// Element-by-element (usually a byte, but wider for wchar) implementations of
+// functions that search for data. Slow, but easy to understand and analyze.
+
+// Returns the length of a string, denoted by the first occurrence
+// of a null terminator.
+LIBC_INLINE size_t string_length(const char *src) {
+ size_t length;
+ for (length = 0; *src; ++src, ++length)
+ ;
+ return length;
+}
+
+template <typename T> LIBC_INLINE size_t string_length_element(const T *src) {
+ size_t length;
+ for (length = 0; *src; ++src, ++length)
+ ;
+ return length;
+}
+
+LIBC_INLINE void *find_first_character(const unsigned char *src,
+ unsigned char ch, size_t n) {
+ for (; n && *src != ch; --n, ++src)
+ ;
+ return n ? const_cast<unsigned char *>(src) : nullptr;
+}
+} // namespace element
+
+namespace word {
+// Non-vector, implementations of functions that search for data by reading from
+// memory word-by-word.
+
+template <typename Word> LIBC_INLINE constexpr Word repeat_byte(Word byte) {
+ static_assert(CHAR_BIT == 8, "repeat_byte assumes a byte is 8 bits.");
+ constexpr size_t BITS_IN_BYTE = CHAR_BIT;
+ constexpr size_t BYTE_MASK = 0xff;
+ Word result = 0;
+ byte = byte & BYTE_MASK;
+ for (size_t i = 0; i < sizeof(Word); ++i)
+ result = (result << BITS_IN_BYTE) | byte;
+ return result;
+}
+
+// The goal of this function is to take in a block of arbitrary size and return
+// if it has any bytes equal to zero without branching. This is done by
+// transforming the block such that zero bytes become non-zero and non-zero
+// bytes become zero.
+// The first transformation relies on the properties of carrying in arithmetic
+// subtraction. Specifically, if 0x01 is subtracted from a byte that is 0x00,
+// then the result for that byte must be equal to 0xff (or 0xfe if the next byte
+// needs a carry as well).
+// The next transformation is a simple mask. All zero bytes will have the high
+// bit set after the subtraction, so each byte is masked with 0x80. This narrows
+// the set of bytes that result in a non-zero value to only zero bytes and bytes
+// with the high bit and any other bit set.
+// The final transformation masks the result of the previous transformations
+// with the inverse of the original byte. This means that any byte that had the
+// high bit set will no longer have it set, narrowing the list of bytes which
+// result in non-zero values to just the zero byte.
+template <typename Word> LIBC_INLINE constexpr bool has_zeroes(Word block) {
+ constexpr unsigned int LOW_BITS = repeat_byte<Word>(0x01);
+ constexpr Word HIGH_BITS = repeat_byte<Word>(0x80);
+ Word subtracted = block - LOW_BITS;
+ Word inverted = ~block;
+ return (subtracted & inverted & HIGH_BITS) != 0;
+}
+
+// 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
+// string a block at a time.
+
+LIBC_INLINE size_t string_length(const char *src) {
+ using Word = unsigned int;
+ const char *char_ptr = src;
+ // Step 1: read 1 byte at a time to align to block size
+ for (; reinterpret_cast<uintptr_t>(char_ptr) % sizeof(Word) != 0;
+ ++char_ptr) {
+ if (*char_ptr == '\0')
+ return static_cast<size_t>(char_ptr - src);
+ }
+ // Step 2: read blocks
+ for (const Word *block_ptr = reinterpret_cast<const Word *>(char_ptr);
+ !has_zeroes<Word>(*block_ptr); ++block_ptr) {
+ char_ptr = reinterpret_cast<const char *>(block_ptr);
+ }
+ // Step 3: find the zero in the block
+ for (; *char_ptr != '\0'; ++char_ptr) {
+ ;
+ }
+ return static_cast<size_t>(char_ptr - src);
+}
+
+LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE void *
+find_first_character(const unsigned char *src, unsigned char ch,
+ size_t max_strlen = cpp::numeric_limits<size_t>::max()) {
+ using Word = unsigned int;
+ const unsigned char *char_ptr = src;
+ size_t cur = 0;
+
+ // 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,
+ // the crossover point between when it's faster to just read bytewise and read
+ // blocks is somewhere between 16 and 32, so 4 times the size of the block
+ // should be in that range.
+ if (max_strlen < (sizeof(Word) * 4)) {
+ return element::find_first_character(src, ch, max_strlen);
+ }
+ size_t n = max_strlen;
+ // Step 1: read 1 byte at a time to align to block size
+ for (; cur < n && reinterpret_cast<uintptr_t>(char_ptr) % sizeof(Word) != 0;
+ ++cur, ++char_ptr) {
+ if (*char_ptr == ch)
+ return const_cast<unsigned char *>(char_ptr);
+ }
+
+ const Word ch_mask = repeat_byte<Word>(ch);
+
+ // Step 2: read blocks
+ const Word *block_ptr = reinterpret_cast<const Word *>(char_ptr);
+ for (; cur < n && !has_zeroes<Word>((*block_ptr) ^ ch_mask);
+ cur += sizeof(Word), ++block_ptr)
+ ;
+ char_ptr = reinterpret_cast<const unsigned char *>(block_ptr);
+
+ // Step 3: find the match in the block
+ for (; cur < n && *char_ptr != ch; ++cur, ++char_ptr) {
+ ;
+ }
+
+ if (cur >= n || *char_ptr != ch)
+ return static_cast<void *>(nullptr);
+
+ return const_cast<unsigned char *>(char_ptr);
+}
+
+} // namespace word
+
+// Dispatch mechanism for implementations of performance-sensitive
+// functions. Always measure, but generally from lower- to higher-performance
+// order:
+//
+// 1. element - read char-by-char or wchar-by-wchar
+// 3. word - read word-by-word
+// 3. clang_vector - read using clang's internal vector types
+// 4. arch_vector - hand-coded per architecture. Possibly in asm, or with
+// intrinsics.
+//
+// The called implemenation is chosen at build-time by setting
+// LIBC_CONF_{FUNC}_IMPL in config.json
+static constexpr auto &string_length_impl =
+ LIBC_COPT_STRING_LENGTH_IMPL::string_length;
+static constexpr auto &find_first_character_impl =
+ LIBC_COPT_FIND_FIRST_CHARACTER_IMPL::find_first_character;
+
+template <typename T> LIBC_INLINE size_t string_length(const T *src) {
+ if constexpr (cpp::is_same_v<T, char>)
+ return string_length_impl(src);
+ return element::string_length_element<T>(src);
+}
+
+} // namespace internal
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STRING_STRING_LENGTH_H
diff --git a/libc/src/string/string_utils.h b/libc/src/string/string_utils.h
index cbce62ead0328..b0144e01a9006 100644
--- a/libc/src/string/string_utils.h
+++ b/libc/src/string/string_utils.h
@@ -14,172 +14,17 @@
#ifndef LLVM_LIBC_SRC_STRING_STRING_UTILS_H
#define LLVM_LIBC_SRC_STRING_STRING_UTILS_H
-#include "hdr/limits_macros.h"
-#include "hdr/stdint_proxy.h" // uintptr_t
#include "hdr/types/size_t.h"
#include "src/__support/CPP/bitset.h"
-#include "src/__support/CPP/type_traits.h" // cpp::is_same_v
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
#include "src/string/memory_utils/inline_memcpy.h"
-
-#if defined(LIBC_COPT_STRING_UNSAFE_WIDE_READ)
-#if LIBC_HAS_VECTOR_TYPE
-#include "src/string/memory_utils/generic/inline_strlen.h"
-#elif defined(LIBC_TARGET_ARCH_IS_X86)
-#include "src/string/memory_utils/x86_64/inline_strlen.h"
-#elif defined(LIBC_TARGET_ARCH_IS_AARCH64) && defined(__ARM_NEON)
-#include "src/string/memory_utils/aarch64/inline_strlen.h"
-#else
-namespace string_length_impl = LIBC_NAMESPACE::wide_read;
-#endif
-#endif // defined(LIBC_COPT_STRING_UNSAFE_WIDE_READ)
+#include "src/string/string_length.h"
namespace LIBC_NAMESPACE_DECL {
namespace internal {
-template <typename Word> LIBC_INLINE constexpr Word repeat_byte(Word byte) {
- static_assert(CHAR_BIT == 8, "repeat_byte assumes a byte is 8 bits.");
- constexpr size_t BITS_IN_BYTE = CHAR_BIT;
- constexpr size_t BYTE_MASK = 0xff;
- Word result = 0;
- byte = byte & BYTE_MASK;
- for (size_t i = 0; i < sizeof(Word); ++i)
- result = (result << BITS_IN_BYTE) | byte;
- return result;
-}
-
-// The goal of this function is to take in a block of arbitrary size and return
-// if it has any bytes equal to zero without branching. This is done by
-// transforming the block such that zero bytes become non-zero and non-zero
-// bytes become zero.
-// The first transformation relies on the properties of carrying in arithmetic
-// subtraction. Specifically, if 0x01 is subtracted from a byte that is 0x00,
-// then the result for that byte must be equal to 0xff (or 0xfe if the next byte
-// needs a carry as well).
-// The next transformation is a simple mask. All zero bytes will have the high
-// bit set after the subtraction, so each byte is masked with 0x80. This narrows
-// the set of bytes that result in a non-zero value to only zero bytes and bytes
-// with the high bit and any other bit set.
-// The final transformation masks the result of the previous transformations
-// with the inverse of the original byte. This means that any byte that had the
-// high bit set will no longer have it set, narrowing the list of bytes which
-// result in non-zero values to just the zero byte.
-template <typename Word> LIBC_INLINE constexpr bool has_zeroes(Word block) {
- constexpr unsigned int LOW_BITS = repeat_byte<Word>(0x01);
- constexpr Word HIGH_BITS = repeat_byte<Word>(0x80);
- Word subtracted = block - LOW_BITS;
- Word inverted = ~block;
- return (subtracted & inverted & HIGH_BITS) != 0;
-}
-
-template <typename Word>
-LIBC_INLINE size_t string_length_wide_read(const char *src) {
- const char *char_ptr = src;
- // Step 1: read 1 byte at a t...
[truncated]
|
Member
|
@llvm/pr-subscribers-backend-risc-v Author: None (Sterling-Augustine) Changes[Previous commit had an incorrect default case when FIND_FIRST_CHARACTER_WIDE_READ_IMPL was not specified in config.json. This PR is identical to that one with one line fixed.] 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:
(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 Patch is 25.78 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170738.diff 13 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 4e9a9b66a63a7..f4e2a62d14b31 100644
--- 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
index a7844e4fe2dd1..f0ab3b9cce2e9 100644
--- 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
index e7ad4544b104d..caa16744d389f 100644
--- 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
index 30e8b2cdadabe..8e7db248dc1bd 100644
--- 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
index e7ad4544b104d..caa16744d389f 100644
--- 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
index 362e293a4b714..43d3c0ec06d3b 100644
--- 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
index eafaca9776a42..87f6cb8cf9bd5 100644
--- 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,7 +43,7 @@ string_length(const char *src) {
(cpp::countr_zero(cmp) >> 3));
}
}
-} // namespace neon
+} // namespace internal::neon
} // namespace LIBC_NAMESPACE_DECL
#endif // __ARM_NEON
@@ -51,7 +51,7 @@ string_length(const char *src) {
#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
index 69700e801bcea..7a565b36617ed 100644
--- 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
index 9e10d58363393..07b4a470f0d77 100644
--- 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
new file mode 100644
index 0000000000000..c828c85c16a17
--- /dev/null
+++ b/libc/src/string/string_length.h
@@ -0,0 +1,213 @@
+//===-- String Length -------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Basic implementation and dispatch mechanism for performance-sensitive string-
+// related code.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STRING_STRING_LENGTH_H
+#define LLVM_LIBC_SRC_STRING_STRING_LENGTH_H
+
+#include "hdr/limits_macros.h"
+#include "hdr/stdint_proxy.h" // uintptr_t
+#include "hdr/types/size_t.h"
+#include "src/__support/CPP/type_traits.h" // cpp::is_same_v
+
+#if LIBC_HAS_VECTOR_TYPE
+#include "src/string/memory_utils/generic/inline_strlen.h"
+#endif
+#if defined(LIBC_TARGET_ARCH_IS_X86)
+#include "src/string/memory_utils/x86_64/inline_strlen.h"
+#elif defined(LIBC_TARGET_ARCH_IS_AARCH64)
+#include "src/string/memory_utils/aarch64/inline_strlen.h"
+#endif
+
+// Set sensible defaults
+#ifndef LIBC_COPT_STRING_LENGTH_IMPL
+#define LIBC_COPT_STRING_LENGTH_IMPL element
+#endif
+#ifndef LIBC_COPT_FIND_FIRST_CHARACTER_IMPL
+#define LIBC_COPT_FIND_FIRST_CHARACTER_IMPL element
+#endif
+
+namespace LIBC_NAMESPACE_DECL {
+namespace internal {
+
+#if !LIBC_HAS_VECTOR_TYPE
+// Forward any clang vector impls to architecture specific ones
+namespace arch_vector {}
+namespace clang_vector = arch_vector;
+#endif
+
+namespace element {
+// Element-by-element (usually a byte, but wider for wchar) implementations of
+// functions that search for data. Slow, but easy to understand and analyze.
+
+// Returns the length of a string, denoted by the first occurrence
+// of a null terminator.
+LIBC_INLINE size_t string_length(const char *src) {
+ size_t length;
+ for (length = 0; *src; ++src, ++length)
+ ;
+ return length;
+}
+
+template <typename T> LIBC_INLINE size_t string_length_element(const T *src) {
+ size_t length;
+ for (length = 0; *src; ++src, ++length)
+ ;
+ return length;
+}
+
+LIBC_INLINE void *find_first_character(const unsigned char *src,
+ unsigned char ch, size_t n) {
+ for (; n && *src != ch; --n, ++src)
+ ;
+ return n ? const_cast<unsigned char *>(src) : nullptr;
+}
+} // namespace element
+
+namespace word {
+// Non-vector, implementations of functions that search for data by reading from
+// memory word-by-word.
+
+template <typename Word> LIBC_INLINE constexpr Word repeat_byte(Word byte) {
+ static_assert(CHAR_BIT == 8, "repeat_byte assumes a byte is 8 bits.");
+ constexpr size_t BITS_IN_BYTE = CHAR_BIT;
+ constexpr size_t BYTE_MASK = 0xff;
+ Word result = 0;
+ byte = byte & BYTE_MASK;
+ for (size_t i = 0; i < sizeof(Word); ++i)
+ result = (result << BITS_IN_BYTE) | byte;
+ return result;
+}
+
+// The goal of this function is to take in a block of arbitrary size and return
+// if it has any bytes equal to zero without branching. This is done by
+// transforming the block such that zero bytes become non-zero and non-zero
+// bytes become zero.
+// The first transformation relies on the properties of carrying in arithmetic
+// subtraction. Specifically, if 0x01 is subtracted from a byte that is 0x00,
+// then the result for that byte must be equal to 0xff (or 0xfe if the next byte
+// needs a carry as well).
+// The next transformation is a simple mask. All zero bytes will have the high
+// bit set after the subtraction, so each byte is masked with 0x80. This narrows
+// the set of bytes that result in a non-zero value to only zero bytes and bytes
+// with the high bit and any other bit set.
+// The final transformation masks the result of the previous transformations
+// with the inverse of the original byte. This means that any byte that had the
+// high bit set will no longer have it set, narrowing the list of bytes which
+// result in non-zero values to just the zero byte.
+template <typename Word> LIBC_INLINE constexpr bool has_zeroes(Word block) {
+ constexpr unsigned int LOW_BITS = repeat_byte<Word>(0x01);
+ constexpr Word HIGH_BITS = repeat_byte<Word>(0x80);
+ Word subtracted = block - LOW_BITS;
+ Word inverted = ~block;
+ return (subtracted & inverted & HIGH_BITS) != 0;
+}
+
+// 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
+// string a block at a time.
+
+LIBC_INLINE size_t string_length(const char *src) {
+ using Word = unsigned int;
+ const char *char_ptr = src;
+ // Step 1: read 1 byte at a time to align to block size
+ for (; reinterpret_cast<uintptr_t>(char_ptr) % sizeof(Word) != 0;
+ ++char_ptr) {
+ if (*char_ptr == '\0')
+ return static_cast<size_t>(char_ptr - src);
+ }
+ // Step 2: read blocks
+ for (const Word *block_ptr = reinterpret_cast<const Word *>(char_ptr);
+ !has_zeroes<Word>(*block_ptr); ++block_ptr) {
+ char_ptr = reinterpret_cast<const char *>(block_ptr);
+ }
+ // Step 3: find the zero in the block
+ for (; *char_ptr != '\0'; ++char_ptr) {
+ ;
+ }
+ return static_cast<size_t>(char_ptr - src);
+}
+
+LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE void *
+find_first_character(const unsigned char *src, unsigned char ch,
+ size_t max_strlen = cpp::numeric_limits<size_t>::max()) {
+ using Word = unsigned int;
+ const unsigned char *char_ptr = src;
+ size_t cur = 0;
+
+ // 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,
+ // the crossover point between when it's faster to just read bytewise and read
+ // blocks is somewhere between 16 and 32, so 4 times the size of the block
+ // should be in that range.
+ if (max_strlen < (sizeof(Word) * 4)) {
+ return element::find_first_character(src, ch, max_strlen);
+ }
+ size_t n = max_strlen;
+ // Step 1: read 1 byte at a time to align to block size
+ for (; cur < n && reinterpret_cast<uintptr_t>(char_ptr) % sizeof(Word) != 0;
+ ++cur, ++char_ptr) {
+ if (*char_ptr == ch)
+ return const_cast<unsigned char *>(char_ptr);
+ }
+
+ const Word ch_mask = repeat_byte<Word>(ch);
+
+ // Step 2: read blocks
+ const Word *block_ptr = reinterpret_cast<const Word *>(char_ptr);
+ for (; cur < n && !has_zeroes<Word>((*block_ptr) ^ ch_mask);
+ cur += sizeof(Word), ++block_ptr)
+ ;
+ char_ptr = reinterpret_cast<const unsigned char *>(block_ptr);
+
+ // Step 3: find the match in the block
+ for (; cur < n && *char_ptr != ch; ++cur, ++char_ptr) {
+ ;
+ }
+
+ if (cur >= n || *char_ptr != ch)
+ return static_cast<void *>(nullptr);
+
+ return const_cast<unsigned char *>(char_ptr);
+}
+
+} // namespace word
+
+// Dispatch mechanism for implementations of performance-sensitive
+// functions. Always measure, but generally from lower- to higher-performance
+// order:
+//
+// 1. element - read char-by-char or wchar-by-wchar
+// 3. word - read word-by-word
+// 3. clang_vector - read using clang's internal vector types
+// 4. arch_vector - hand-coded per architecture. Possibly in asm, or with
+// intrinsics.
+//
+// The called implemenation is chosen at build-time by setting
+// LIBC_CONF_{FUNC}_IMPL in config.json
+static constexpr auto &string_length_impl =
+ LIBC_COPT_STRING_LENGTH_IMPL::string_length;
+static constexpr auto &find_first_character_impl =
+ LIBC_COPT_FIND_FIRST_CHARACTER_IMPL::find_first_character;
+
+template <typename T> LIBC_INLINE size_t string_length(const T *src) {
+ if constexpr (cpp::is_same_v<T, char>)
+ return string_length_impl(src);
+ return element::string_length_element<T>(src);
+}
+
+} // namespace internal
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STRING_STRING_LENGTH_H
diff --git a/libc/src/string/string_utils.h b/libc/src/string/string_utils.h
index cbce62ead0328..b0144e01a9006 100644
--- a/libc/src/string/string_utils.h
+++ b/libc/src/string/string_utils.h
@@ -14,172 +14,17 @@
#ifndef LLVM_LIBC_SRC_STRING_STRING_UTILS_H
#define LLVM_LIBC_SRC_STRING_STRING_UTILS_H
-#include "hdr/limits_macros.h"
-#include "hdr/stdint_proxy.h" // uintptr_t
#include "hdr/types/size_t.h"
#include "src/__support/CPP/bitset.h"
-#include "src/__support/CPP/type_traits.h" // cpp::is_same_v
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
#include "src/string/memory_utils/inline_memcpy.h"
-
-#if defined(LIBC_COPT_STRING_UNSAFE_WIDE_READ)
-#if LIBC_HAS_VECTOR_TYPE
-#include "src/string/memory_utils/generic/inline_strlen.h"
-#elif defined(LIBC_TARGET_ARCH_IS_X86)
-#include "src/string/memory_utils/x86_64/inline_strlen.h"
-#elif defined(LIBC_TARGET_ARCH_IS_AARCH64) && defined(__ARM_NEON)
-#include "src/string/memory_utils/aarch64/inline_strlen.h"
-#else
-namespace string_length_impl = LIBC_NAMESPACE::wide_read;
-#endif
-#endif // defined(LIBC_COPT_STRING_UNSAFE_WIDE_READ)
+#include "src/string/string_length.h"
namespace LIBC_NAMESPACE_DECL {
namespace internal {
-template <typename Word> LIBC_INLINE constexpr Word repeat_byte(Word byte) {
- static_assert(CHAR_BIT == 8, "repeat_byte assumes a byte is 8 bits.");
- constexpr size_t BITS_IN_BYTE = CHAR_BIT;
- constexpr size_t BYTE_MASK = 0xff;
- Word result = 0;
- byte = byte & BYTE_MASK;
- for (size_t i = 0; i < sizeof(Word); ++i)
- result = (result << BITS_IN_BYTE) | byte;
- return result;
-}
-
-// The goal of this function is to take in a block of arbitrary size and return
-// if it has any bytes equal to zero without branching. This is done by
-// transforming the block such that zero bytes become non-zero and non-zero
-// bytes become zero.
-// The first transformation relies on the properties of carrying in arithmetic
-// subtraction. Specifically, if 0x01 is subtracted from a byte that is 0x00,
-// then the result for that byte must be equal to 0xff (or 0xfe if the next byte
-// needs a carry as well).
-// The next transformation is a simple mask. All zero bytes will have the high
-// bit set after the subtraction, so each byte is masked with 0x80. This narrows
-// the set of bytes that result in a non-zero value to only zero bytes and bytes
-// with the high bit and any other bit set.
-// The final transformation masks the result of the previous transformations
-// with the inverse of the original byte. This means that any byte that had the
-// high bit set will no longer have it set, narrowing the list of bytes which
-// result in non-zero values to just the zero byte.
-template <typename Word> LIBC_INLINE constexpr bool has_zeroes(Word block) {
- constexpr unsigned int LOW_BITS = repeat_byte<Word>(0x01);
- constexpr Word HIGH_BITS = repeat_byte<Word>(0x80);
- Word subtracted = block - LOW_BITS;
- Word inverted = ~block;
- return (subtracted & inverted & HIGH_BITS) != 0;
-}
-
-template <typename Word>
-LIBC_INLINE size_t string_length_wide_read(const char *src) {
- const char *char_ptr = src;
- // Step 1: read 1 byte at a t...
[truncated]
|
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/215/builds/11020 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/104/builds/36783 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/71/builds/36775 Here is the relevant piece of the build log for the reference |
| "value": false | ||
| "LIBC_CONF_STRING_LENGTH_IMPL": { | ||
| "value": "element" | ||
| } |
Contributor
There was a problem hiding this comment.
it seems like the arm and riscv configs don't have commas on line 5
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/196/builds/14093 Here is the relevant piece of the build log for the reference |
statham-arm
reviewed
Dec 5, 2025
| return sve::string_length(src); | ||
| #elif defined(__ARM_NEON) | ||
| namespace string_length_impl = neon; | ||
| return neon::string_length(src); |
Collaborator
There was a problem hiding this comment.
This #if caused a build failure for us downstream, because it doesn't have a fallback else clause, so that if you compile for the rare AArch64 targets without SVE or NEON, string_length ends up being an empty function, provoking compiler warnings – or errors, if you build at -Werror – about not returning a value and not using the src parameter.
Contributor
Author
There was a problem hiding this comment.
I am unable to test this, but hopefully will be fixed with #170892
honeygoyal
pushed a commit
to honeygoyal/llvm-project
that referenced
this pull request
Dec 9, 2025
…e function selection (llvm#165613) (llvm#170738) [Previous commit had an incorrect default case when FIND_FIRST_CHARACTER_WIDE_READ_IMPL was not specified in config.json. This PR is identical to that one with one line fixed.] 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 complexity out of the command-line. But that's a future problem.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Previous commit had an incorrect default case when FIND_FIRST_CHARACTER_WIDE_READ_IMPL was not specified in config.json. This PR is identical to that one with one line fixed.]
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:
(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 complexity out of the command-line. But that's a future problem.