Skip to content

Extend range of arm_mult_q31 - #274

Closed
saturn691 wants to merge 1 commit into
ARM-software:mainfrom
saturn691:arm_mult_q31
Closed

Extend range of arm_mult_q31#274
saturn691 wants to merge 1 commit into
ARM-software:mainfrom
saturn691:arm_mult_q31

Conversation

@saturn691

@saturn691 saturn691 commented Sep 30, 2025

Copy link
Copy Markdown

This is the approach used by arm_mult_q7 and arm_mult_q15. The approach currently used would set the LSB to 0 due to the final left shift. My understanding of why that approach was used is because __SSAT(out, 32) doesn't work, because it operates on 32-bit integers, which means it cannot detect that 0x80000000 > 0x7fffffff, as it will interpret this signed.

On devices that support sqdmulh, the compiler can optimise out the core calculation for that function. It must match these semantics, to generate the optimisations.

@christophe0606 christophe0606 added the review Under review label Oct 1, 2025
@christophe0606

Copy link
Copy Markdown
Contributor

@saturn691 I have not benchmarked but I think this version would be slower. That's why we sometimes have different versions of the same kernel with different accuracy / speed tradedoff.

It would be better as a new function since it is a different accuracy / speed tradeoff and I don't want to change too much the behavior of a function that is already in use.

Your understanding is right : this version is using __SSAT (because it is faster) but it implies that one bit of accuracy has to be lost otherwise __SSAT could not detect the saturation.

@christophe0606 christophe0606 added enhancement New feature or request and removed review Under review labels Oct 2, 2025
@christophe0606

Copy link
Copy Markdown
Contributor

I have created a new github issue to track this enhancement for future.
I close this PR.
A new function will be required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants