[FFmpeg-devel] [PATCH] avutil/aarch64/float_dsp_neon: Refactor ff_vector_fmul_add_neon

[Martin Storsjö]

On Sun, 19 Jan 2025, Krzysztof Pyrkosz via ffmpeg-devel wrote:

> Removed a branch, unrolled loop. Speed increase bumped from 3.95 to 5.60.

On what core is that? Please quote the actual output including the 
absolute numbers.

I'm getting much more inconclusive numbers for this one:

Before:            Cortex A53    A72     A73    A78
vector_fmul_add_neon:   620.0  257.2   624.5  162.8
After:
vector_fmul_add_neon:   767.0  259.2   767.5  110.5

This seems to make things quite a lot slower on 2 of these 4 cores. On the 
A78, I'm getting numbers that look like yours though.

So while it makes things better on one kind of core, it also regresses 
things quite a bit on others, so I'm not quite as convinced about this 
one.


> diff --git a/libavutil/aarch64/float_dsp_neon.S b/libavutil/aarch64/float_dsp_neon.S
> index 35e2715b87..0ee5c67b91 100644
> --- a/libavutil/aarch64/float_dsp_neon.S
> +++ b/libavutil/aarch64/float_dsp_neon.S
> @@ -138,19 +138,21 @@ function ff_vector_fmul_window_neon, export=1
> endfunc
>
> function ff_vector_fmul_add_neon, export=1
> -        ld1             {v0.4s, v1.4s},  [x1], #32
> -        ld1             {v2.4s, v3.4s},  [x2], #32
> -        ld1             {v4.4s, v5.4s},  [x3], #32
> -1:      subs            w4,  w4,  #8
> -        fmla            v4.4s,  v0.4s,  v2.4s
> -        fmla            v5.4s,  v1.4s,  v3.4s
> -        b.eq            2f
> -        ld1             {v0.4s, v1.4s},  [x1], #32
> -        ld1             {v2.4s, v3.4s},  [x2], #32
> -        st1             {v4.4s, v5.4s},  [x0], #32
> -        ld1             {v4.4s, v5.4s},  [x3], #32
> -        b               1b
> -2:      st1             {v4.4s, v5.4s},  [x0], #32
> +1:
> +        ldp            q0, q1, [x1], #32
> +        ldp            q2, q3, [x2], #32
> +        ldp            q4, q5, [x3], #32
> +        fmla           v4.4s, v0.4s, v2.4s
> +        fmla           v5.4s, v1.4s, v3.4s
> +        stp            q4, q5, [x0], #32
> +        ldp            q0, q1, [x1], #32
> +        ldp            q2, q3, [x2], #32
> +        ldp            q4, q5, [x3], #32
> +        fmla           v4.4s, v0.4s, v2.4s
> +        fmla           v5.4s, v1.4s, v3.4s
> +        stp            q4, q5, [x0], #32
> +        sub            w4, w4, #16
> +        cbnz           w4, 1b
>         ret

Why do you use sub+cbnz, instead of subs+b.gt? Also, having the cbnz 
directly follow the sub that updates the register, usually causes stalls 
on in-order cores; the usual pattern would be to have such a decrement 
e.g. between the last fmla and the following store.

Doing that change, i.e.

          fmla           v4.4s, v0.4s, v2.4s
          fmla           v5.4s, v1.4s, v3.4s
+        subs           w4, w4, #16
          stp            q4, q5, [x0], #32
-        sub            w4, w4, #16
-        cbnz           w4, 1b
+        b.gt           1b
          ret

has this effect on numbers:

Before:            Cortex A53    A72     A73    A78
vector_fmul_add_neon:   767.0  259.2   769.5  109.0
After:
vector_fmul_add_neon:   751.0  254.5   751.0  109.2

That makes things a little bit better on A53,A72,A73, but it's still 
overall a notable regression on the A53 and A73.

// Martin