Fix saturating subtraction when the subtrahend is the type minimum

[vsaraikin]

Bug

Saturating subtraction ssub wraps instead of saturating when the subtrahend is the type minimum:

xsimd::ssub<int8_t>(1, -128);   // got -127, expected 127
xsimd::ssub<int8_t>(0, -128);   // got -128, expected 127
// batch<int32_t> on SSE2 (which uses the common kernel for 32-bit ints):
// ssub(1, INT_MIN) got -2147483647, expected 2147483647

Root cause

Both the scalar overload (arch/xsimd_scalar.hpp) and the common batch kernel (arch/common/xsimd_common_arithmetic.hpp) implement signed ssub(lhs, rhs) as sadd(lhs, -rhs). When rhs == numeric_limits<T>::min(), -rhs is not representable (it wraps back to min(), and is signed-overflow UB in the scalar case), so the saturating add is fed the wrong operand and the result wraps.

The common kernel matters beyond the scalar API: SSE/SSE2 only provide native saturating subtraction for 8- and 16-bit lanes, so batch<int32_t> / batch<int64_t> ssub dispatch to this common implementation on x86.

Fix

Compute the saturating difference directly, mirroring the existing signed sadd:

• Scalar: branch on the sign of rhs and compare against max() + rhs / lowest() + rhs (both safe from overflow because of the sign), else lhs - rhs.
• Batch kernel: select(other >= 0, max(min + other, self), min(max + other, self)) - other, the direct analogue of the signed sadd kernel above it.

The native NEON/SSE 8/16-bit paths (vqsubq_*, _mm_subs_*) are unaffected — only the scalar overload and the common fallback change.

Testing

Extended test_ssub in test/test_xsimd_api.cpp with the min-subtrahend cases (ssub(0, min) and ssub(1, min) must saturate to max for signed types). The previous test only covered ssub(122, 121), so this edge was untested.

Verified exhaustively out of tree: an all-pairs int8 scalar sweep is now 0/65536 wrong (was failing for every rhs == -128), and the forced common-kernel batch<int32_t> cases with INT_MIN now return INT_MAX, on Apple Silicon (arm64).

[serge-sans-paille]

Thanks for spotting this. There's a bit of cruft in the patch (including some surprising formatting change), but the approach looks good. I leave it to you to fix CI before having a look ;-)

[vsaraikin]

Thanks for the review! Fixed both points:

• Cruft removed — the stray {} → { } reformatting (from a local clang-format with a different brace style) is gone; the diff is now only the ssub logic + one test.
• CI failures — root cause was that my first pass only fixed the scalar overload and the common batch kernel. AVX2 and AVX512 route 32/64-bit integer ssub through separate avx/avx512f overloads that had their own copy of the sadd(x, -rhs) pattern, so those lanes still wrapped on INT_MIN — hence the AVX/AVX512 job failures while SSE-only and NEON passed. Both now delegate to the fixed common kernel (the same fallback SSE2 already uses). Verified the common kernel produces the saturated result for 8/16/32/64-bit signed types.

[DiamonDinoia]

why directly to commmon here? are there no intrinsics for it?

[serge-sans-paille]

indeed there's

 _mm256_subs_epi16
 _mm256_subs_epu16
 _mm256_subs_epi8
 _mm256_subs_epu8

[vsaraikin]

You're right, I've since dropped the common detour and implemented it inline here, mirroring the sadd right above

And yeah, as Serge noted, AVX2 has subs_epi8/epi16, those stay on the native path in avx2::ssub. This overload only handles the 32/64-bit case, which has no saturating-sub intrinsic

[DiamonDinoia]

I mean you can also emulate saturating using sub + select no? Would that be faster that a scalar loop?

[DiamonDinoia]

same here.

[serge-sans-paille]

it comes with AVX512BW. Yet falling back to avx for [u]int[8[16]_t is probably better.

[vsaraikin]

happy to make the 8/16 path fall back to avx instead if you'd prefer

[DiamonDinoia]

why max vs lowest?

[vsaraikin]

matching the sadd just above it, which uses max() / lowest()

lowest() (not min()) is deliberate so it's also correct for floating-point T, where min() is the smallest positive value

for integers they're identical

[serge-sans-paille]

it comes with AVX512BW. Yet falling back to avx for [u]int[8[16]_t is probably better.

[vsaraikin]

hey, the MSVC crash was the test blowing the stack in the big templated test case (moved the new checks to a standalone one) + added the AVX512F -> AVX2 fallback for 8/16-bit