PR #22830 opened by mkver
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22830
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22830.patch

This function is exported, so has to abide by the ABI
and therefore issues emms since commit
5b85ca5317f28a73eadf10d604d2a7421b84bed8. Yet this is
expensive and using SSE2 instead improves performance.
Also avoid the initial zeroing and the last pointer
increment while just at it.
This removes the last usage of mmx from libavutil*.

Old benchmarks:
sad_8x8_0_c:                                            13.2 ( 1.00x)
sad_8x8_0_mmxext:                                       27.8 ( 0.48x)
sad_8x8_1_c:                                            13.2 ( 1.00x)
sad_8x8_1_mmxext:                                       27.6 ( 0.48x)
sad_8x8_2_c:                                            13.3 ( 1.00x)
sad_8x8_2_mmxext:                                       27.6 ( 0.48x)

New benchmarks:
sad_8x8_0_c:                                            13.3 ( 1.00x)
sad_8x8_0_sse2:                                         11.7 ( 1.13x)
sad_8x8_1_c:                                            13.8 ( 1.00x)
sad_8x8_1_sse2:                                         11.6 ( 1.20x)
sad_8x8_2_c:                                            13.2 ( 1.00x)
sad_8x8_2_sse2:                                         11.8 ( 1.12x)

Hint: Using two psadbw or one psadbw and movhps made no difference
in the benchmarks, so I chose the latter due to smaller codesize.

*: except if lavu provides avpriv_emms for other libraries


>From af52750e193c721a6c276bc844ea56e37fe832d3 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <[email protected]>
Date: Sat, 11 Apr 2026 10:58:01 +0200
Subject: [PATCH] avutil/x86/pixelutils: Don't use mmx in 8x8 SAD

This function is exported, so has to abide by the ABI
and therefore issues emms since commit
5b85ca5317f28a73eadf10d604d2a7421b84bed8. Yet this is
expensive and using SSE2 instead improves performance.
Also avoid the initial zeroing and the last pointer
increment while just at it.
This removes the last usage of mmx from libavutil*.

Old benchmarks:
sad_8x8_0_c:                                            13.2 ( 1.00x)
sad_8x8_0_mmxext:                                       27.8 ( 0.48x)
sad_8x8_1_c:                                            13.2 ( 1.00x)
sad_8x8_1_mmxext:                                       27.6 ( 0.48x)
sad_8x8_2_c:                                            13.3 ( 1.00x)
sad_8x8_2_mmxext:                                       27.6 ( 0.48x)

New benchmarks:
sad_8x8_0_c:                                            13.3 ( 1.00x)
sad_8x8_0_sse2:                                         11.7 ( 1.13x)
sad_8x8_1_c:                                            13.8 ( 1.00x)
sad_8x8_1_sse2:                                         11.6 ( 1.20x)
sad_8x8_2_c:                                            13.2 ( 1.00x)
sad_8x8_2_sse2:                                         11.8 ( 1.12x)

Hint: Using two psadbw or one psadbw and movhps made no difference
in the benchmarks, so I chose the latter due to smaller codesize.

*: except if lavu provides avpriv_emms for other libraries

Signed-off-by: Andreas Rheinhardt <[email protected]>
---
 libavutil/x86/pixelutils.asm | 43 ++++++++++++++++++++++--------------
 libavutil/x86/pixelutils.h   | 15 +++----------
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/libavutil/x86/pixelutils.asm b/libavutil/x86/pixelutils.asm
index a80202ef75..264b92cfcb 100644
--- a/libavutil/x86/pixelutils.asm
+++ b/libavutil/x86/pixelutils.asm
@@ -25,25 +25,34 @@
 
 SECTION .text
 
+%macro SAD_XMM_8x2 2
+    movq       %1, [src1q]
+    movq       m2, [src2q]
+    movhps     %1, [src1q+stride1q]
+    movhps     m2, [src2q+stride2q]
+%ifn %2
+    lea     src1q, [src1q+2*stride1q]
+    lea     src2q, [src2q+2*stride2q]
+%endif
+    psadbw     %1, m2
+%ifnidn %1, m0
+    paddw      m0, %1
+%endif
+%endmacro
+
 
;-------------------------------------------------------------------------------
-; int ff_pixelutils_sad_8x8_mmxext(const uint8_t *src1, ptrdiff_t stride1,
-;                                  const uint8_t *src2, ptrdiff_t stride2);
+; int ff_pixelutils_sad_8x8_sse2(const uint8_t *src1, ptrdiff_t stride1,
+;                                const uint8_t *src2, ptrdiff_t stride2);
 
;-------------------------------------------------------------------------------
-INIT_MMX mmxext
-cglobal pixelutils_sad_8x8, 4,4,0, src1, stride1, src2, stride2
-    pxor        m2, m2
-%rep 4
-    mova        m0, [src1q]
-    mova        m1, [src1q + stride1q]
-    psadbw      m0, [src2q]
-    psadbw      m1, [src2q + stride2q]
-    paddw       m2, m0
-    paddw       m2, m1
-    lea         src1q, [src1q + 2*stride1q]
-    lea         src2q, [src2q + 2*stride2q]
-%endrep
-    movd        eax, m2
-    emms
+INIT_XMM sse2
+cglobal pixelutils_sad_8x8, 4,4,3, src1, stride1, src2, stride2
+    SAD_XMM_8x2 m0, 0
+    SAD_XMM_8x2 m1, 0
+    SAD_XMM_8x2 m1, 0
+    SAD_XMM_8x2 m1, 1
+    movhlps     m1, m0
+    paddw       m0, m1
+    movd       eax, m0
     RET
 
 
;-------------------------------------------------------------------------------
diff --git a/libavutil/x86/pixelutils.h b/libavutil/x86/pixelutils.h
index 20a675f667..5a6998a243 100644
--- a/libavutil/x86/pixelutils.h
+++ b/libavutil/x86/pixelutils.h
@@ -28,8 +28,8 @@
 #include "libavutil/attributes.h"
 #include "libavutil/pixelutils.h"
 
-int ff_pixelutils_sad_8x8_mmxext(const uint8_t *src1, ptrdiff_t stride1,
-                                 const uint8_t *src2, ptrdiff_t stride2);
+int ff_pixelutils_sad_8x8_sse2(const uint8_t *src1, ptrdiff_t stride1,
+                               const uint8_t *src2, ptrdiff_t stride2);
 
 int ff_pixelutils_sad_16x16_sse2(const uint8_t *src1, ptrdiff_t stride1,
                                  const uint8_t *src2, ptrdiff_t stride2);
@@ -52,17 +52,8 @@ static inline av_cold void 
ff_pixelutils_sad_init_x86(av_pixelutils_sad_fn *sad,
 {
     int cpu_flags = av_get_cpu_flags();
 
-    // The best way to use SSE2 would be to do 2 SADs in parallel,
-    // but we'd have to modify the pixelutils API to return SIMD functions.
-
-    // It's probably not faster to shuffle data around
-    // to get two lines of 8 pixels into a single 16byte register,
-    // so just use the MMX 8x8 version even when SSE2 is available.
-    if (EXTERNAL_MMXEXT(cpu_flags)) {
-        sad[2] = ff_pixelutils_sad_8x8_mmxext;
-    }
-
     if (EXTERNAL_SSE2(cpu_flags)) {
+        sad[2] = ff_pixelutils_sad_8x8_sse2;
         switch (aligned) {
         case 0: sad[3] = ff_pixelutils_sad_16x16_sse2;   break; // src1 
unaligned, src2 unaligned
         case 1: sad[3] = ff_pixelutils_sad_u_16x16_sse2; break; // src1   
aligned, src2 unaligned
-- 
2.52.0

_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to