From 62d6cd8f28fa10d57f056c1c076bdff8e9904992 Mon Sep 17 00:00:00 2001 From: Saikumar Mandaji Date: Sat, 18 Jul 2026 18:53:03 +0530 Subject: [PATCH] fix(none.h): widen SMLALD/SMLALDX products to 64-bit before summing The software fallback __SMLALD/__SMLALDX in Include/dsp/none.h (used on cores without the hardware DSP extension, e.g. Cortex-M0/M0+/M3/M23) computed both 16x16 products as 32-bit q31_t and summed them before widening to the 64-bit accumulator. Two same-sign near-full-scale products (each up to 2^30) sum to 2^31, overflowing signed 32-bit and wrapping negative before the (q63_t)sum widening ever applies. This silently produces wrong results in every Q15 function that hits this fallback on a non-DSP core, including arm_fir_q15, arm_mat_vec_mult_q15, arm_lms_q15, arm_lms_norm_q15, arm_conv_opt_q15/arm_conv_partial_opt_q15, and arm_correlate_opt_q15 -- directly contradicting the documented guarantee that the 64-bit accumulator carries no risk of internal overflow. Minimal repro (from the issue): x = y = 0x80008000 (both 16-bit halves = -32768). Each product is (-32768)*(-32768) = 2^30; their 32-bit sum 2^31 wraps to -2^31 under the old code. The true value is +2^31. Fix: cast one operand of each product to q63_t before multiplying, so C's usual arithmetic conversions promote the whole product (and the sum) to 64-bit, matching the fix already suggested in the issue. Verified by hand against the issue's worked arithmetic; no C compiler was available in the environment this patch was authored in to run the reproduction directly, so please double check against CI/local build before merging. Fixes #319 Signed-off-by: Saikumar Mandaji --- Include/dsp/none.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Include/dsp/none.h b/Include/dsp/none.h index 019c41f33..03c3cc939 100755 --- a/Include/dsp/none.h +++ b/Include/dsp/none.h @@ -500,9 +500,13 @@ __STATIC_FORCEINLINE uint32_t __ROR(uint32_t op1, uint32_t op2) uint64_t sum) { /* return (sum + ((q15_t) (x >> 16) * (q15_t) (y >> 16)) + ((q15_t) x * (q15_t) y)); */ - return ((uint64_t)(((((q31_t)x << 16) >> 16) * (((q31_t)y << 16) >> 16)) + - ((((q31_t)x ) >> 16) * (((q31_t)y ) >> 16)) + - ( ((q63_t)sum ) ) )); + /* Both products must be widened to 64-bit before they are summed: with + * both operands left as 32-bit `q31_t`, two same-sign near-full-scale + * products (each up to 2^30) sum to 2^31, which overflows signed 32-bit + * and wraps negative before ever reaching the 64-bit accumulator. */ + return ((uint64_t)((q63_t)(((q31_t)x << 16) >> 16) * (((q31_t)y << 16) >> 16) + + (q63_t)(((q31_t)x ) >> 16) * (((q31_t)y ) >> 16) + + ( (q63_t)sum ) )); } @@ -515,9 +519,10 @@ __STATIC_FORCEINLINE uint32_t __ROR(uint32_t op1, uint32_t op2) uint64_t sum) { /* return (sum + ((q15_t) (x >> 16) * (q15_t) y)) + ((q15_t) x * (q15_t) (y >> 16)); */ - return ((uint64_t)(((((q31_t)x << 16) >> 16) * (((q31_t)y ) >> 16)) + - ((((q31_t)x ) >> 16) * (((q31_t)y << 16) >> 16)) + - ( ((q63_t)sum ) ) )); + /* See __SMLALD above: widen to 64-bit before summing the two products. */ + return ((uint64_t)((q63_t)(((q31_t)x << 16) >> 16) * (((q31_t)y ) >> 16) + + (q63_t)(((q31_t)x ) >> 16) * (((q31_t)y << 16) >> 16) + + ( (q63_t)sum ) )); }