From 6b1f597d77e79b943d3f3e08727bd17709f05c1d Mon Sep 17 00:00:00 2001 From: Weaselbot Date: Sun, 2 Aug 2026 23:35:32 -0400 Subject: [PATCH] Fix ARM NEON scan helpers truncating 64-bit versions When USE_64_BIT=1, InternalVersionT stores an int64_t, but the aarch64 NEON fast paths in scan16 (both the indexed Node48 variant and the Node256 variant) and checkMaxBetweenExclusiveImpl(Node16) only copied the low 32 bits of each version into uint32x4_t lanes and compared against a truncated 32-bit readVersion. This produced wrong conflict/commit decisions once any version exceeded 2^32 - 1. Extract the per-version "greater than readVersion" computation into a shared conflictMask16 helper. For 64-bit versions it uses vcgtq_s64 on the full int64 values and narrows the resulting 16-byte mask into the same nibble-packed layout the callers already consume. The 32-bit path is preserved unchanged. Closes #69 --- ConflictSet.cpp | 113 ++++++++++++++++++++++++------------------------ 1 file changed, 56 insertions(+), 57 deletions(-) diff --git a/ConflictSet.cpp b/ConflictSet.cpp index 8ebab6c..82c4558 100644 --- a/ConflictSet.cpp +++ b/ConflictSet.cpp @@ -2057,6 +2057,52 @@ compare16_avx512(const InternalVersionT *vs, InternalVersionT rv) { } #endif +#ifdef HAS_ARM_NEON +// Returns a uint8x16_t whose byte i is 0xff if vs[i] > readVersion and 0 +// otherwise. When USE_64_BIT is set the comparison uses the full 64-bit value +// (via vcgtq_s64); otherwise it falls back to the 32-bit subtract-and-compare +// used by the original NEON fast path. The caller narrows this 16-byte vector +// into the nibble-packed (4 bits per version) mask expected by scan16 and +// checkMaxBetweenExclusiveImpl. +inline uint8x16_t conflictMask16(const InternalVersionT *vs, + InternalVersionT readVersion) { +#if USE_64_BIT + int64_t rv; + memcpy(&rv, &readVersion, sizeof(rv)); + const auto rvVec = vdupq_n_s64(rv); + int32x2_t r32[8]; + const auto *vsp = reinterpret_cast(vs); + for (int j = 0; j < 8; ++j) { + r32[j] = vmovn_s64(vcgtq_s64(vld1q_s64(vsp + 2 * j), rvVec)); + } + uint32x4_t w4[4]; + for (int k = 0; k < 4; ++k) { + w4[k] = vreinterpretq_u32_s32(vcombine_s32(r32[2 * k], r32[2 * k + 1])); + } + return vcombine_u8( + vmovn_u16(vcombine_u16(vmovn_u32(w4[0]), vmovn_u32(w4[1]))), + vmovn_u16(vcombine_u16(vmovn_u32(w4[2]), vmovn_u32(w4[3])))); +#else + uint32x4_t w4[4]; + memcpy(w4, vs, sizeof(w4)); + uint32_t rv; + memcpy(&rv, &readVersion, sizeof(rv)); + const auto rvVec = vdupq_n_u32(rv); + + int32x4_t z; + memset(&z, 0, sizeof(z)); + + uint16x4_t conflicting[4]; + for (int i = 0; i < 4; ++i) { + conflicting[i] = + vmovn_u32(vcgtq_s32(vreinterpretq_s32_u32(vsubq_u32(w4[i], rvVec)), z)); + } + return vcombine_u8(vmovn_u16(vcombine_u16(conflicting[0], conflicting[1])), + vmovn_u16(vcombine_u16(conflicting[2], conflicting[3]))); +#endif +} +#endif + // Returns true if v[i] <= readVersion for all i such that begin <= is[i] < end // Preconditions: begin <= end, end - begin < 256 template @@ -2077,26 +2123,10 @@ bool scan16(const InternalVersionT *vs, const uint8_t *is, int begin, int end, uint64_t mask = vget_lane_u64( vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(results), 4)), 0); - uint32x4_t w4[4]; - memcpy(w4, vs, sizeof(w4)); - uint32_t rv; - memcpy(&rv, &readVersion, sizeof(rv)); - const auto rvVec = vdupq_n_u32(rv); - - int32x4_t z; - memset(&z, 0, sizeof(z)); - - uint16x4_t conflicting[4]; - for (int i = 0; i < 4; ++i) { - conflicting[i] = - vmovn_u32(vcgtq_s32(vreinterpretq_s32_u32(vsubq_u32(w4[i], rvVec)), z)); - } - auto combined = - vcombine_u8(vmovn_u16(vcombine_u16(conflicting[0], conflicting[1])), - vmovn_u16(vcombine_u16(conflicting[2], conflicting[3]))); - uint64_t compared = vget_lane_u64( - vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(combined), 4)), 0); + vreinterpret_u64_u8(vshrn_n_u16( + vreinterpretq_u16_u8(conflictMask16(vs, readVersion)), 4)), + 0); return !(compared & mask); @@ -2142,26 +2172,10 @@ bool scan16(const InternalVersionT *vs, int begin, int end, assert(begin <= end); #if defined(HAS_ARM_NEON) - uint32x4_t w4[4]; - memcpy(w4, vs, sizeof(w4)); - uint32_t rv; - memcpy(&rv, &readVersion, sizeof(rv)); - const auto rvVec = vdupq_n_u32(rv); - - int32x4_t z; - memset(&z, 0, sizeof(z)); - - uint16x4_t conflicting[4]; - for (int i = 0; i < 4; ++i) { - conflicting[i] = - vmovn_u32(vcgtq_s32(vreinterpretq_s32_u32(vsubq_u32(w4[i], rvVec)), z)); - } - auto combined = - vcombine_u8(vmovn_u16(vcombine_u16(conflicting[0], conflicting[1])), - vmovn_u16(vcombine_u16(conflicting[2], conflicting[3]))); - uint64_t conflict = vget_lane_u64( - vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(combined), 4)), 0); + vreinterpret_u64_u8(vshrn_n_u16( + vreinterpretq_u16_u8(conflictMask16(vs, readVersion)), 4)), + 0); conflict &= end == 16 ? -1 : (uint64_t(1) << (end << 2)) - 1; conflict >>= begin << 2; @@ -2275,26 +2289,11 @@ bool checkMaxBetweenExclusiveImpl(Node16 *n, int begin, int end, const bool firstRangeOk = !child->entryPresent || child->entry.rangeVersion <= readVersion; - uint32x4_t w4[4]; - memcpy(w4, self->childMaxVersion, sizeof(w4)); - uint32_t rv; - memcpy(&rv, &readVersion, sizeof(rv)); - const auto rvVec = vdupq_n_u32(rv); - - int32x4_t z; - memset(&z, 0, sizeof(z)); - - uint16x4_t conflicting[4]; - for (int i = 0; i < 4; ++i) { - conflicting[i] = - vmovn_u32(vcgtq_s32(vreinterpretq_s32_u32(vsubq_u32(w4[i], rvVec)), z)); - } - auto combined = - vcombine_u8(vmovn_u16(vcombine_u16(conflicting[0], conflicting[1])), - vmovn_u16(vcombine_u16(conflicting[2], conflicting[3]))); - uint64_t compared = vget_lane_u64( - vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(combined), 4)), 0); + vreinterpret_u64_u8(vshrn_n_u16(vreinterpretq_u16_u8(conflictMask16( + self->childMaxVersion, readVersion)), + 4)), + 0); return !(compared & mask) && firstRangeOk;