[FFmpeg-devel] [PATCH v2 0/1] lavc/aarch64: add some neon pix_abs functions

[Martin Storsjö]

On Thu, 14 Apr 2022, Swinney, Jonathan wrote:

> Thanks Martin for the review. I made some updates according to the 
> suggestions you made.
>
> I added a checkasm function, but I'm new to the test framework, so it 
> may need some work still.

Thanks for putting in the effort to make a test - that adds a lot of value 
to the project! I'll follow up with a review of the newly added test.

>> Would it be beneficial to not do the addv here on each iteration, but
>> accumulate in v18.8h, and the just do one single addv at the end?
>
> I did some testing with this, and it is, indeed, faster, however the 
> problem I ran into is that the bounds for the loop can be higher than 
> the capacity of a 16-bit unsigned integer used for accumulating in 
> v16.8h. With the addv inside the loop, this register can hold the 
> maximum possible value without overflowing before we increase the width 
> of the field with the add instruction. If you have a trick or there are 
> restrictions on the inputs that I am not aware of, let me know.

Ok, I see! Can the accumulation be done with widening, i.e. uaddw v18.4s, 
v18.4s, v16.4h, uaddw2 v19.4s, v19.4s, v16.8h? (Or you could add both 
halves into the same v18.4s, but using two destination registers there 
probably helps.) That would keep the logic more SIMD-like while doing the 
flattening to one scalar value only once at the end.

>> This version most certainly would be slower indeed. If we could be ok with
>> doing a bit of overread, the simplest version might be to load e.g. "ld1
>> {v4.16b, v5.16b}, [x2]" followed by "ext v5.16b, v4.16b, v5.16b, #1" to
>> shift it. But doing an overlapping unaligned load probably is fine too.
>
> I experimented with these instructions and found that it was marginally 
> faster on Graviton 2, but it was slower on Graviton 3. Graviton 3 has 
> more vector pipelines than Graviton 2 and is therefore a better 
> candidate for video encoding workloads, so I chose to prefer Graviton 3. 
> I didn't test it other chips.

Ok - I guess the current implementation of that aspect is fine then!

// Martin