On Wed, 15 Apr 2026, 12:15 mkver via ffmpeg-devel, <[email protected]> wrote:
> 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] LGTM and congratulations on removing MMX from avutil > > _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
