Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avutil/aarch64/float_dsp_neon: Refactor ff_vector_fmul_add_neon
@ 2025-01-19 17:33 Krzysztof Pyrkosz via ffmpeg-devel
  2025-01-19 20:57 ` Martin Storsjö
  0 siblings, 1 reply; 3+ messages in thread
From: Krzysztof Pyrkosz via ffmpeg-devel @ 2025-01-19 17:33 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Krzysztof Pyrkosz

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

Krzysztof

---
 libavutil/aarch64/float_dsp_neon.S | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

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
 endfunc
 
-- 
2.45.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avutil/aarch64/float_dsp_neon: Refactor ff_vector_fmul_add_neon
  2025-01-19 17:33 [FFmpeg-devel] [PATCH] avutil/aarch64/float_dsp_neon: Refactor ff_vector_fmul_add_neon Krzysztof Pyrkosz via ffmpeg-devel
@ 2025-01-19 20:57 ` Martin Storsjö
  2025-01-23 21:31   ` Krzysztof Pyrkosz via ffmpeg-devel
  0 siblings, 1 reply; 3+ messages in thread
From: Martin Storsjö @ 2025-01-19 20:57 UTC (permalink / raw)
  To: Krzysztof Pyrkosz via ffmpeg-devel; +Cc: Krzysztof Pyrkosz

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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avutil/aarch64/float_dsp_neon: Refactor ff_vector_fmul_add_neon
  2025-01-19 20:57 ` Martin Storsjö
@ 2025-01-23 21:31   ` Krzysztof Pyrkosz via ffmpeg-devel
  0 siblings, 0 replies; 3+ messages in thread
From: Krzysztof Pyrkosz via ffmpeg-devel @ 2025-01-23 21:31 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Krzysztof Pyrkosz

On Sun, Jan 19, 2025 at 10:57:57PM +0200, Martin Storsjö wrote:
> 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.

Indeed, I conducted the tests and gathered the benchmark results on the
A78, Rock5B to be more specific.

> 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
> 

Below are my results for different cores I have lying around. First is
the mainline version, second uses the patch from the original email,
third improves things by reordering instructions:

- vector_fmul_neon
                 A72      A78      Thinkpad x13s
Mainline:        218.42   114.7    62.85
Original patch:  223.92   86.16    64.77
Reordered instr: 221.75   85.66    61.16

- vector_fmul_add_neon
                 A72      A78      Thinkpad x13s
Mainline:        269.49   163.43   84.48
Original patch:  269.42   114.16   85.31
Reordered instr: 266.92   111.62   85.04

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

Let's skip this patch and two others then, thank you for the review.

Krzysztof
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-01-23 21:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-19 17:33 [FFmpeg-devel] [PATCH] avutil/aarch64/float_dsp_neon: Refactor ff_vector_fmul_add_neon Krzysztof Pyrkosz via ffmpeg-devel
2025-01-19 20:57 ` Martin Storsjö
2025-01-23 21:31   ` Krzysztof Pyrkosz via ffmpeg-devel

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git