[libc-commits] [libc] [libc] Keep the vectorised string loads observable to the compiler (PR #236612)
Mike Lothian via libc-commits
libc-commits at lists.llvm.org
Tue Nov 3 05:51:07 PST 2026
https://github.com/FireBurn updated https://github.com/llvm/llvm-project/pull/236612
>From f93658d502dea842f5fa29b42030e181724d8c99 Mon Sep 17 00:00:00 2001
From: Mike Lothian <mike at fireburn.co.uk>
Date: Thu, 29 Oct 2026 00:34:06 +0000
Subject: [PATCH 1/4] [libc] Keep the vectorised string loads observable to the
compiler
The clang_vector, x86_64 and aarch64 string_length and
find_first_character load the whole aligned vector around the string,
which reads outside the object on purpose. When a call is inlined with a
known array, as under LTO in the unit tests, the compiler treats the
bytes outside the array as undefined and may drop the stores it does not
see being read: strlen gave 30, 48 or 38 for a six character string
built with -O3 -flto=thin, and the x86_64 versions gave 39 and 21.
align_down_for_overread passes the pointer through an empty asm that
clobbers memory, so the object counts as read, and the aligned address
through another so the compiler cannot reason about the load. This is
the idiom memset_explicit uses, and it costs nothing at run time.
Assisted-by: Claude
---
.../memory_utils/aarch64/inline_strlen.h | 4 ++--
.../memory_utils/generic/inline_strlen.h | 10 ++++------
libc/src/string/memory_utils/utils.h | 20 +++++++++++++++++++
.../memory_utils/x86_64/inline_strlen.h | 7 +++----
4 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/libc/src/string/memory_utils/aarch64/inline_strlen.h b/libc/src/string/memory_utils/aarch64/inline_strlen.h
index 87f6cb8cf9bd55..408490e55da66d 100644
--- a/libc/src/string/memory_utils/aarch64/inline_strlen.h
+++ b/libc/src/string/memory_utils/aarch64/inline_strlen.h
@@ -12,6 +12,7 @@
#if defined(__ARM_NEON)
#include "src/__support/CPP/bit.h" // countr_zero
+#include "src/string/memory_utils/utils.h"
#include <arm_neon.h>
#include <stddef.h> // size_t
namespace LIBC_NAMESPACE_DECL {
@@ -21,8 +22,7 @@ string_length(const char *src) {
using Vector __attribute__((may_alias)) = uint8x8_t;
uintptr_t misalign_bytes = reinterpret_cast<uintptr_t>(src) % sizeof(Vector);
- const Vector *block_ptr =
- reinterpret_cast<const Vector *>(src - misalign_bytes);
+ const Vector *block_ptr = align_down_for_overread<Vector>(src);
Vector v = *block_ptr;
Vector vcmp = vceqz_u8(v);
uint64x1_t cmp_mask = vreinterpret_u64_u8(vcmp);
diff --git a/libc/src/string/memory_utils/generic/inline_strlen.h b/libc/src/string/memory_utils/generic/inline_strlen.h
index 833ff9fbcbf80a..54d5e0448e4163 100644
--- a/libc/src/string/memory_utils/generic/inline_strlen.h
+++ b/libc/src/string/memory_utils/generic/inline_strlen.h
@@ -12,6 +12,7 @@
#include "src/__support/CPP/bit.h"
#include "src/__support/CPP/simd.h"
#include "src/__support/common.h"
+#include "src/string/memory_utils/utils.h"
namespace LIBC_NAMESPACE_DECL {
namespace clang_vector {
@@ -28,9 +29,8 @@ LIBC_INLINE constexpr cpp::simd_mask<byte_ty> shift_mask(cpp::simd_mask<char> m,
LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE size_t string_length(const char *src) {
constexpr cpp::simd<char> null_byte = cpp::splat('\0');
- size_t alignment = alignof(cpp::simd<char>);
- const cpp::simd<char> *aligned = reinterpret_cast<const cpp::simd<char> *>(
- __builtin_align_down(src, alignment));
+ using Vector = cpp::simd<char>;
+ const Vector *aligned = align_down_for_overread<Vector>(src);
cpp::simd<char> chars = cpp::load<cpp::simd<char>>(aligned, /*aligned=*/true);
cpp::simd_mask<char> mask = chars == null_byte;
@@ -61,9 +61,7 @@ find_first_character(const unsigned char *s, unsigned char c, size_t n) {
using Mask = cpp::simd_mask<char>;
Vector c_byte = c;
- size_t alignment = alignof(Vector);
- const Vector *aligned =
- reinterpret_cast<const Vector *>(__builtin_align_down(s, alignment));
+ const Vector *aligned = align_down_for_overread<Vector>(s);
Vector chars = cpp::load<Vector>(aligned, /*aligned=*/true);
Mask cmp_v = chars == c_byte;
diff --git a/libc/src/string/memory_utils/utils.h b/libc/src/string/memory_utils/utils.h
index b1a4885cb8c1f2..3dc54882325124 100644
--- a/libc/src/string/memory_utils/utils.h
+++ b/libc/src/string/memory_utils/utils.h
@@ -50,6 +50,26 @@ LIBC_INLINE uintptr_t distance_to_next_aligned(const void *ptr) {
return alignment - distance_to_align_down<alignment>(ptr);
}
+// Returns ptr rounded down to the alignment of Vector, for loads that read
+// outside the object ptr points into on purpose. The compiler may treat such a
+// read as undefined or drop the stores it does not see being read, which
+// happens once a call is inlined on a known array. Passing the pointer through
+// an asm that clobbers memory makes the object observable and hides the
+// aligned address.
+template <typename Vector>
+LIBC_INLINE const Vector *align_down_for_overread(const void *ptr) {
+#ifndef LIBC_COMPILER_IS_MSVC
+ asm("" : : "r"(ptr) : "memory");
+#endif
+ const Vector *aligned = reinterpret_cast<const Vector *>(
+ static_cast<const char *>(ptr) -
+ distance_to_align_down<alignof(Vector)>(ptr));
+#ifndef LIBC_COMPILER_IS_MSVC
+ asm("" : "+r"(aligned));
+#endif
+ return aligned;
+}
+
// Returns the same pointer but notifies the compiler that it is aligned.
template <size_t alignment, typename T> LIBC_INLINE T *assume_aligned(T *ptr) {
return reinterpret_cast<T *>(__builtin_assume_aligned(ptr, alignment));
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 0343e22f3e58e3..ec7410463c1d15 100644
--- a/libc/src/string/memory_utils/x86_64/inline_strlen.h
+++ b/libc/src/string/memory_utils/x86_64/inline_strlen.h
@@ -9,6 +9,7 @@
#define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_X86_64_INLINE_STRLEN_H
#include "src/__support/CPP/bit.h" // countr_zero
+#include "src/string/memory_utils/utils.h"
#include <immintrin.h>
#include <stddef.h> // size_t
@@ -29,8 +30,7 @@ LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE static size_t
string_length_vector(const char *src) {
uintptr_t misalign_bytes = reinterpret_cast<uintptr_t>(src) % sizeof(Vector);
- const Vector *block_ptr =
- reinterpret_cast<const Vector *>(src - misalign_bytes);
+ const Vector *block_ptr = align_down_for_overread<Vector>(src);
auto cmp = CompareAndMask(block_ptr, 0) >> misalign_bytes;
if (cmp)
return cpp::countr_zero(cmp);
@@ -61,8 +61,7 @@ LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE static void *
find_first_character_vector(const unsigned char *s, unsigned char c, size_t n) {
uintptr_t misalign_bytes = reinterpret_cast<uintptr_t>(s) % sizeof(Vector);
- const Vector *block_ptr =
- reinterpret_cast<const Vector *>(s - misalign_bytes);
+ const Vector *block_ptr = align_down_for_overread<Vector>(s);
auto cmp_bytes = CompareAndMask(block_ptr, c) >> misalign_bytes;
if (cmp_bytes)
return calculate_find_first_character_return<Mask>(
>From edab66ca226e575cb626b6b526c5562adad56e48 Mon Sep 17 00:00:00 2001
From: Mike Lothian <mike at fireburn.co.uk>
Date: Tue, 3 Nov 2026 13:17:17 +0000
Subject: [PATCH 2/4] [libc] Do not return a match one past the searched range
in the vector find loops
find_first_character returned a match at index n, the first byte after
the range, because the check was n_left < c_offset instead of
n_left <= c_offset. memchr on a buffer whose byte at n matched returned a
pointer outside the range it was asked to search.
Assisted-by: Claude
---
libc/src/string/memory_utils/generic/inline_strlen.h | 2 +-
libc/src/string/memory_utils/x86_64/inline_strlen.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libc/src/string/memory_utils/generic/inline_strlen.h b/libc/src/string/memory_utils/generic/inline_strlen.h
index 54d5e0448e4163..d2e3377b468886 100644
--- a/libc/src/string/memory_utils/generic/inline_strlen.h
+++ b/libc/src/string/memory_utils/generic/inline_strlen.h
@@ -50,7 +50,7 @@ LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE size_t string_length(const char *src) {
LIBC_INLINE static void *calculate_find_first_character_return(
const char *src, cpp::simd_mask<char> c_mask, size_t n_left) {
size_t c_offset = cpp::find_first_set(c_mask);
- if (n_left < c_offset)
+ if (n_left <= c_offset)
return nullptr;
return const_cast<char *>(src) + c_offset;
}
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 ec7410463c1d15..a03e6040d38595 100644
--- a/libc/src/string/memory_utils/x86_64/inline_strlen.h
+++ b/libc/src/string/memory_utils/x86_64/inline_strlen.h
@@ -50,7 +50,7 @@ LIBC_INLINE static void *
calculate_find_first_character_return(const unsigned char *src, Mask c_mask,
size_t n_left) {
size_t c_offset = cpp::countr_zero(c_mask);
- if (n_left < c_offset)
+ if (n_left <= c_offset)
return nullptr;
return const_cast<unsigned char *>(src) + c_offset;
}
>From 45f2f0cd6e4bcf265f3e0588af022489813fbb52 Mon Sep 17 00:00:00 2001
From: Mike Lothian <mike at fireburn.co.uk>
Date: Tue, 3 Nov 2026 13:18:01 +0000
Subject: [PATCH 3/4] [libc] Fix the AVX2 find_first_character compare
compare_and_mask for __m256i broadcast the character with
_mm256_set1_epi16, which sets 16-bit lanes, and then compared bytes. Only
a character whose upper byte pattern matched could be found, so memchr
missed most matches with AVX2 enabled. Use _mm256_set1_epi8, as the 128
and 512 bit versions do.
Assisted-by: Claude
---
libc/src/string/memory_utils/x86_64/inline_strlen.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 a03e6040d38595..5c5369ce08b1c1 100644
--- a/libc/src/string/memory_utils/x86_64/inline_strlen.h
+++ b/libc/src/string/memory_utils/x86_64/inline_strlen.h
@@ -109,7 +109,7 @@ template <>
LIBC_INLINE uint32_t
compare_and_mask<__m256i, uint32_t>(const __m256i *block_ptr, char c) {
__m256i b = _mm256_load_si256(block_ptr);
- __m256i set = _mm256_set1_epi16(c);
+ __m256i set = _mm256_set1_epi8(c);
__m256i cmp = _mm256_cmpeq_epi8(b, set);
return _mm256_movemask_epi8(cmp);
}
>From 1feba1cc2745089b16c3081a703ca43e63284b50 Mon Sep 17 00:00:00 2001
From: Mike Lothian <mike at fireburn.co.uk>
Date: Tue, 3 Nov 2026 13:18:48 +0000
Subject: [PATCH 4/4] [libc] Test the vector string search loops
Call each string_length and find_first_character implementation on local
arrays of several sizes, at every offset into them. The arrays are filled
in a separate function so a read outside the array, or a search that runs
past the range, shows up as a wrong answer once the call is inlined.
Without the previous three commits it fails at -O3 on the length checks
and at any level on the find checks.
Assisted-by: Claude
---
.../src/string/memory_utils/CMakeLists.txt | 12 +++
.../memory_utils/inline_strlen_test.cpp | 101 ++++++++++++++++++
2 files changed, 113 insertions(+)
create mode 100644 libc/test/src/string/memory_utils/inline_strlen_test.cpp
diff --git a/libc/test/src/string/memory_utils/CMakeLists.txt b/libc/test/src/string/memory_utils/CMakeLists.txt
index 0b94cd22df1aca..6b5e74d361b32c 100644
--- a/libc/test/src/string/memory_utils/CMakeLists.txt
+++ b/libc/test/src/string/memory_utils/CMakeLists.txt
@@ -23,3 +23,15 @@ if(LIBC_TARGET_ARCHITECTURE_IS_AARCH64 OR
libc.src.string.memory_utils.memory_utils
)
endif()
+
+add_libc_test(
+ inline_strlen_test
+ SUITE
+ libc-string-tests
+ SRCS
+ inline_strlen_test.cpp
+ COMPILE_OPTIONS
+ ${LIBC_COMPILE_OPTIONS_NATIVE}
+ DEPENDS
+ libc.src.string.memory_utils.memory_utils
+)
diff --git a/libc/test/src/string/memory_utils/inline_strlen_test.cpp b/libc/test/src/string/memory_utils/inline_strlen_test.cpp
new file mode 100644
index 00000000000000..3310587dd4aae6
--- /dev/null
+++ b/libc/test/src/string/memory_utils/inline_strlen_test.cpp
@@ -0,0 +1,101 @@
+//===-- Unit tests for the vectorised string search loops -----------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/string/string_length.h"
+#include "test/UnitTest/Test.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+// The vector loops read whole aligned vectors around the string, so they read
+// outside the array on purpose. These tests call them on local arrays that the
+// compiler can see through once the call is inlined, which is where a read
+// outside the object used to give a wrong answer. They only fail in an
+// optimised build, and under LTO when the array is filled in another file.
+
+namespace {
+
+// Fills the array in a separate function so the stores come right before the
+// call and are dropped if the compiler does not see them being read.
+template <size_t N> [[gnu::noinline]] void fill(char (&a)[N]) {
+ for (size_t i = 0; i + 1 < N; ++i)
+ a[i] = static_cast<char>('a' + i % 26);
+ a[N - 1] = '\0';
+}
+
+template <size_t (*Length)(const char *),
+ void *(*Find)(const unsigned char *, unsigned char, size_t)>
+struct Impl {
+ template <size_t N> static void check_array() {
+ char a[N];
+ fill(a);
+ for (size_t offset = 0; offset < N; ++offset) {
+ ASSERT_EQ(Length(a + offset), N - 1 - offset);
+ if (N - 1 - offset == 0)
+ continue;
+ if constexpr (N >= 2) {
+ // The last character of the string, which may also occur earlier.
+ size_t n = N - 1 - offset;
+ size_t first = offset;
+ while (a[first] != a[N - 2])
+ ++first;
+ void *found = Find(reinterpret_cast<unsigned char *>(a) + offset,
+ static_cast<unsigned char>(a[N - 2]), n);
+ ASSERT_TRUE(found == a + first);
+ // Neither a character that is absent nor one just past the range.
+ ASSERT_TRUE(Find(reinterpret_cast<unsigned char *>(a) + offset, '0',
+ n) == nullptr);
+ a[N - 1] = '0';
+ ASSERT_TRUE(Find(reinterpret_cast<unsigned char *>(a) + offset, '0',
+ n) == nullptr);
+ a[N - 1] = '\0';
+ }
+ }
+ }
+
+ static void run() {
+ check_array<1>();
+ check_array<2>();
+ check_array<7>();
+ check_array<16>();
+ check_array<17>();
+ check_array<31>();
+ check_array<32>();
+ check_array<33>();
+ check_array<63>();
+ check_array<64>();
+ check_array<65>();
+ check_array<100>();
+ }
+};
+
+} // namespace
+
+TEST(LlvmLibcInlineStrlenTest, Element) {
+ Impl<internal::element::string_length,
+ internal::element::find_first_character>::run();
+}
+
+TEST(LlvmLibcInlineStrlenTest, Word) {
+ Impl<internal::word::string_length,
+ internal::word::find_first_character>::run();
+}
+
+#if LIBC_HAS_VECTOR_TYPE
+TEST(LlvmLibcInlineStrlenTest, ClangVector) {
+ Impl<clang_vector::string_length, clang_vector::find_first_character>::run();
+}
+#endif
+
+#if defined(LIBC_TARGET_ARCH_IS_X86)
+TEST(LlvmLibcInlineStrlenTest, ArchVector) {
+ Impl<internal::arch_vector::string_length,
+ internal::arch_vector::find_first_character>::run();
+}
+#endif
+
+} // namespace LIBC_NAMESPACE_DECL
More information about the libc-commits
mailing list