Merge pull request 'Replace SIMD assembly with initialized Node16::index + valgrind client request' (#77) from client-request-instead-of-asm into main

Reviewed-on: #77

CMakeLists.txt

@@ -5,7 +5,7 @@ project(
   DESCRIPTION
     "A data structure for optimistic concurrency control on ranges of bitwise-lexicographically-ordered keys."
   HOMEPAGE_URL "https://git.weaselab.dev/weaselab/conflict-set"
-  LANGUAGES C CXX ASM)
+  LANGUAGES C CXX)
 set(CMAKE_CXX_STANDARD 20)
 
 file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/version.txt ${PROJECT_VERSION})
@@ -130,19 +130,7 @@ endif()
 
 set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "")
 
-# Architecture-specific SIMD assembly. These functions operate on
-# potentially-indeterminate memory, which is UB in C++ but well-defined in
-# assembly.
-set(SIMD_ASM_FILES)
-if(CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64 AND NOT USE_SIMD_FALLBACK)
-  set(SIMD_ASM_FILES ${CMAKE_CURRENT_SOURCE_DIR}/simd_x86_64.S)
-elseif((CMAKE_SYSTEM_PROCESSOR STREQUAL aarch64 OR CMAKE_SYSTEM_PROCESSOR
-                                                   STREQUAL arm64)
-       AND NOT USE_SIMD_FALLBACK)
-  set(SIMD_ASM_FILES ${CMAKE_CURRENT_SOURCE_DIR}/simd_aarch64.S)
-endif()
-
-add_library(${PROJECT_NAME}-object OBJECT ConflictSet.cpp ${SIMD_ASM_FILES})
+add_library(${PROJECT_NAME}-object OBJECT ConflictSet.cpp)
 target_compile_options(${PROJECT_NAME}-object PRIVATE -fno-exceptions
                                                       -fvisibility=hidden)
 target_include_directories(${PROJECT_NAME}-object
@@ -245,7 +233,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
   endif()
 
   # ad hoc testing
-  add_executable(conflict_set_main ConflictSet.cpp ${SIMD_ASM_FILES})
+  add_executable(conflict_set_main ConflictSet.cpp)
   target_include_directories(conflict_set_main
                              PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include)
   target_compile_definitions(conflict_set_main PRIVATE ENABLE_MAIN)
@@ -261,7 +249,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
     cmake_pop_check_state()
 
     if(HAS_LIB_FUZZER)
-      add_executable(conflict_set_fuzz_test ConflictSet.cpp ${SIMD_ASM_FILES})
+      add_executable(conflict_set_fuzz_test ConflictSet.cpp)
       target_include_directories(conflict_set_fuzz_test
                                  PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include)
       target_compile_definitions(conflict_set_fuzz_test PRIVATE ENABLE_FUZZ)
@@ -273,8 +261,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
   endif()
 
   # whitebox tests asan+ubsan
-  add_executable(fuzz_driver ConflictSet.cpp FuzzTestDriver.cpp
-                             ${SIMD_ASM_FILES})
+  add_executable(fuzz_driver ConflictSet.cpp FuzzTestDriver.cpp)
   target_compile_options(fuzz_driver PRIVATE ${TEST_FLAGS})
   if(NOT CMAKE_CROSSCOMPILING)
     target_compile_options(fuzz_driver PRIVATE -fsanitize=address,undefined)
@@ -290,8 +277,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
 
   # whitebox tests msan
   if(MSAN_TOOLCHAIN_PATH)
-    add_executable(fuzz_driver_msan ConflictSet.cpp FuzzTestDriver.cpp
-                                    ${SIMD_ASM_FILES})
+    add_executable(fuzz_driver_msan ConflictSet.cpp FuzzTestDriver.cpp)
     target_compile_options(fuzz_driver_msan PRIVATE ${TEST_FLAGS})
     if(NOT CMAKE_CROSSCOMPILING)
       target_compile_options(
@@ -319,8 +305,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
 
   # tsan tests
   if(NOT CMAKE_CROSSCOMPILING AND NOT DISABLE_TSAN)
-    add_executable(tsan_driver ConflictSet.cpp FuzzTestDriver.cpp
-                               ${SIMD_ASM_FILES})
+    add_executable(tsan_driver ConflictSet.cpp FuzzTestDriver.cpp)
     target_compile_options(tsan_driver PRIVATE ${TEST_FLAGS} -fsanitize=thread)
     target_link_options(tsan_driver PRIVATE -fsanitize=thread)
     target_compile_definitions(tsan_driver PRIVATE ENABLE_FUZZ THREAD_TEST)

ConflictSet.cpp

@@ -28,7 +28,6 @@ limitations under the License.
 #include "Internal.h"
 #include "LongestCommonPrefix.h"
 #include "Metrics.h"
-#include "simd.h"
 
 #include <algorithm>
 #include <bit>
@@ -489,6 +488,7 @@ inline void Node3::copyChildrenAndKeyFrom(const Node16 &other) {
 
 inline void Node16::copyChildrenAndKeyFrom(const Node3 &other) {
   copyCommon(*this, other);
+  memset(index, 0, sizeof(index));
   memcpy(index, other.index, Node3::kMaxNodes);
   memcpy(children, other.children,
          Node3::kMaxNodes * sizeof(children[0])); // NOLINT
@@ -500,6 +500,12 @@ inline void Node16::copyChildrenAndKeyFrom(const Node3 &other) {
     assert(children[i]->parent == &other);
     children[i]->parent = this;
   }
+  // The index bytes beyond numChildren are initialized so that the SIMD loads
+  // of the full 16 bytes are well-defined (reading indeterminate bytes is UB
+  // in C++). valgrind then treats them as undefined so that it can still flag
+  // any read that relies on them.
+  VALGRIND_MAKE_MEM_UNDEFINED(index + numChildren,
+                              sizeof(index) - numChildren * sizeof(index[0]));
 }
 
 inline void Node16::copyChildrenAndKeyFrom(const Node16 &other) {
@@ -516,6 +522,7 @@ inline void Node16::copyChildrenAndKeyFrom(const Node16 &other) {
 
 inline void Node16::copyChildrenAndKeyFrom(const Node48 &other) {
   copyCommon(*this, other);
+  memset(index, 0, sizeof(index));
   int i = 0;
   other.bitSet.forEachSet([&](int c) {
     // Suppress a false positive -Waggressive-loop-optimizations warning
@@ -529,6 +536,12 @@ inline void Node16::copyChildrenAndKeyFrom(const Node48 &other) {
     ++i;
   });
   memcpy(partialKey(), &other + 1, partialKeyLen);
+  // The index bytes beyond numChildren are initialized so that the SIMD loads
+  // of the full 16 bytes are well-defined (reading indeterminate bytes is UB
+  // in C++). valgrind then treats them as undefined so that it can still flag
+  // any read that relies on them.
+  VALGRIND_MAKE_MEM_UNDEFINED(index + numChildren,
+                              sizeof(index) - numChildren * sizeof(index[0]));
 }
 
 inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
@@ -536,6 +549,7 @@ inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
   assert(numChildren == Node16::kMaxNodes);
   memset(index, -1, sizeof(index));
   memset(children, 0, sizeof(children));
+  memset(reverseIndex, 0, sizeof(reverseIndex));
   const auto z = InternalVersionT::zero;
   for (auto &v : childMaxVersion) {
     v = z;
@@ -555,6 +569,13 @@ inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
         std::max(maxOfMax[i >> Node48::kMaxOfMaxShift], childMaxVersion[i]);
     ++i;
   }
+  // The reverseIndex bytes beyond numChildren are initialized so that the
+  // SIMD loads of a full 16-byte page in scan16 are well-defined (reading
+  // indeterminate bytes is UB in C++). valgrind then treats them as undefined
+  // so that it can still flag any read that relies on them.
+  VALGRIND_MAKE_MEM_UNDEFINED(reverseIndex + numChildren,
+                              sizeof(reverseIndex) -
+                                  numChildren * sizeof(reverseIndex[0]));
 }
 
 inline void Node48::copyChildrenAndKeyFrom(const Node48 &other) {
@@ -581,6 +602,7 @@ inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
   copyCommon(*this, other);
   memset(index, -1, sizeof(index));
   memset(children, 0, sizeof(children));
+  memset(reverseIndex, 0, sizeof(reverseIndex));
   const auto z = InternalVersionT::zero;
   for (auto &v : childMaxVersion) {
     v = z;
@@ -602,6 +624,13 @@ inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
     ++i;
   });
   memcpy(partialKey(), &other + 1, partialKeyLen);
+  // The reverseIndex bytes beyond numChildren are initialized so that the
+  // SIMD loads of a full 16-byte page in scan16 are well-defined (reading
+  // indeterminate bytes is UB in C++). valgrind then treats them as undefined
+  // so that it can still flag any read that relies on them.
+  VALGRIND_MAKE_MEM_UNDEFINED(reverseIndex + numChildren,
+                              sizeof(reverseIndex) -
+                                  numChildren * sizeof(reverseIndex[0]));
 }
 
 inline void Node256::copyChildrenAndKeyFrom(const Node48 &other) {
@@ -911,21 +940,51 @@ int getNodeIndexExists(Node3 *self, uint8_t index) {
 
 int getNodeIndex(Node16 *self, uint8_t index) {
 
-#if defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
-  uint32_t bitfield =
-      find_eq_16(self->index, index) & ((1 << self->numChildren) - 1);
+#ifdef HAS_AVX
+  // Based on https://www.the-paper-trail.org/post/art-paper-notes/
+
+  // key_vec is 16 repeated copies of the searched-for byte, one for every
+  // possible position in child_keys that needs to be searched.
+  __m128i key_vec = _mm_set1_epi8(index);
+
+  // Compare all child_keys to 'index' in parallel. Don't worry if some of the
+  // keys aren't valid, we'll mask the results to only consider the valid ones
+  // below.
+  __m128i indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  __m128i results = _mm_cmpeq_epi8(key_vec, indices);
+
+  // Build a mask to select only the first node->num_children values from the
+  // comparison (because the other values are meaningless)
+  uint32_t mask = (1 << self->numChildren) - 1;
+
+  // Change the results of the comparison into a bitfield, masking off any
+  // invalid comparisons.
+  uint32_t bitfield = _mm_movemask_epi8(results) & mask;
+
+  // No match if there are no '1's in the bitfield.
   if (bitfield == 0)
     return -1;
+
+  // Find the index of the first '1' in the bitfield by counting the leading
+  // zeros.
   return std::countr_zero(bitfield);
 #elif defined(HAS_ARM_NEON)
-  // The index load is done in assembly (find_eq_16) so that reading the
-  // potentially-indeterminate unused index bytes is well-defined.
-  uint64_t bitfield = find_eq_16(self->index, index);
+  // Based on
+  // https://community.arm.com/arm-community-blogs/b/infrastructure-solutions-blog/posts/porting-x86-vector-bitmask-optimizations-to-arm-neon
+
+  uint8x16_t indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  // 0xff for each match
+  uint16x8_t results =
+      vreinterpretq_u16_u8(vceqq_u8(vdupq_n_u8(index), indices));
   assume(self->numChildren <= Node16::kMaxNodes);
   uint64_t mask = self->numChildren == 16
                       ? uint64_t(-1)
                       : (uint64_t(1) << (self->numChildren * 4)) - 1;
-  bitfield &= mask;
+  // 0xf for each match in valid range
+  uint64_t bitfield =
+      vget_lane_u64(vreinterpret_u64_u8(vshrn_n_u16(results, 4)), 0) & mask;
   if (bitfield == 0)
     return -1;
   return std::countr_zero(bitfield) / 4;
@@ -941,20 +1000,31 @@ int getNodeIndex(Node16 *self, uint8_t index) {
 
 int getNodeIndexExists(Node16 *self, uint8_t index) {
 
-#if defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
-  uint32_t bitfield =
-      find_eq_16(self->index, index) & ((1 << self->numChildren) - 1);
+#ifdef HAS_AVX
+  __m128i key_vec = _mm_set1_epi8(index);
+  __m128i indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  __m128i results = _mm_cmpeq_epi8(key_vec, indices);
+  uint32_t mask = (1 << self->numChildren) - 1;
+  uint32_t bitfield = _mm_movemask_epi8(results) & mask;
   assume(bitfield != 0);
   return std::countr_zero(bitfield);
 #elif defined(HAS_ARM_NEON)
-  // The index load is done in assembly (find_eq_16) so that reading the
-  // potentially-indeterminate unused index bytes is well-defined.
-  uint64_t bitfield = find_eq_16(self->index, index);
+  // Based on
+  // https://community.arm.com/arm-community-blogs/b/infrastructure-solutions-blog/posts/porting-x86-vector-bitmask-optimizations-to-arm-neon
+
+  uint8x16_t indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  // 0xff for each match
+  uint16x8_t results =
+      vreinterpretq_u16_u8(vceqq_u8(vdupq_n_u8(index), indices));
   assume(self->numChildren <= Node16::kMaxNodes);
   uint64_t mask = self->numChildren == 16
                       ? uint64_t(-1)
                       : (uint64_t(1) << (self->numChildren * 4)) - 1;
-  bitfield &= mask;
+  // 0xf for each match in valid range
+  uint64_t bitfield =
+      vget_lane_u64(vreinterpret_u64_u8(vshrn_n_u16(results, 4)), 0) & mask;
   assume(bitfield != 0);
   return std::countr_zero(bitfield) / 4;
 #else
@@ -1226,19 +1296,29 @@ TaggedNodePointer getChildGeq(Node16 *self, int child) {
     return nullptr;
   }
 
-#if defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
-  uint32_t bitfield =
-      find_ge_16(self->index, child) & ((1 << self->numChildren) - 1);
+#ifdef HAS_AVX
+  __m128i key_vec = _mm_set1_epi8(child);
+  __m128i indices;
+  memcpy(&indices, self->index, Node16::kMaxNodes);
+  __m128i results = _mm_cmpeq_epi8(key_vec, _mm_min_epu8(key_vec, indices));
+  int mask = (1 << self->numChildren) - 1;
+  uint32_t bitfield = _mm_movemask_epi8(results) & mask;
   return bitfield == 0 ? nullptr : self->children[std::countr_zero(bitfield)];
 #elif defined(HAS_ARM_NEON)
-  // The index load is done in assembly (find_ge_16) so that reading the
-  // potentially-indeterminate unused index bytes is well-defined.
-  uint64_t bitfield = find_ge_16(self->index, child);
+  uint8x16_t indices;
+  memcpy(&indices, self->index, sizeof(self->index));
+  // 0xff for each leq
+  auto results = vcleq_u8(vdupq_n_u8(child), indices);
   assume(self->numChildren <= Node16::kMaxNodes);
   uint64_t mask = self->numChildren == 16
                       ? uint64_t(-1)
                       : (uint64_t(1) << (self->numChildren * 4)) - 1;
-  bitfield &= mask;
+  // 0xf for each 0xff (within mask)
+  uint64_t bitfield =
+      vget_lane_u64(
+          vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)),
+          0) &
+      mask;
   return bitfield == 0 ? nullptr
                        : self->children[std::countr_zero(bitfield) / 4];
 #else
@@ -1945,6 +2025,8 @@ Node *erase(Node *self, WriteContext *writeContext, bool logical) {
     parent48->childMaxVersion[lastChildrenIndex] = writeContext->zero;
     VALGRIND_MAKE_MEM_UNDEFINED(parent48->children + lastChildrenIndex,
                                 sizeof(parent48->children[0]));
+    VALGRIND_MAKE_MEM_UNDEFINED(parent48->reverseIndex + lastChildrenIndex,
+                                sizeof(parent48->reverseIndex[0]));
 
     if (needsDownsize(parent48)) {
       downsize(parent48, writeContext);
@@ -2094,11 +2176,14 @@ bool scan16(const InternalVersionT *vs, const uint8_t *is, int begin, int end,
 
 #ifdef HAS_ARM_NEON
 
-  // The index load is done in assembly (mask_in_range_16) so that reading
-  // potentially-indeterminate unused index bytes is well-defined. `vs` slots
-  // beyond the in-use range are always initialized (to zero) by the allocator,
-  // so the version compare below stays in C++.
-  uint64_t mask = mask_in_range_16(is, begin, end);
+  uint8x16_t indices;
+  memcpy(&indices, is, 16);
+  // 0xff for each in bounds
+  auto results =
+      vcltq_u8(vsubq_u8(indices, vdupq_n_u8(begin)), vdupq_n_u8(end - begin));
+  // 0xf for each 0xff
+  uint64_t mask = vget_lane_u64(
+      vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)), 0);
 
   uint64_t compared = vget_lane_u64(
       vreinterpret_u64_u8(vshrn_n_u16(
@@ -2107,9 +2192,13 @@ bool scan16(const InternalVersionT *vs, const uint8_t *is, int begin, int end,
 
   return !(compared & mask);
 
-#elif defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
+#elif defined(HAS_AVX)
 
-  uint32_t mask = mask_in_range_16(is, begin, end);
+  __m128i indices;
+  memcpy(&indices, is, 16);
+  indices = _mm_sub_epi8(indices, _mm_set1_epi8(begin));
+  uint32_t mask = ~_mm_movemask_epi8(_mm_cmpeq_epi8(
+      indices, _mm_max_epu8(indices, _mm_set1_epi8(end - begin))));
 
   uint32_t compared = 0;
   if constexpr (kAVX512) {
@@ -2247,11 +2336,14 @@ bool checkMaxBetweenExclusiveImpl(Node16 *n, int begin, int end,
 
 #ifdef HAS_ARM_NEON
 
-  // The index load is done in assembly (mask_in_range_16) so that reading the
-  // potentially-indeterminate unused index bytes is well-defined. The unused
-  // childMaxVersion slots are always initialized (to zero) by the allocator,
-  // so the version compare below stays in C++.
-  uint64_t mask = mask_in_range_16(self->index, begin, end);
+  uint8x16_t indices;
+  memcpy(&indices, self->index, 16);
+  // 0xff for each in bounds
+  auto results =
+      vcltq_u8(vsubq_u8(indices, vdupq_n_u8(begin)), vdupq_n_u8(end - begin));
+  // 0xf for each 0xff
+  uint64_t mask = vget_lane_u64(
+      vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)), 0);
 
   mask &= self->numChildren == 16
               ? uint64_t(-1)
@@ -2271,10 +2363,15 @@ bool checkMaxBetweenExclusiveImpl(Node16 *n, int begin, int end,
 
   return !(compared & mask) && firstRangeOk;
 
-#elif defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
+#elif defined(HAS_AVX)
 
-  uint32_t mask = mask_in_range_16(self->index, begin, end) &
-                  ((1 << self->numChildren) - 1);
+  __m128i indices;
+  memcpy(&indices, self->index, 16);
+  indices = _mm_sub_epi8(indices, _mm_set1_epi8(begin));
+  uint32_t mask =
+      0xffff & ~_mm_movemask_epi8(_mm_cmpeq_epi8(
+                   indices, _mm_max_epu8(indices, _mm_set1_epi8(end - begin))));
+  mask &= (1 << self->numChildren) - 1;
   if (!mask) {
     return true;
   }

simd.h

@@ -1,48 +0,0 @@
-#pragma once
-
-#include <cstdint>
-
-#if defined(__x86_64__) && !defined(USE_SIMD_FALLBACK)
-
-// SIMD operations on potentially-indeterminate Node16::index[16] bytes.
-// Implemented in file-level assembly (simd_x86_64.S) because loading and
-// operating on indeterminate values is UB in C++ but well-defined in
-// assembly. The caller must mask the returned bitfield to
-// [0, numChildren) before using it.
-//
-// Each function returns a 16-bit bitmask in the low 16 bits of a uint32_t
-// (upper 16 bits are zero). Bit i is set iff the condition holds at index i.
-
-extern "C" {
-// Returns bit i set iff idx[i] == key
-uint32_t find_eq_16(const uint8_t idx[16], uint8_t key);
-// Returns bit i set iff idx[i] >= child
-uint32_t find_ge_16(const uint8_t idx[16], uint8_t child);
-// Returns bit i set iff begin <= idx[i] < end
-uint32_t mask_in_range_16(const uint8_t idx[16], uint8_t begin, uint8_t end);
-}
-
-#elif defined(__aarch64__) && !defined(USE_SIMD_FALLBACK)
-
-// SIMD operations on potentially-indeterminate Node16::index[16] bytes.
-// Implemented in file-level assembly (simd_aarch64.S) because loading and
-// operating on indeterminate values is UB in C++ but well-defined in
-// assembly. The caller must mask the returned bitfield to [0, numChildren)
-// before using it.
-//
-// AArch64 has no pmovmskb-equivalent, so (unlike x86-64) each function returns
-// a 64-bit "nibble mask": nibble i (bits [4i, 4i+4)) is 0xf iff the condition
-// holds at index i. Bit (4i + 3) is the high bit of byte i's result. Callers
-// locate a set lane with std::countr_zero(bitfield) / 4 and mask the valid
-// lanes with (uint64_t(1) << (numChildren * 4)) - 1.
-
-extern "C" {
-// Returns nibble i = 0xf iff idx[i] == key
-uint64_t find_eq_16(const uint8_t idx[16], uint8_t key);
-// Returns nibble i = 0xf iff idx[i] >= child
-uint64_t find_ge_16(const uint8_t idx[16], uint8_t child);
-// Returns nibble i = 0xf iff begin <= idx[i] < end
-uint64_t mask_in_range_16(const uint8_t idx[16], uint8_t begin, uint8_t end);
-}
-
-#endif

simd_aarch64.S

@@ -1,84 +0,0 @@
-// SIMD operations on potentially-indeterminate Node16::index[16] bytes.
-// Written in assembly because loading and operating on indeterminate values
-// is undefined behavior in C++ ([basic.indet]) but well-defined in assembly.
-// The caller is responsible for masking the returned bitfield to
-// [0, numChildren) before using it.
-//
-// Unlike x86-64 (which has pmovmskb), AArch64 has no single instruction that
-// produces a 1-bit-per-byte mask. Each function therefore returns a 64-bit
-// "nibble mask" in x0: nibble i (bits [4i, 4i+4)) is 0xf iff the condition
-// holds at index i. Bit (4i + 3) is the high bit of byte i's result. Callers
-// locate a set lane with countr_zero(bitfield) / 4 and mask the valid lanes
-// with (uint64_t(1) << (numChildren * 4)) - 1.
-//
-// AArch64 AAPCS:
-//   x0 = const uint8_t *idx  (16 bytes, may contain indeterminate data)
-//   w1 = uint8_t key         (find_eq_16, find_ge_16)
-//   w1 = uint8_t begin       (mask_in_range_16)
-//   w2 = uint8_t end         (mask_in_range_16)
-//
-// These functions are only ever called directly (never indirectly), so they
-// do not need BTI landing pads; the object is still marked BTI/PAC/GCS-aware
-// below so a -z force-bti link keeps BTI enabled for the whole binary.
-
-    .text
-
-// uint64_t find_eq_16(const uint8_t idx[16], uint8_t key)
-//   nibble i = 0xf iff idx[i] == key
-    .globl find_eq_16
-    .type  find_eq_16, %function
-find_eq_16:
-    dup   v1.16b, w1         // broadcast key
-    ldr   q0, [x0]           // load 16 bytes (may be indeterminate)
-    cmeq  v0.16b, v0.16b, v1.16b  // 0xff for each match
-    shrn  v0.8b, v0.8h, 4   // pack 16 byte-flags into 8 nibble-pairs
-    umov  x0, v0.d[0]
-    ret
-    .size find_eq_16, .-find_eq_16
-
-// uint64_t find_ge_16(const uint8_t idx[16], uint8_t child)
-//   nibble i = 0xf iff idx[i] >= child (unsigned)
-//   cmhs gives unsigned ">=" (higher-or-same): Vd = Vn >= Vm.
-    .globl find_ge_16
-    .type  find_ge_16, %function
-find_ge_16:
-    dup   v1.16b, w1         // broadcast child
-    ldr   q0, [x0]           // load 16 bytes
-    cmhs  v0.16b, v0.16b, v1.16b  // 0xff where idx[i] >= child (unsigned)
-    shrn  v0.8b, v0.8h, 4
-    umov  x0, v0.d[0]
-    ret
-    .size find_ge_16, .-find_ge_16
-
-// uint64_t mask_in_range_16(const uint8_t idx[16], uint8_t begin, uint8_t end)
-//   nibble i = 0xf iff begin <= idx[i] < end  (unsigned, wrapping arithmetic)
-//   Logic: (idx[i] - begin) < (end - begin), valid when end - begin < 256.
-//   cmhi gives unsigned ">" (higher): Vd = Vn > Vm. We want
-//   (end - begin) > (idx - begin), so Vn = (end - begin).
-    .globl mask_in_range_16
-    .type  mask_in_range_16, %function
-mask_in_range_16:
-    dup   v1.16b, w1         // broadcast begin
-    dup   v2.16b, w2         // broadcast end
-    ldr   q0, [x0]           // load 16 bytes
-    sub   v0.16b, v0.16b, v1.16b  // idx - begin (wrapping)
-    sub   v2.16b, v2.16b, v1.16b  // end - begin (range size)
-    cmhi  v0.16b, v2.16b, v0.16b  // 0xff where (end-begin) > (idx-begin)
-    shrn  v0.8b, v0.8h, 4
-    umov  x0, v0.d[0]
-    ret
-    .size mask_in_range_16, .-mask_in_range_16
-
-    // Declare AArch64 branch-protection compatibility, matching what the
-    // compiler emits for -mbranch-protection=standard (BTI + PAC + GCS). This
-    // keeps the object indistinguishable from C/C++ translation units for
-    // linkers enforcing BTI (-z force-bti). The functions above are only ever
-    // called directly, so they need no BTI landing pads; PAC/GCS compatibility
-    // holds trivially since they use no stack.
-
-    .aeabi_subsection aeabi_feature_and_bits, optional, ULEB128
-    .aeabi_attribute Tag_Feature_BTI, 1
-    .aeabi_attribute Tag_Feature_PAC, 1
-    .aeabi_attribute Tag_Feature_GCS, 1
-
-    .section .note.GNU-stack,"",@progbits

simd_x86_64.S

@@ -1,86 +0,0 @@
-// SIMD operations on potentially-indeterminate Node16::index[16] bytes.
-// Written in assembly because loading and operating on indeterminate values
-// is UB in C++ but well-defined in assembly. (A side effect is that msan
-// doesn't track the loads.) The caller is responsible for masking the
-// returned bitfield to [0, numChildren) before using it.
-//
-// All functions return a 16-bit bitmask in %eax (bit i set = condition true
-// at index i). The upper 16 bits of %eax are zero.
-//
-// System V AMD64 ABI:
-//   %rdi = const uint8_t *idx  (16 bytes)
-//   %esi = uint8_t key         (find_eq_16, find_ge_16)
-//   %sil = uint8_t begin       (mask_in_range_16)
-//   %dl  = uint8_t end         (mask_in_range_16)
-
-    .text
-
-// uint32_t find_eq_16(const uint8_t idx[16], uint8_t key)
-//   Returns bit i set if idx[i] == key
-    .globl find_eq_16
-    .type  find_eq_16, @function
-find_eq_16:
-    vmovd      %esi, %xmm1          // broadcast key
-    vpbroadcastb %xmm1, %xmm1
-    vmovdqu    (%rdi), %xmm0        // load 16 bytes (may contain indeterminate data)
-    vpcmpeqb   %xmm0, %xmm1, %xmm0  // 0xff for each match
-    vpmovmskb  %xmm0, %eax          // 16-bit bitmask
-    movzwl     %ax, %eax            // zero-extend to 32 bits
-    ret
-    .size find_eq_16, .-find_eq_16
-
-// uint32_t find_ge_16(const uint8_t idx[16], uint8_t child)
-//   Returns bit i set if idx[i] >= child
-//   x86 doesn't have a "compare unsigned >=" for bytes directly, so we use:
-//   min(key, idx[i]) == key  iff  idx[i] >= key
-    .globl find_ge_16
-    .type  find_ge_16, @function
-find_ge_16:
-    vmovd      %esi, %xmm1
-    vpbroadcastb %xmm1, %xmm1       // key broadcast
-    vmovdqu    (%rdi), %xmm0        // load 16 bytes
-    vpminub    %xmm0, %xmm1, %xmm2  // min(key, idx[i])
-    vpcmpeqb   %xmm2, %xmm1, %xmm0  // 0xff where min == key, i.e. idx[i] >= key
-    vpmovmskb  %xmm0, %eax
-    movzwl     %ax, %eax
-    ret
-    .size find_ge_16, .-find_ge_16
-
-// uint32_t mask_in_range_16(const uint8_t idx[16], uint8_t begin, uint8_t end)
-//   Returns bit i set if begin <= idx[i] < end
-//   Logic: (idx[i] - begin) < (end - begin)  [unsigned wrapping arithmetic]
-//   Equivalently: idx[i] - begin != max(idx[i] - begin, end - begin)
-//   i.e. idx[i] - begin is NOT equal to the saturated value.
-//   We compute: sub = idx - begin; result = (sub < (end-begin)) for each byte.
-//   Using: sub == max(sub, end-begin) means NOT in range.
-//   So: in_range = ~(movemask(cmpeq(sub, max(sub, range_size))))
-    .globl mask_in_range_16
-    .type  mask_in_range_16, @function
-mask_in_range_16:
-    vmovd      %esi, %xmm1          // begin
-    vpbroadcastb %xmm1, %xmm1
-    vmovd      %edx, %xmm2          // end
-    vpbroadcastb %xmm2, %xmm2
-    vmovdqu    (%rdi), %xmm0        // load 16 bytes
-    vpsubb     %xmm1, %xmm0, %xmm0  // idx - begin (wrapping)
-    vpsubb     %xmm1, %xmm2, %xmm2  // end - begin (range size)
-    vpmaxub    %xmm0, %xmm2, %xmm3  // max(idx-begin, range_size)
-    vpcmpeqb   %xmm3, %xmm0, %xmm0  // 0xff where NOT in range
-    vpmovmskb  %xmm0, %eax
-    not        %eax                 // invert: 1 = in range
-    movzwl     %ax, %eax
-    ret
-    .size mask_in_range_16, .-mask_in_range_16
-
-    .section .note.gnu.property,"a",@note
-    .p2align 3, 0x0
-    .long 4
-    .long 16
-    .long 5
-    .asciz "GNU"
-    .long 0xc0000002
-    .long 4
-    .long 0x3
-    .p2align 3, 0x0
-
-    .section .note.GNU-stack,"",@progbits