Fix ARM NEON scan helpers truncating 64-bit versions
CI / release (arm64, , ubuntu-latest-arm64) (pull_request) Successful in 2m13s
CI / pre-commit (pull_request) Successful in 2m2s
CI / test (-DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, debug) (pull_request) Successful in 3m47s
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Successful in 3m15s
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Successful in 3m12s
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Successful in 3m17s
CI / release (amd64, -DMSAN_TOOLCHAIN_PATH=/opt/msan, ubuntu-latest-amd64) (pull_request) Successful in 5m34s
CI / coverage (pull_request) Successful in 3m43s
CI / release (arm64, , ubuntu-latest-arm64) (pull_request) Successful in 2m13s
CI / pre-commit (pull_request) Successful in 2m2s
CI / test (-DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, debug) (pull_request) Successful in 3m47s
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Successful in 3m15s
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Successful in 3m12s
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Successful in 3m17s
CI / release (amd64, -DMSAN_TOOLCHAIN_PATH=/opt/msan, ubuntu-latest-amd64) (pull_request) Successful in 5m34s
CI / coverage (pull_request) Successful in 3m43s
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
This commit is contained in:
+56
-57
@@ -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<const int64_t *>(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 <bool kAVX512>
|
||||
@@ -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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user