Skip to content

Optimize Neon HBD SAD functions#911

Open
alex-davicenko-arm wants to merge 2 commits into
Multicorewareinc:masterfrom
alex-davicenko-arm:opt-neon-sad
Open

Optimize Neon HBD SAD functions#911
alex-davicenko-arm wants to merge 2 commits into
Multicorewareinc:masterfrom
alex-davicenko-arm:opt-neon-sad

Conversation

@alex-davicenko-arm

Copy link
Copy Markdown
Contributor

Optimize 10-bit and 12-bit Neon SAD functions by accumulating into 16-bit vectors and widening only at the point of
overflow. This provides an uplift of up to 20% for pixel sad and 40% for sad_x3/sad_x4 when measured on a Neoverse V3 platform with LLVM-22.

Optimize 10-bit and 12-bit pixel_sad_wxh_neon functions by
accumulating into 16-bit vectors and widening only at the point of
overflow.
Optimize 10-bit and 12-bit sad_x3_wxh_neon and sad_x4_wxh_neon
functions by accumulating into 16-bit vectors and widening only at the
point of overflow.
@jwright-arm

Copy link
Copy Markdown

Could we get a review here please @chenm001?

@kirithika7 kirithika7 requested review from mcw-Lavanya and removed request for mcw-Lavanya June 26, 2026 12:14

@chenm001 chenm001 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, need smoke-test check before merge

@chenm001 chenm001 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some code need adjustment

ld1 {v0.8h-v2.8h}, [x0], x7
ld1 {v3.8h-v5.8h}, [x1], x5
\f v16.8h, v0.8h, v3.8h
uaba v16.8h, v1.8h, v4.8h

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this code style. It mixed UABA and \f instructions, a little difficult readable.
I understand that \f may be replaced with UABD to reduce one zero-clearing instruction.
But there is a paradox here:
Low-end CPUs (such as A53/A55) in-order execute. This instruction has data dependencies and may cause a pipeline stall;
High-end CPUs out-of-order execute, so pre-zeroing register has almost no cost.
Another issue is that, to accommodate UABD, you split the loop into two segments, using UABD for one segment and UABA for the remaining part. The result is that there are additional instructions outside the loop, and the number of loop iterations is reduced by one. The actual runtime cache consumption and impact on branch prediction make the total number of cycles not necessarily less.
So, I still suggest zero v16-v19 in advance to maintain consistency in the code within the loop.
Another patch also has a similar issue, but it unrolls count is higher, so it's not as severe as this one, that one has more cache performance risk.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants