diff mbox series

[FFmpeg-devel] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup. AVX2 version is ready and tested, however, although local tests show a significant speed-up in this function using avx2 swscale code overall slow

Message ID 20201023131716.292761-1-alankelly@google.com
State Superseded
Headers show
Series [FFmpeg-devel] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup. AVX2 version is ready and tested, however, although local tests show a significant speed-up in this function using avx2 swscale code overall slow | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Alan Kelly Oct. 23, 2020, 1:17 p.m. UTC 
Fixed. The wrong step size was used causing a write passed the end of
 the buffer. yuv2yuvX_mmxext is now called if there are any remaining pixels.
---
 libswscale/x86/Makefile     |   1 +
 libswscale/x86/swscale.c    |  75 ++++----------------------
 libswscale/x86/yuv2yuvX.asm | 105 ++++++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 65 deletions(-)
 create mode 100644 libswscale/x86/yuv2yuvX.asm

Comments

James Almer Oct. 23, 2020, 1:42 p.m. UTC | #1 
On 10/23/2020 10:17 AM, Alan Kelly wrote:
>  Fixed. The wrong step size was used causing a write passed the end of
>  the buffer. yuv2yuvX_mmxext is now called if there are any remaining pixels.

Please fix the commit subject (It's too long and contains commentary),
and keep comments about fixes between versions outside of the commit
message body. You can manually place them after the --- below, or in a
separate reply.

> ---
>  libswscale/x86/Makefile     |   1 +
>  libswscale/x86/swscale.c    |  75 ++++----------------------
>  libswscale/x86/yuv2yuvX.asm | 105 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 116 insertions(+), 65 deletions(-)
>  create mode 100644 libswscale/x86/yuv2yuvX.asm
> 
> diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
> index 831d5359aa..bfe383364e 100644
> --- a/libswscale/x86/Makefile
> +++ b/libswscale/x86/Makefile
> @@ -13,3 +13,4 @@ X86ASM-OBJS                     += x86/input.o                          \
>                                     x86/scale.o                          \
>                                     x86/rgb_2_rgb.o                      \
>                                     x86/yuv_2_rgb.o                      \
> +                                   x86/yuv2yuvX.o                       \
> diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
> index 3160fedf04..fec9fa22e0 100644
> --- a/libswscale/x86/swscale.c
> +++ b/libswscale/x86/swscale.c
> @@ -197,80 +197,25 @@ void ff_updateMMXDitherTables(SwsContext *c, int dstY)
>  }
>  
>  #if HAVE_MMXEXT
> +void ff_yuv2yuvX_sse3(const int16_t *filter, int filterSize,
> +                           uint8_t *dest, int dstW,
> +                           const uint8_t *dither, int offset);
> +
>  static void yuv2yuvX_sse3(const int16_t *filter, int filterSize,
>                             const int16_t **src, uint8_t *dest, int dstW,
>                             const uint8_t *dither, int offset)
>  {
> +    int remainder = (dstW % 32);
> +    int pixelsProcessed = dstW - remainder;
>      if(((uintptr_t)dest) & 15){
>          yuv2yuvX_mmxext(filter, filterSize, src, dest, dstW, dither, offset);
>          return;
>      }
> -    filterSize--;
> -#define MAIN_FUNCTION \
> -        "pxor       %%xmm0, %%xmm0 \n\t" \
> -        "punpcklbw  %%xmm0, %%xmm3 \n\t" \
> -        "movd           %4, %%xmm1 \n\t" \
> -        "punpcklwd  %%xmm1, %%xmm1 \n\t" \
> -        "punpckldq  %%xmm1, %%xmm1 \n\t" \
> -        "punpcklqdq %%xmm1, %%xmm1 \n\t" \
> -        "psllw          $3, %%xmm1 \n\t" \
> -        "paddw      %%xmm1, %%xmm3 \n\t" \
> -        "psraw          $4, %%xmm3 \n\t" \
> -        "movdqa     %%xmm3, %%xmm4 \n\t" \
> -        "movdqa     %%xmm3, %%xmm7 \n\t" \
> -        "movl           %3, %%ecx  \n\t" \
> -        "mov                                 %0, %%"FF_REG_d"        \n\t"\
> -        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"     \n\t"\
> -        ".p2align                             4             \n\t" /* FIXME Unroll? */\
> -        "1:                                                 \n\t"\
> -        "movddup                  8(%%"FF_REG_d"), %%xmm0   \n\t" /* filterCoeff */\
> -        "movdqa              (%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm2 \n\t" /* srcData */\
> -        "movdqa            16(%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm5 \n\t" /* srcData */\
> -        "add                                $16, %%"FF_REG_d"        \n\t"\
> -        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"     \n\t"\
> -        "test                         %%"FF_REG_S", %%"FF_REG_S"     \n\t"\
> -        "pmulhw                           %%xmm0, %%xmm2      \n\t"\
> -        "pmulhw                           %%xmm0, %%xmm5      \n\t"\
> -        "paddw                            %%xmm2, %%xmm3      \n\t"\
> -        "paddw                            %%xmm5, %%xmm4      \n\t"\
> -        " jnz                                1b             \n\t"\
> -        "psraw                               $3, %%xmm3      \n\t"\
> -        "psraw                               $3, %%xmm4      \n\t"\
> -        "packuswb                         %%xmm4, %%xmm3      \n\t"\
> -        "movntdq                          %%xmm3, (%1, %%"FF_REG_c") \n\t"\
> -        "add                         $16, %%"FF_REG_c"        \n\t"\
> -        "cmp                          %2, %%"FF_REG_c"        \n\t"\
> -        "movdqa                   %%xmm7, %%xmm3            \n\t" \
> -        "movdqa                   %%xmm7, %%xmm4            \n\t" \
> -        "mov                                 %0, %%"FF_REG_d"        \n\t"\
> -        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"     \n\t"\
> -        "jb                                  1b             \n\t"
> -
> -    if (offset) {
> -        __asm__ volatile(
> -            "movq          %5, %%xmm3  \n\t"
> -            "movdqa    %%xmm3, %%xmm4  \n\t"
> -            "psrlq        $24, %%xmm3  \n\t"
> -            "psllq        $40, %%xmm4  \n\t"
> -            "por       %%xmm4, %%xmm3  \n\t"
> -            MAIN_FUNCTION
> -              :: "g" (filter),
> -              "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m" (offset),
> -              "m"(filterSize), "m"(((uint64_t *) dither)[0])
> -              : XMM_CLOBBERS("%xmm0" , "%xmm1" , "%xmm2" , "%xmm3" , "%xmm4" , "%xmm5" , "%xmm7" ,)
> -                "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
> -              );
> -    } else {
> -        __asm__ volatile(
> -            "movq          %5, %%xmm3   \n\t"
> -            MAIN_FUNCTION
> -              :: "g" (filter),
> -              "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m" (offset),
> -              "m"(filterSize), "m"(((uint64_t *) dither)[0])
> -              : XMM_CLOBBERS("%xmm0" , "%xmm1" , "%xmm2" , "%xmm3" , "%xmm4" , "%xmm5" , "%xmm7" ,)
> -                "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
> -              );
> +    ff_yuv2yuvX_sse3(filter, filterSize - 1, dest - offset, pixelsProcessed + offset, dither, offset);
> +    if(remainder > 0){
> +      yuv2yuvX_mmxext(filter, filterSize, src, dest + pixelsProcessed, remainder, dither, offset + pixelsProcessed);
>      }
> +    return;
>  }
>  #endif
>  
> diff --git a/libswscale/x86/yuv2yuvX.asm b/libswscale/x86/yuv2yuvX.asm
> new file mode 100644
> index 0000000000..84727de599
> --- /dev/null
> +++ b/libswscale/x86/yuv2yuvX.asm
> @@ -0,0 +1,105 @@
> +;******************************************************************************
> +;* x86-optimized yuv2yuvX
> +;* Copyright 2020 Google LLC
> +;*
> +;* This file is part of FFmpeg.
> +;*
> +;* FFmpeg is free software; you can redistribute it and/or
> +;* modify it under the terms of the GNU Lesser General Public
> +;* License as published by the Free Software Foundation; either
> +;* version 2.1 of the License, or (at your option) any later version.
> +;*
> +;* FFmpeg is distributed in the hope that it will be useful,
> +;* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +;* Lesser General Public License for more details.
> +;*
> +;* You should have received a copy of the GNU Lesser General Public
> +;* License along with FFmpeg; if not, write to the Free Software
> +;* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +;******************************************************************************
> +
> +%include "libavutil/x86/x86util.asm"
> +
> +SECTION .text
> +
> +;-----------------------------------------------------------------------------
> +; yuv2yuvX
> +;
> +; void ff_yuv2yuvX_<opt>(const int16_t *filter, int filterSize,
> +;                        uint8_t *dest, int dstW,
> +;                        const uint8_t *dither, int offset);
> +;
> +;-----------------------------------------------------------------------------
> +
> +%macro YUV2YUVX_FUNC 0
> +cglobal yuv2yuvX, 6, 7, 16, filter, rsi, dest, dstW, dither, offset, src
> +%if ARCH_X86_64
> +    movsxd               dstWq, dstWd
> +    movsxd               offsetq, offsetd
> +%endif ; x86-64
> +    movq                 xmm3, [ditherq]
> +    cmp                  offsetd, 0
> +    jz                   .offset
> +
> +    ; offset != 0 path.
> +    psrlq                m5, m3, $18
> +    psllq                m3, m3, $28
> +    por                  m3, m3, m5
> +
> +.offset:
> +%if cpuflag(avx2)
> +    vperm2i128           m3, m3, m3, 0
> +%endif ; avx2
> +%if ARCH_X86_64
> +    movq                 xmm1, rsiq
> +%else
> +    movd                 mm1, rsi

What uses mm1 after this on x86_32?

> +%endif
> +    vpbroadcastw         m1, xmm1

AVX2 instruction being used in the SSE3 version.

> +    pxor                 m0, m0, m0
> +    mov                  rsiq, filterq
> +    mov                  srcq, [rsiq]
> +    punpcklbw            m3, m0
> +    psllw                m1, m1, 3
> +    paddw                m3, m3, m1
> +    psraw                m7, m3, 4
> +.outerloop:
> +    mova                 m4, m7
> +    mova                 m3, m7
> +    mova                 m6, m7
> +    mova                 m1, m7
> +.loop:
> +    vpbroadcastq         m0, [rsiq + 8]

Same.

> +    pmulhw               m2, m0, [srcq + offsetq * 2]
> +    pmulhw               m5, m0, [srcq + offsetq * 2 + mmsize]
> +    paddw                m3, m3, m2
> +    paddw                m4, m4, m5
> +    pmulhw               m2, m0, [srcq + offsetq * 2 + 2 * mmsize]
> +    pmulhw               m5, m0, [srcq + offsetq * 2 + 3 * mmsize]
> +    paddw                m6, m6, m2
> +    paddw                m1, m1, m5
> +    add                  rsiq, $10
> +    mov                  srcq, [rsiq]
> +    test                 srcd, srcd
> +    jnz                  .loop
> +    psraw                m3, m3, 3
> +    psraw                m4, m4, 3
> +    psraw                m6, m6, 3
> +    psraw                m1, m1, 3
> +    packuswb             m3, m3, m4
> +    packuswb             m6, m6, m1
> +    mov                  srcq, [filterq]
> +    movntdq              [destq + offsetq], m3
> +    movntdq              [destq + offsetq + mmsize], m6
> +    add                  offsetq, mmsize * 2
> +    mov                  rsiq, filterq
> +    cmp                  offsetq, dstWq
> +    jb                  .outerloop
> +    REP_RET
> +%endmacro
> +
> +INIT_XMM sse3

The only SSE3 instruction was movddup, and you removed it. I assume the
vpbroadcastq is meant to be it for the non-AVX2 version of the function.

> +YUV2YUVX_FUNC
> +INIT_YMM avx2
> +YUV2YUVX_FUNC
>
Alan Kelly Oct. 27, 2020, 8:57 a.m. UTC | #2 
Thanks for the review, I have made the required changes. As I have changed
the subject the patch is in a new thread.

On Fri, Oct 23, 2020 at 4:10 PM James Almer <jamrial@gmail.com> wrote:

> On 10/23/2020 10:17 AM, Alan Kelly wrote:
> >  Fixed. The wrong step size was used causing a write passed the end of
> >  the buffer. yuv2yuvX_mmxext is now called if there are any remaining
> pixels.
>
> Please fix the commit subject (It's too long and contains commentary),
> and keep comments about fixes between versions outside of the commit
> message body. You can manually place them after the --- below, or in a
> separate reply.
>
> > ---
> >  libswscale/x86/Makefile     |   1 +
> >  libswscale/x86/swscale.c    |  75 ++++----------------------
> >  libswscale/x86/yuv2yuvX.asm | 105 ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 116 insertions(+), 65 deletions(-)
> >  create mode 100644 libswscale/x86/yuv2yuvX.asm
> >
> > diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
> > index 831d5359aa..bfe383364e 100644
> > --- a/libswscale/x86/Makefile
> > +++ b/libswscale/x86/Makefile
> > @@ -13,3 +13,4 @@ X86ASM-OBJS                     += x86/input.o
>                   \
> >                                     x86/scale.o
> \
> >                                     x86/rgb_2_rgb.o
> \
> >                                     x86/yuv_2_rgb.o
> \
> > +                                   x86/yuv2yuvX.o
>  \
> > diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
> > index 3160fedf04..fec9fa22e0 100644
> > --- a/libswscale/x86/swscale.c
> > +++ b/libswscale/x86/swscale.c
> > @@ -197,80 +197,25 @@ void ff_updateMMXDitherTables(SwsContext *c, int
> dstY)
> >  }
> >
> >  #if HAVE_MMXEXT
> > +void ff_yuv2yuvX_sse3(const int16_t *filter, int filterSize,
> > +                           uint8_t *dest, int dstW,
> > +                           const uint8_t *dither, int offset);
> > +
> >  static void yuv2yuvX_sse3(const int16_t *filter, int filterSize,
> >                             const int16_t **src, uint8_t *dest, int dstW,
> >                             const uint8_t *dither, int offset)
> >  {
> > +    int remainder = (dstW % 32);
> > +    int pixelsProcessed = dstW - remainder;
> >      if(((uintptr_t)dest) & 15){
> >          yuv2yuvX_mmxext(filter, filterSize, src, dest, dstW, dither,
> offset);
> >          return;
> >      }
> > -    filterSize--;
> > -#define MAIN_FUNCTION \
> > -        "pxor       %%xmm0, %%xmm0 \n\t" \
> > -        "punpcklbw  %%xmm0, %%xmm3 \n\t" \
> > -        "movd           %4, %%xmm1 \n\t" \
> > -        "punpcklwd  %%xmm1, %%xmm1 \n\t" \
> > -        "punpckldq  %%xmm1, %%xmm1 \n\t" \
> > -        "punpcklqdq %%xmm1, %%xmm1 \n\t" \
> > -        "psllw          $3, %%xmm1 \n\t" \
> > -        "paddw      %%xmm1, %%xmm3 \n\t" \
> > -        "psraw          $4, %%xmm3 \n\t" \
> > -        "movdqa     %%xmm3, %%xmm4 \n\t" \
> > -        "movdqa     %%xmm3, %%xmm7 \n\t" \
> > -        "movl           %3, %%ecx  \n\t" \
> > -        "mov                                 %0, %%"FF_REG_d"
> \n\t"\
> > -        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"
>  \n\t"\
> > -        ".p2align                             4             \n\t" /*
> FIXME Unroll? */\
> > -        "1:                                                 \n\t"\
> > -        "movddup                  8(%%"FF_REG_d"), %%xmm0   \n\t" /*
> filterCoeff */\
> > -        "movdqa              (%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm2
> \n\t" /* srcData */\
> > -        "movdqa            16(%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm5
> \n\t" /* srcData */\
> > -        "add                                $16, %%"FF_REG_d"
> \n\t"\
> > -        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"
>  \n\t"\
> > -        "test                         %%"FF_REG_S", %%"FF_REG_S"
>  \n\t"\
> > -        "pmulhw                           %%xmm0, %%xmm2      \n\t"\
> > -        "pmulhw                           %%xmm0, %%xmm5      \n\t"\
> > -        "paddw                            %%xmm2, %%xmm3      \n\t"\
> > -        "paddw                            %%xmm5, %%xmm4      \n\t"\
> > -        " jnz                                1b             \n\t"\
> > -        "psraw                               $3, %%xmm3      \n\t"\
> > -        "psraw                               $3, %%xmm4      \n\t"\
> > -        "packuswb                         %%xmm4, %%xmm3      \n\t"\
> > -        "movntdq                          %%xmm3, (%1, %%"FF_REG_c")
> \n\t"\
> > -        "add                         $16, %%"FF_REG_c"        \n\t"\
> > -        "cmp                          %2, %%"FF_REG_c"        \n\t"\
> > -        "movdqa                   %%xmm7, %%xmm3            \n\t" \
> > -        "movdqa                   %%xmm7, %%xmm4            \n\t" \
> > -        "mov                                 %0, %%"FF_REG_d"
> \n\t"\
> > -        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"
>  \n\t"\
> > -        "jb                                  1b             \n\t"
> > -
> > -    if (offset) {
> > -        __asm__ volatile(
> > -            "movq          %5, %%xmm3  \n\t"
> > -            "movdqa    %%xmm3, %%xmm4  \n\t"
> > -            "psrlq        $24, %%xmm3  \n\t"
> > -            "psllq        $40, %%xmm4  \n\t"
> > -            "por       %%xmm4, %%xmm3  \n\t"
> > -            MAIN_FUNCTION
> > -              :: "g" (filter),
> > -              "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m"
> (offset),
> > -              "m"(filterSize), "m"(((uint64_t *) dither)[0])
> > -              : XMM_CLOBBERS("%xmm0" , "%xmm1" , "%xmm2" , "%xmm3" ,
> "%xmm4" , "%xmm5" , "%xmm7" ,)
> > -                "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
> > -              );
> > -    } else {
> > -        __asm__ volatile(
> > -            "movq          %5, %%xmm3   \n\t"
> > -            MAIN_FUNCTION
> > -              :: "g" (filter),
> > -              "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m"
> (offset),
> > -              "m"(filterSize), "m"(((uint64_t *) dither)[0])
> > -              : XMM_CLOBBERS("%xmm0" , "%xmm1" , "%xmm2" , "%xmm3" ,
> "%xmm4" , "%xmm5" , "%xmm7" ,)
> > -                "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
> > -              );
> > +    ff_yuv2yuvX_sse3(filter, filterSize - 1, dest - offset,
> pixelsProcessed + offset, dither, offset);
> > +    if(remainder > 0){
> > +      yuv2yuvX_mmxext(filter, filterSize, src, dest + pixelsProcessed,
> remainder, dither, offset + pixelsProcessed);
> >      }
> > +    return;
> >  }
> >  #endif
> >
> > diff --git a/libswscale/x86/yuv2yuvX.asm b/libswscale/x86/yuv2yuvX.asm
> > new file mode 100644
> > index 0000000000..84727de599
> > --- /dev/null
> > +++ b/libswscale/x86/yuv2yuvX.asm
> > @@ -0,0 +1,105 @@
> >
> +;******************************************************************************
> > +;* x86-optimized yuv2yuvX
> > +;* Copyright 2020 Google LLC
> > +;*
> > +;* This file is part of FFmpeg.
> > +;*
> > +;* FFmpeg is free software; you can redistribute it and/or
> > +;* modify it under the terms of the GNU Lesser General Public
> > +;* License as published by the Free Software Foundation; either
> > +;* version 2.1 of the License, or (at your option) any later version.
> > +;*
> > +;* FFmpeg is distributed in the hope that it will be useful,
> > +;* but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +;* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +;* Lesser General Public License for more details.
> > +;*
> > +;* You should have received a copy of the GNU Lesser General Public
> > +;* License along with FFmpeg; if not, write to the Free Software
> > +;* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> >
> +;******************************************************************************
> > +
> > +%include "libavutil/x86/x86util.asm"
> > +
> > +SECTION .text
> > +
> >
> +;-----------------------------------------------------------------------------
> > +; yuv2yuvX
> > +;
> > +; void ff_yuv2yuvX_<opt>(const int16_t *filter, int filterSize,
> > +;                        uint8_t *dest, int dstW,
> > +;                        const uint8_t *dither, int offset);
> > +;
> >
> +;-----------------------------------------------------------------------------
> > +
> > +%macro YUV2YUVX_FUNC 0
> > +cglobal yuv2yuvX, 6, 7, 16, filter, rsi, dest, dstW, dither, offset, src
> > +%if ARCH_X86_64
> > +    movsxd               dstWq, dstWd
> > +    movsxd               offsetq, offsetd
> > +%endif ; x86-64
> > +    movq                 xmm3, [ditherq]
> > +    cmp                  offsetd, 0
> > +    jz                   .offset
> > +
> > +    ; offset != 0 path.
> > +    psrlq                m5, m3, $18
> > +    psllq                m3, m3, $28
> > +    por                  m3, m3, m5
> > +
> > +.offset:
> > +%if cpuflag(avx2)
> > +    vperm2i128           m3, m3, m3, 0
> > +%endif ; avx2
> > +%if ARCH_X86_64
> > +    movq                 xmm1, rsiq
> > +%else
> > +    movd                 mm1, rsi
>
> What uses mm1 after this on x86_32?
>
> > +%endif
> > +    vpbroadcastw         m1, xmm1
>
> AVX2 instruction being used in the SSE3 version.
>
> > +    pxor                 m0, m0, m0
> > +    mov                  rsiq, filterq
> > +    mov                  srcq, [rsiq]
> > +    punpcklbw            m3, m0
> > +    psllw                m1, m1, 3
> > +    paddw                m3, m3, m1
> > +    psraw                m7, m3, 4
> > +.outerloop:
> > +    mova                 m4, m7
> > +    mova                 m3, m7
> > +    mova                 m6, m7
> > +    mova                 m1, m7
> > +.loop:
> > +    vpbroadcastq         m0, [rsiq + 8]
>
> Same.
>
> > +    pmulhw               m2, m0, [srcq + offsetq * 2]
> > +    pmulhw               m5, m0, [srcq + offsetq * 2 + mmsize]
> > +    paddw                m3, m3, m2
> > +    paddw                m4, m4, m5
> > +    pmulhw               m2, m0, [srcq + offsetq * 2 + 2 * mmsize]
> > +    pmulhw               m5, m0, [srcq + offsetq * 2 + 3 * mmsize]
> > +    paddw                m6, m6, m2
> > +    paddw                m1, m1, m5
> > +    add                  rsiq, $10
> > +    mov                  srcq, [rsiq]
> > +    test                 srcd, srcd
> > +    jnz                  .loop
> > +    psraw                m3, m3, 3
> > +    psraw                m4, m4, 3
> > +    psraw                m6, m6, 3
> > +    psraw                m1, m1, 3
> > +    packuswb             m3, m3, m4
> > +    packuswb             m6, m6, m1
> > +    mov                  srcq, [filterq]
> > +    movntdq              [destq + offsetq], m3
> > +    movntdq              [destq + offsetq + mmsize], m6
> > +    add                  offsetq, mmsize * 2
> > +    mov                  rsiq, filterq
> > +    cmp                  offsetq, dstWq
> > +    jb                  .outerloop
> > +    REP_RET
> > +%endmacro
> > +
> > +INIT_XMM sse3
>
> The only SSE3 instruction was movddup, and you removed it. I assume the
> vpbroadcastq is meant to be it for the non-AVX2 version of the function.
>
> > +YUV2YUVX_FUNC
> > +INIT_YMM avx2
> > +YUV2YUVX_FUNC
> >
>
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
index 831d5359aa..bfe383364e 100644
--- a/libswscale/x86/Makefile
+++ b/libswscale/x86/Makefile
@@ -13,3 +13,4 @@  X86ASM-OBJS                     += x86/input.o                          \
                                    x86/scale.o                          \
                                    x86/rgb_2_rgb.o                      \
                                    x86/yuv_2_rgb.o                      \
+                                   x86/yuv2yuvX.o                       \
diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
index 3160fedf04..fec9fa22e0 100644
--- a/libswscale/x86/swscale.c
+++ b/libswscale/x86/swscale.c
@@ -197,80 +197,25 @@  void ff_updateMMXDitherTables(SwsContext *c, int dstY)
 }
 
 #if HAVE_MMXEXT
+void ff_yuv2yuvX_sse3(const int16_t *filter, int filterSize,
+                           uint8_t *dest, int dstW,
+                           const uint8_t *dither, int offset);
+
 static void yuv2yuvX_sse3(const int16_t *filter, int filterSize,
                            const int16_t **src, uint8_t *dest, int dstW,
                            const uint8_t *dither, int offset)
 {
+    int remainder = (dstW % 32);
+    int pixelsProcessed = dstW - remainder;
     if(((uintptr_t)dest) & 15){
         yuv2yuvX_mmxext(filter, filterSize, src, dest, dstW, dither, offset);
         return;
     }
-    filterSize--;
-#define MAIN_FUNCTION \
-        "pxor       %%xmm0, %%xmm0 \n\t" \
-        "punpcklbw  %%xmm0, %%xmm3 \n\t" \
-        "movd           %4, %%xmm1 \n\t" \
-        "punpcklwd  %%xmm1, %%xmm1 \n\t" \
-        "punpckldq  %%xmm1, %%xmm1 \n\t" \
-        "punpcklqdq %%xmm1, %%xmm1 \n\t" \
-        "psllw          $3, %%xmm1 \n\t" \
-        "paddw      %%xmm1, %%xmm3 \n\t" \
-        "psraw          $4, %%xmm3 \n\t" \
-        "movdqa     %%xmm3, %%xmm4 \n\t" \
-        "movdqa     %%xmm3, %%xmm7 \n\t" \
-        "movl           %3, %%ecx  \n\t" \
-        "mov                                 %0, %%"FF_REG_d"        \n\t"\
-        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"     \n\t"\
-        ".p2align                             4             \n\t" /* FIXME Unroll? */\
-        "1:                                                 \n\t"\
-        "movddup                  8(%%"FF_REG_d"), %%xmm0   \n\t" /* filterCoeff */\
-        "movdqa              (%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm2 \n\t" /* srcData */\
-        "movdqa            16(%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm5 \n\t" /* srcData */\
-        "add                                $16, %%"FF_REG_d"        \n\t"\
-        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"     \n\t"\
-        "test                         %%"FF_REG_S", %%"FF_REG_S"     \n\t"\
-        "pmulhw                           %%xmm0, %%xmm2      \n\t"\
-        "pmulhw                           %%xmm0, %%xmm5      \n\t"\
-        "paddw                            %%xmm2, %%xmm3      \n\t"\
-        "paddw                            %%xmm5, %%xmm4      \n\t"\
-        " jnz                                1b             \n\t"\
-        "psraw                               $3, %%xmm3      \n\t"\
-        "psraw                               $3, %%xmm4      \n\t"\
-        "packuswb                         %%xmm4, %%xmm3      \n\t"\
-        "movntdq                          %%xmm3, (%1, %%"FF_REG_c") \n\t"\
-        "add                         $16, %%"FF_REG_c"        \n\t"\
-        "cmp                          %2, %%"FF_REG_c"        \n\t"\
-        "movdqa                   %%xmm7, %%xmm3            \n\t" \
-        "movdqa                   %%xmm7, %%xmm4            \n\t" \
-        "mov                                 %0, %%"FF_REG_d"        \n\t"\
-        "mov                        (%%"FF_REG_d"), %%"FF_REG_S"     \n\t"\
-        "jb                                  1b             \n\t"
-
-    if (offset) {
-        __asm__ volatile(
-            "movq          %5, %%xmm3  \n\t"
-            "movdqa    %%xmm3, %%xmm4  \n\t"
-            "psrlq        $24, %%xmm3  \n\t"
-            "psllq        $40, %%xmm4  \n\t"
-            "por       %%xmm4, %%xmm3  \n\t"
-            MAIN_FUNCTION
-              :: "g" (filter),
-              "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m" (offset),
-              "m"(filterSize), "m"(((uint64_t *) dither)[0])
-              : XMM_CLOBBERS("%xmm0" , "%xmm1" , "%xmm2" , "%xmm3" , "%xmm4" , "%xmm5" , "%xmm7" ,)
-                "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
-              );
-    } else {
-        __asm__ volatile(
-            "movq          %5, %%xmm3   \n\t"
-            MAIN_FUNCTION
-              :: "g" (filter),
-              "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m" (offset),
-              "m"(filterSize), "m"(((uint64_t *) dither)[0])
-              : XMM_CLOBBERS("%xmm0" , "%xmm1" , "%xmm2" , "%xmm3" , "%xmm4" , "%xmm5" , "%xmm7" ,)
-                "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
-              );
+    ff_yuv2yuvX_sse3(filter, filterSize - 1, dest - offset, pixelsProcessed + offset, dither, offset);
+    if(remainder > 0){
+      yuv2yuvX_mmxext(filter, filterSize, src, dest + pixelsProcessed, remainder, dither, offset + pixelsProcessed);
     }
+    return;
 }
 #endif
 
diff --git a/libswscale/x86/yuv2yuvX.asm b/libswscale/x86/yuv2yuvX.asm
new file mode 100644
index 0000000000..84727de599
--- /dev/null
+++ b/libswscale/x86/yuv2yuvX.asm
@@ -0,0 +1,105 @@ 
+;******************************************************************************
+;* x86-optimized yuv2yuvX
+;* Copyright 2020 Google LLC
+;*
+;* This file is part of FFmpeg.
+;*
+;* FFmpeg is free software; you can redistribute it and/or
+;* modify it under the terms of the GNU Lesser General Public
+;* License as published by the Free Software Foundation; either
+;* version 2.1 of the License, or (at your option) any later version.
+;*
+;* FFmpeg is distributed in the hope that it will be useful,
+;* but WITHOUT ANY WARRANTY; without even the implied warranty of
+;* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;* Lesser General Public License for more details.
+;*
+;* You should have received a copy of the GNU Lesser General Public
+;* License along with FFmpeg; if not, write to the Free Software
+;* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+;******************************************************************************
+
+%include "libavutil/x86/x86util.asm"
+
+SECTION .text
+
+;-----------------------------------------------------------------------------
+; yuv2yuvX
+;
+; void ff_yuv2yuvX_<opt>(const int16_t *filter, int filterSize,
+;                        uint8_t *dest, int dstW,
+;                        const uint8_t *dither, int offset);
+;
+;-----------------------------------------------------------------------------
+
+%macro YUV2YUVX_FUNC 0
+cglobal yuv2yuvX, 6, 7, 16, filter, rsi, dest, dstW, dither, offset, src
+%if ARCH_X86_64
+    movsxd               dstWq, dstWd
+    movsxd               offsetq, offsetd
+%endif ; x86-64
+    movq                 xmm3, [ditherq]
+    cmp                  offsetd, 0
+    jz                   .offset
+
+    ; offset != 0 path.
+    psrlq                m5, m3, $18
+    psllq                m3, m3, $28
+    por                  m3, m3, m5
+
+.offset:
+%if cpuflag(avx2)
+    vperm2i128           m3, m3, m3, 0
+%endif ; avx2
+%if ARCH_X86_64
+    movq                 xmm1, rsiq
+%else
+    movd                 mm1, rsi
+%endif
+    vpbroadcastw         m1, xmm1
+    pxor                 m0, m0, m0
+    mov                  rsiq, filterq
+    mov                  srcq, [rsiq]
+    punpcklbw            m3, m0
+    psllw                m1, m1, 3
+    paddw                m3, m3, m1
+    psraw                m7, m3, 4
+.outerloop:
+    mova                 m4, m7
+    mova                 m3, m7
+    mova                 m6, m7
+    mova                 m1, m7
+.loop:
+    vpbroadcastq         m0, [rsiq + 8]
+    pmulhw               m2, m0, [srcq + offsetq * 2]
+    pmulhw               m5, m0, [srcq + offsetq * 2 + mmsize]
+    paddw                m3, m3, m2
+    paddw                m4, m4, m5
+    pmulhw               m2, m0, [srcq + offsetq * 2 + 2 * mmsize]
+    pmulhw               m5, m0, [srcq + offsetq * 2 + 3 * mmsize]
+    paddw                m6, m6, m2
+    paddw                m1, m1, m5
+    add                  rsiq, $10
+    mov                  srcq, [rsiq]
+    test                 srcd, srcd
+    jnz                  .loop
+    psraw                m3, m3, 3
+    psraw                m4, m4, 3
+    psraw                m6, m6, 3
+    psraw                m1, m1, 3
+    packuswb             m3, m3, m4
+    packuswb             m6, m6, m1
+    mov                  srcq, [filterq]
+    movntdq              [destq + offsetq], m3
+    movntdq              [destq + offsetq + mmsize], m6
+    add                  offsetq, mmsize * 2
+    mov                  rsiq, filterq
+    cmp                  offsetq, dstWq
+    jb                  .outerloop
+    REP_RET
+%endmacro
+
+INIT_XMM sse3
+YUV2YUVX_FUNC
+INIT_YMM avx2
+YUV2YUVX_FUNC