Optimize Neon HBD SAD functions#911
Conversation
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.
|
Could we get a review here please @chenm001? |
kirithika7
requested review from
mcw-Lavanya
and removed request for
mcw-Lavanya
chenm001
left a comment
chenm001
left a comment
There was a problem hiding this comment.
Looks good to me, need smoke-test check before merge
chenm001
left a comment
chenm001
left a comment
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This is the existing coding style that we're just adding to in this file - to change it would be to introduce inconsistency between the 8-bit and 10/12-bit implementations. What are the data dependencies introduced by UABD? This instruction is wholly destructive; thus breaking dependencies on previous values. Changing to MOVI #0 followed by UABA introduces an extra data dependency - particularly on in-order cores which don't have the ability to do zero-cost 0 initialization. When benchmarking your suggested change, we observe a ~7% regression on Neoverse V2 and a ~1% regression on Neoverse V3 platforms.
3 participants
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.