From 23c7901f8e445baad8912d76a9115c0440fbf4d8 Mon Sep 17 00:00:00 2001 From: Daniel M'BOUYOU Date: Tue, 18 Aug 2026 11:21:00 +0200 Subject: [PATCH] arm_mult_q31: saturate to 0x7FFFFFFF, as the other two mult kernels do arm_mult_q31, arm_mult_q15 and arm_mult_q7 all document that results outside the allowable range are saturated. arm_mult_q15 and arm_mult_q7 shift by the full fractional width and then saturate. arm_mult_q31 shifts by 32, saturates to 31 bits and doubles, so its reachable ceiling is 0x7FFFFFFE. The only product that leaves the Q31 range, -1.0 x -1.0, therefore returns 0x7FFFFFFE where the documentation promises 0x7FFFFFFF. TEST_MULT_Q31_29 already loads 0x80000000 into both inputs and already compares against a reference pattern holding 0x7FFFFFFF. It passed only because ABS_ERROR_Q31 is 4, which absorbs the one LSB miss. The assertion is tightened to exact equality for that saturation case alone; ABS_ERROR_Q31 is left untouched and every other test keeps it. Stated because the diff understates it: the committed form forces the low bit of the result to zero, so the change is not confined to that one input pair. Measured over 1000000 uniform random pairs, running both kernels in one binary, 500079 outputs change, each by exactly one LSB, difference always within [0, 1]. Whether that accuracy is worth the cost on Cortex-M is a call for the maintainers; no cycle cost was measured here. Built and tested on x86-64 only. Nothing was run on an Arm target. --- Source/BasicMathFunctions/arm_mult_q31.c | 21 +++++---------------- Testing/Include/Tests/BasicTestsQ31.h | 4 ++++ Testing/Source/Tests/BasicTestsQ31.cpp | 16 ++++++++++++++-- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Source/BasicMathFunctions/arm_mult_q31.c b/Source/BasicMathFunctions/arm_mult_q31.c index 0b528b989..064e9087b 100644 --- a/Source/BasicMathFunctions/arm_mult_q31.c +++ b/Source/BasicMathFunctions/arm_mult_q31.c @@ -107,7 +107,6 @@ ARM_DSP_ATTRIBUTE void arm_mult_q31( uint32_t blockSize) { uint32_t blkCnt; /* Loop counter */ - q31_t out; /* Temporary output variable */ #if defined (ARM_MATH_LOOPUNROLL) @@ -119,21 +118,13 @@ ARM_DSP_ATTRIBUTE void arm_mult_q31( /* C = A * B */ /* Multiply inputs and store result in destination buffer. */ - out = ((q63_t) *pSrcA++ * *pSrcB++) >> 32; - out = __SSAT(out, 31); - *pDst++ = out << 1U; + *pDst++ = (q31_t) clip_q63_to_q31(((q63_t) *pSrcA++ * *pSrcB++) >> 31); - out = ((q63_t) *pSrcA++ * *pSrcB++) >> 32; - out = __SSAT(out, 31); - *pDst++ = out << 1U; + *pDst++ = (q31_t) clip_q63_to_q31(((q63_t) *pSrcA++ * *pSrcB++) >> 31); - out = ((q63_t) *pSrcA++ * *pSrcB++) >> 32; - out = __SSAT(out, 31); - *pDst++ = out << 1U; + *pDst++ = (q31_t) clip_q63_to_q31(((q63_t) *pSrcA++ * *pSrcB++) >> 31); - out = ((q63_t) *pSrcA++ * *pSrcB++) >> 32; - out = __SSAT(out, 31); - *pDst++ = out << 1U; + *pDst++ = (q31_t) clip_q63_to_q31(((q63_t) *pSrcA++ * *pSrcB++) >> 31); /* Decrement loop counter */ blkCnt--; @@ -154,9 +145,7 @@ ARM_DSP_ATTRIBUTE void arm_mult_q31( /* C = A * B */ /* Multiply inputs and store result in destination buffer. */ - out = ((q63_t) *pSrcA++ * *pSrcB++) >> 32; - out = __SSAT(out, 31); - *pDst++ = out << 1U; + *pDst++ = (q31_t) clip_q63_to_q31(((q63_t) *pSrcA++ * *pSrcB++) >> 31); /* Decrement loop counter */ blkCnt--; diff --git a/Testing/Include/Tests/BasicTestsQ31.h b/Testing/Include/Tests/BasicTestsQ31.h index e9df9b3e0..8af184b38 100755 --- a/Testing/Include/Tests/BasicTestsQ31.h +++ b/Testing/Include/Tests/BasicTestsQ31.h @@ -29,5 +29,9 @@ class BasicTestsQ31:public Client::Suite /* Offset or scale value */ q31_t scalar; + /* Set for the saturation tests, where the reference pattern is the + exact Q31 limit and no tolerance is allowed. */ + bool exactRef; + q31_t min,max; }; diff --git a/Testing/Source/Tests/BasicTestsQ31.cpp b/Testing/Source/Tests/BasicTestsQ31.cpp index 8559961b8..5b465c5f0 100755 --- a/Testing/Source/Tests/BasicTestsQ31.cpp +++ b/Testing/Source/Tests/BasicTestsQ31.cpp @@ -79,8 +79,17 @@ uint32_t *outp=outputLogical.ptr(); ASSERT_SNR(output,ref,(float32_t)SNR_THRESHOLD); - ASSERT_NEAR_EQ(output,ref,ABS_ERROR_Q31); - + if (this->exactRef) + { + /* Saturation test: the reference pattern is the Q31 limit itself, + so ABS_ERROR_Q31 must not be allowed to absorb the difference. */ + ASSERT_EQ(output,ref); + } + else + { + ASSERT_NEAR_EQ(output,ref,ABS_ERROR_Q31); + } + } void BasicTestsQ31::test_negate_q31() @@ -239,6 +248,8 @@ uint32_t *outp=outputLogical.ptr(); this->scalar = ONEHALF; + this->exactRef = false; + switch(id) { @@ -444,6 +455,7 @@ uint32_t *outp=outputLogical.ptr(); output.create(ref.nbSamples(),BasicTestsQ31::OUT_SAMPLES_ID,mgr); input1.reload(BasicTestsQ31::MAXNEG2_Q31_ID,mgr); input2.reload(BasicTestsQ31::MAXNEG2_Q31_ID,mgr); + this->exactRef = true; break; case BasicTestsQ31::TEST_NEGATE_Q31_30: