On 2016-10-19 22:18:43 +0300, Martin Storsjö wrote:
> This work is sponsored by, and copyright, Google.
> 
> The filter coefficients are signed values, where the product of the
> multiplication with one individual filter coefficient doesn't
> overflow a 16 bit signed value (the largest filter coefficient is
> 127). But when the products are accumulated, the resulting sum can
> overflow the 16 bit signed range. Instead of accumulating in 32 bit,
> we accumulate all filter taps but the largest one in one register, and
> the largest one (either index 3 or 4) in a separate one, added with
> saturation afterwards.

"Instead of ..., we accumulate the largest product (either index 3 or 4)
last with a saturated addition."

Is shorter and easier to understand.

...

> ---
>  libavcodec/arm/Makefile          |   2 +
>  libavcodec/arm/vp9dsp_init_arm.c | 140 +++++++
>  libavcodec/arm/vp9mc_neon.S      | 787 
> +++++++++++++++++++++++++++++++++++++++
>  libavcodec/vp9.h                 |   4 +-
>  libavcodec/vp9block.c            |  10 +-
>  libavcodec/vp9dsp.c              |   2 +
>  6 files changed, 942 insertions(+), 3 deletions(-)
>  create mode 100644 libavcodec/arm/vp9dsp_init_arm.c
>  create mode 100644 libavcodec/arm/vp9mc_neon.S
> 
> diff --git a/libavcodec/arm/Makefile b/libavcodec/arm/Makefile
> index bd4dd4e..2638230 100644
> --- a/libavcodec/arm/Makefile
> +++ b/libavcodec/arm/Makefile
> @@ -45,6 +45,7 @@ OBJS-$(CONFIG_MLP_DECODER)             += 
> arm/mlpdsp_init_arm.o
>  OBJS-$(CONFIG_RV40_DECODER)            += arm/rv40dsp_init_arm.o
>  OBJS-$(CONFIG_VORBIS_DECODER)          += arm/vorbisdsp_init_arm.o
>  OBJS-$(CONFIG_VP6_DECODER)             += arm/vp6dsp_init_arm.o
> +OBJS-$(CONFIG_VP9_DECODER)             += arm/vp9dsp_init_arm.o
>  
>  
>  # ARMv5 optimizations
> @@ -138,3 +139,4 @@ NEON-OBJS-$(CONFIG_RV40_DECODER)       += 
> arm/rv34dsp_neon.o            \
>                                            arm/rv40dsp_neon.o
>  NEON-OBJS-$(CONFIG_VORBIS_DECODER)     += arm/vorbisdsp_neon.o
>  NEON-OBJS-$(CONFIG_VP6_DECODER)        += arm/vp6dsp_neon.o
> +NEON-OBJS-$(CONFIG_VP9_DECODER)        += arm/vp9mc_neon.o
> diff --git a/libavcodec/arm/vp9dsp_init_arm.c 
> b/libavcodec/arm/vp9dsp_init_arm.c
> new file mode 100644
> index 0000000..db8c683
> --- /dev/null
> +++ b/libavcodec/arm/vp9dsp_init_arm.c
> @@ -0,0 +1,140 @@
> +/*
> + * Copyright (c) 2016 Google Inc.
> + *
> + * This file is part of Libav.
> + *
> + * Libav 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.
> + *
> + * Libav 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 Libav; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +#include <stdint.h>
> +
> +#include "libavutil/attributes.h"
> +#include "libavutil/arm/cpu.h"
> +#include "libavcodec/vp9.h"
> +
> +#define declare_fpel(type, sz)                                          \
> +void ff_vp9_##type##sz##_neon(uint8_t *dst, ptrdiff_t dst_stride,       \
> +                              const uint8_t *src, ptrdiff_t src_stride, \
> +                              int h, int mx, int my)
> +
> +#define declare_copy_avg(sz) \
> +    declare_fpel(copy, sz);  \
> +    declare_fpel(avg , sz)
> +
> +#define decl_mc_func(op, filter, dir, sz)                                    
>             \
> +void ff_vp9_##op##_##filter##sz##_##dir##_neon(uint8_t *dst, ptrdiff_t 
> dst_stride,       \
> +                                               const uint8_t *src, ptrdiff_t 
> src_stride, \
> +                                               int h, int mx, int my)
> +
> +#define define_8tap_2d_fn(op, filter, sz)                                    
>      \
> +static void op##_##filter##sz##_hv_neon(uint8_t *dst, ptrdiff_t dst_stride,  
>      \
> +                                        const uint8_t *src, ptrdiff_t 
> src_stride, \
> +                                        int h, int mx, int my)               
>      \
> +{                                                                            
>      \
> +    LOCAL_ALIGNED_16(uint8_t, temp, [72 * 64]);                              
>      \

Is there a reason why the temporary array doesn't depend on the size?

> +    /* We only need h + 7 lines, but the horizontal filter assumes an        
>      \
> +     * even number of rows, so filter h + 8 lines here. */                   
>      \
> +    ff_vp9_put_##filter##sz##_h_neon(temp, 64,                               
>      \
> +                                     src - 3 * src_stride, src_stride,       
>      \
> +                                     h + 8, mx, 0);                          
>      \
> +    ff_vp9_##op##_##filter##sz##_v_neon(dst, dst_stride,                     
>      \
> +                                        temp + 3 * 64, 64,                   
>      \
> +                                        h, 0, my);                           
>      \

I guess you haven't tried implementing the diagonal mc fully in asm?
Probably not worth the effort since the only significant gain comes from
keeping the temporary values in registers which works on size 4 and 8 and
maybe 16 for aarch64 wich are probably not that important.

...

> diff --git a/libavcodec/arm/vp9mc_neon.S b/libavcodec/arm/vp9mc_neon.S
> new file mode 100644
> index 0000000..754ac50
> --- /dev/null
> +++ b/libavcodec/arm/vp9mc_neon.S
> @@ -0,0 +1,787 @@

...

> +@ All public functions in this file have the following signature:
> +@ typedef void (*vp9_mc_func)(uint8_t *dst, ptrdiff_t dst_stride,
> +@                            const uint8_t *ref, ptrdiff_t ref_stride,
> +@                            int h, int mx, int my);
> +
> +function ff_vp9_copy64_neon, export=1
> +        ldr             r12, [sp]
> +        sub             r1,  r1,  #48
> +        sub             r3,  r3,  #48
> +1:
> +        vld1.8          {q0},  [r2]!
> +        vld1.8          {q1},  [r2]!
> +        vst1.8          {q0},  [r0, :128]!
> +        vld1.8          {q2},  [r2]!
> +        vst1.8          {q1},  [r0, :128]!
> +        vld1.8          {q3},  [r2], r3
> +        vst1.8          {q2},  [r0, :128]!
> +        subs            r12, r12, #1
> +        vst1.8          {q3},  [r0, :128], r1
> +        bne             1b
> +        bx              lr
> +endfunc
> +
> +function ff_vp9_avg64_neon, export=1
> +        ldr             r12, [sp]
> +        sub             r1,  r1,  #48
> +        sub             r3,  r3,  #48
> +1:
> +        vld1.8          {q8},  [r2]!
> +        vld1.8          {q0},  [r0, :128]!
> +        vld1.8          {q9},  [r2]!
> +        vld1.8          {q1},  [r0, :128]!
> +        vrhadd.u8       q0,  q0,  q8
> +        vld1.8          {q10}, [r2]!
> +        vld1.8          {q2},  [r0, :128]!
> +        vrhadd.u8       q1,  q1,  q9
> +        vld1.8          {q11}, [r2], r3
> +        vld1.8          {q3},  [r0, :128]
> +        vrhadd.u8       q2,  q2,  q10
> +        sub             r0,  r0,  #48

using an additional register for writing to dst proabably makes sense

> +        vrhadd.u8       q3,  q3,  q11
> +        vst1.8          {q0},  [r0, :128]!
> +        vst1.8          {q1},  [r0, :128]!
> +        vst1.8          {q2},  [r0, :128]!
> +        vst1.8          {q3},  [r0, :128], r1
> +        subs            r12, r12, #1
> +        bne             1b
> +        bx              lr
> +endfunc

Have you tried 4 d-register loads/stores? If it's not slower it's
preferable since it uses less instructions. Same for size 32 with the
additional advantange of not having to adjust the stride

> +
> +function ff_vp9_copy32_neon, export=1
> +        ldr             r12, [sp]
> +        sub             r1,  r1,  #16
> +        sub             r3,  r3,  #16
> +1:
> +        vld1.8          {q0},  [r2]!
> +        vst1.8          {q0},  [r0, :128]!
> +        subs            r12, r12, #1
> +        vld1.8          {q1},  [r2], r3
> +        vst1.8          {q1},  [r0, :128], r1
> +        bne             1b
> +        bx              lr
> +endfunc
> +
> +function ff_vp9_avg32_neon, export=1
> +        ldr             r12, [sp]
> +        sub             r1,  r1,  #16
> +        sub             r3,  r3,  #16
> +1:
> +        vld1.8          {q2},  [r2]!
> +        vld1.8          {q0},  [r0, :128]!
> +        vld1.8          {q3},  [r2], r3
> +        vrhadd.u8       q0,  q0,  q2
> +        vld1.8          {q1},  [r0, :128]
> +        vrhadd.u8       q1,  q1,  q3
> +        sub             r0,  r0,  #16

32-byte load for dst will leave r0 unmodified

> +        vst1.8          {q0},  [r0, :128]!
> +        vst1.8          {q1},  [r0, :128], r1
> +        subs            r12, r12, #1
> +        bne             1b
> +        bx              lr
> +endfunc
> +
> +function ff_vp9_copy16_neon, export=1
> +        ldr             r12, [sp]
> +        push            {r4-r5}
> +        add             r4,  r0,  r1
> +        add             r5,  r2,  r3
> +        add             r1,  r1,  r1
> +        add             r3,  r3,  r3
> +1:
> +        vld1.8          {q0},  [r2], r3
> +        vld1.8          {q1},  [r5], r3
> +        subs            r12, r12, #2
> +        vst1.8          {q0},  [r0, :128], r1
> +        vst1.8          {q1},  [r4, :128], r1
> +        bne             1b
> +        pop             {r4-r5}
> +        bx              lr
> +endfunc
> +
> +function ff_vp9_avg16_neon, export=1
> +        ldr             r12, [sp]
> +1:
> +        vld1.8          {q2},  [r2], r3
> +        vld1.8          {q0},  [r0, :128], r1
> +        vld1.8          {q3},  [r2], r3
> +        vrhadd.u8       q0,  q0,  q2
> +        vld1.8          {q1},  [r0, :128]
> +        sub             r0,  r0,  r1

have you tried storing q0 with writeback and then loading q1?

> +        vrhadd.u8       q1,  q1,  q3
> +        subs            r12, r12, #2
> +        vst1.8          {q0},  [r0, :128], r1
> +        vst1.8          {q1},  [r0, :128], r1
> +        bne             1b
> +        bx              lr
> +endfunc
> +
> +function ff_vp9_copy8_neon, export=1
> +        ldr             r12, [sp]
> +1:
> +        vld1.8          {d0},  [r2], r3
> +        vld1.8          {d1},  [r2], r3
> +        subs            r12, r12, #2
> +        vst1.8          {d0},  [r0, :64], r1
> +        vst1.8          {d1},  [r0, :64], r1
> +        bne             1b
> +        bx              lr
> +endfunc
> +
> +function ff_vp9_avg8_neon, export=1
> +        ldr             r12, [sp]
> +1:
> +        vld1.8          {d2},  [r2], r3
> +        vld1.8          {d0},  [r0, :64], r1
> +        vld1.8          {d3},  [r2], r3
> +        vrhadd.u8       d0,  d0,  d2
> +        vld1.8          {d1},  [r0, :64]
> +        sub             r0,  r0,  r1

same as 16 although it's probably silly for size 8, using an additional
register would the other possibility to speed it up

> +        vrhadd.u8       d1,  d1,  d3
> +        subs            r12, r12, #2
> +        vst1.8          {d0},  [r0, :64], r1
> +        vst1.8          {d1},  [r0, :64], r1
> +        bne             1b
> +        bx              lr
> +endfunc
> +
> +function ff_vp9_copy4_neon, export=1
> +        ldr             r12, [sp]
> +1:
> +        vld1.32         {d0[]},   [r2], r3
> +        vld1.32         {d1[]},   [r2], r3
> +        vst1.32         {d0[0]},  [r0, :32], r1
> +        vld1.32         {d2[]},   [r2], r3
> +        vst1.32         {d1[0]},  [r0, :32], r1
> +        vld1.32         {d3[]},   [r2], r3
> +        subs            r12, r12, #4
> +        vst1.32         {d2[0]},  [r0, :32], r1
> +        vst1.32         {d3[0]},  [r0, :32], r1
> +        bne             1b
> +        bx              lr
> +endfunc
> +
> +function ff_vp9_avg4_neon, export=1
> +        ldr             r12, [sp]
> +1:
> +        vld1.32         {d4[]},   [r2], r3
> +        vld1.32         {d0[]},   [r0, :32], r1
> +        vld1.32         {d5[]},   [r2], r3
> +        vrhadd.u8       d0,  d0,  d4
> +        vld1.32         {d1[]},   [r0, :32], r1
> +        vld1.32         {d6[]},   [r2], r3
> +        vrhadd.u8       d1,  d1,  d5
> +        vld1.32         {d2[]},   [r0, :32], r1
> +        vld1.32         {d7[]},   [r2], r3
> +        vrhadd.u8       d2,  d2,  d6
> +        vld1.32         {d3[]},   [r0, :32], r1
> +        sub             r0,  r0,  r1, lsl #2
> +        subs            r12, r12, #4
> +        vst1.32         {d0[0]},  [r0, :32], r1
> +        vrhadd.u8       d3,  d3,  d7
> +        vst1.32         {d1[0]},  [r0, :32], r1
> +        vst1.32         {d2[0]},  [r0, :32], r1
> +        vst1.32         {d3[0]},  [r0, :32], r1
> +        bne             1b
> +        bx              lr
> +endfunc

...

> +@ Instantiate a horizontal filter function for the given size.
> +@ This can work on 4, 8 or 16 pixels in parallel; for larger
> +@ widths it will do 16 pixels at a time and loop horizontally.
> +@ The actual width is passed in r12, and the height in r4.
> +@ idx2 is the index of the largest filter coefficient (3 or 4)
> +@ and idx1 is the other one of them.
> +.macro do_8tap_h type, size, idx1, idx2
> +function \type\()_8tap_\size\()h_\idx1\idx2
> +        sub             r2,  r2,  #3
> +        add             r6,  r0,  r1
> +        add             r7,  r2,  r3
> +        add             r1,  r1,  r1
> +        add             r3,  r3,  r3
> +        @ Only size > 16 loops horizontally and needs
> +        @ reduced dst stride
> +.if \size > 16
> +        sub             r1,  r1,  r12
> +.endif
> +        @ size >= 16 loads two qwords and increments r2,
> +        @ for size 4/8 it's enough with one qword and no
> +        @ postincrement
> +.if \size >= 16
> +        sub             r3,  r3,  r12
> +        sub             r3,  r3,  #8
> +.endif
> +        @ Load the filter vector
> +        vld1.16         {q0},  [r5,:128]
> +1:
> +.if \size > 16
> +        mov             r5,  r12
> +.endif
> +        @ Load src
> +.if \size >= 16
> +        vld1.8          {q8},  [r2]!
> +        vld1.8          {q11}, [r7]!
> +        vld1.8          {d20}, [r2]!
> +        vld1.8          {d26}, [r7]!
> +.else
> +        vld1.8          {q8},  [r2]
> +        vld1.8          {q11}, [r7]
> +.endif
> +        vmovl.u8        q9,  d17
> +        vmovl.u8        q8,  d16
> +        vmovl.u8        q12, d23
> +        vmovl.u8        q11, d22
> +.if \size >= 16
> +        vmovl.u8        q10, d20
> +        vmovl.u8        q13, d26
> +.endif
> +2:
> +
> +        @ Accumulate, adding idx2 last with a separate
> +        @ saturating add. The positive filter coefficients
> +        @ for all indices except idx2 must add up to less
> +        @ than 127 for this not to overflow.
> +        vmul.s16        q1,  q8,  d0[0]
> +        vmul.s16        q3,  q11, d0[0]
> +.if \size >= 16
> +        vmul.s16        q2,  q9,  d0[0]
> +        vmul.s16        q4,  q12, d0[0]
> +.endif
> +        extmla          q1,  q2,  q3,  q4,  q8,  q9,  q10,  q11, q12, q13, 
> 1,     \size
> +        extmla          q1,  q2,  q3,  q4,  q8,  q9,  q10,  q11, q12, q13, 
> 2,     \size
> +        extmla          q1,  q2,  q3,  q4,  q8,  q9,  q10,  q11, q12, q13, 
> \idx1, \size
> +        extmla          q1,  q2,  q3,  q4,  q8,  q9,  q10,  q11, q12, q13, 
> 5,     \size
> +        extmla          q1,  q2,  q3,  q4,  q8,  q9,  q10,  q11, q12, q13, 
> 6,     \size
> +        extmla          q1,  q2,  q3,  q4,  q8,  q9,  q10,  q11, q12, q13, 
> 7,     \size
> +        extmulqadd      q1,  q2,  q3,  q4,  q8,  q9,  q10,  q11, q12, q13, 
> \idx2, \size
> +
> +        @ Round, shift and saturate
> +        vqrshrun.s16    d2,  q1,  #7
> +        vqrshrun.s16    d6,  q3,  #7
> +.if \size >= 16
> +        vqrshrun.s16    d3,  q2,  #7
> +        vqrshrun.s16    d7,  q4,  #7
> +.endif
> +        @ Average
> +.ifc \type,avg
> +.if \size >= 16
> +        vld1.8          {q14}, [r0,:128]
> +        vld1.8          {q15}, [r6,:128]
> +        vrhadd.u8       q1,  q1,  q14
> +        vrhadd.u8       q3,  q3,  q15
> +.elseif \size == 8
> +        vld1.8          {d28}, [r0,:64]
> +        vld1.8          {d30}, [r6,:64]
> +        vrhadd.u8       d2,  d2,  d28
> +        vrhadd.u8       d6,  d6,  d30
> +.else
> +        @ We only need d28[0], but [] is faster on some cores
> +        vld1.32         {d28[]}, [r0,:32]
> +        vld1.32         {d30[]}, [r6,:32]
> +        vrhadd.u8       d2,  d2,  d28
> +        vrhadd.u8       d6,  d6,  d30
> +.endif
> +.endif
> +        @ Store and loop horizontally (for size > 16)
> +.if \size > 16
> +        vst1.8          {q1}, [r0,:128]!
> +        vst1.8          {q3}, [r6,:128]!
> +        vmov            q8,  q10
> +        vmov            q11, q13
> +        subs            r5,  r5,  #16
> +        beq             3f
> +        vld1.8          {q10}, [r2]!
> +        vld1.8          {q13}, [r7]!
> +        vmovl.u8        q9,  d20
> +        vmovl.u8        q10, d21
> +        vmovl.u8        q12, d26
> +        vmovl.u8        q13, d27
> +        b               2b
> +.elseif \size == 16
> +        vst1.8          {q1}, [r0,:128]
> +        vst1.8          {q3}, [r6,:128]
> +.elseif \size == 8
> +        vst1.8          {d2}, [r0,:64]
> +        vst1.8          {d6}, [r6,:64]
> +.else @ \size == 4
> +        vst1.32         {d2[0]}, [r0,:32]
> +        vst1.32         {d6[0]}, [r6,:32]
> +.endif
> +3:
> +        @ Loop vertically
> +        add             r0,  r0,  r1
> +        add             r6,  r6,  r1
> +        add             r2,  r2,  r3
> +        add             r7,  r7,  r3
> +        subs            r4,  r4,  #2
> +        bne             1b
> +.if \size >= 16
> +        vpop            {q4-q6}
> +.endif
> +        pop             {r4-r7}
> +        bx              lr
> +endfunc
> +.endm
> +
> +.macro do_8tap_h_size size
> +do_8tap_h put, \size, 3, 4
> +do_8tap_h avg, \size, 3, 4
> +do_8tap_h put, \size, 4, 3
> +do_8tap_h avg, \size, 4, 3
> +.endm
> +
> +do_8tap_h_size 4
> +do_8tap_h_size 8
> +do_8tap_h_size 16
> +do_8tap_h_size 32

I'm not sure if it's worth to have the size 32+ version. The additional
loop checks are probably not noticeable in size 16 case and it would
reduce the binary size.

The other idea for keeping the calculation in signed 16-bit would have
been using a negative constant as offset since the range doesn't exceed
16-bit. Not faster in checkasm but smaller binary size.

No need to try that. Current implementation is fine for me and looks
quite nice.

...

> +@ Vertical filters

...

> +@ Evaluate the filter twice in parallel, from the inputs src1-src9 
> into dst1-dst2
> +@ (src1-src8 into dst1, src2-src9 into dst2), adding idx2 separately
> +@ at the end with saturation
> +.macro convolve dst1, dst2, src1, src2, src3, src4, src5, src6, src7, src8, 
> src9, idx1, idx2, tmp1, tmp2
> +        vmul.s16        \dst1, \src1, d0[0]
> +        vmul.s16        \dst2, \src2, d0[0]
> +        vmla.s16        \dst1, \src2, d0[1]
> +        vmla.s16        \dst2, \src3, d0[1]
> +        vmla.s16        \dst1, \src3, d0[2]
> +        vmla.s16        \dst2, \src4, d0[2]
> +.if \idx1 == 3
> +        vmla.s16        \dst1, \src4, d0[3]
> +        vmla.s16        \dst2, \src5, d0[3]
> +.else
> +        vmla.s16        \dst1, \src5, d1[0]
> +        vmla.s16        \dst2, \src6, d1[0]
> +.endif
> +        vmla.s16        \dst1, \src6, d1[1]
> +        vmla.s16        \dst2, \src7, d1[1]
> +        vmla.s16        \dst1, \src7, d1[2]
> +        vmla.s16        \dst2, \src8, d1[2]
> +        vmla.s16        \dst1, \src8, d1[3]
> +        vmla.s16        \dst2, \src9, d1[3]
> +.if \idx2 == 3
> +        vmul.s16        \tmp1, \src4, d0[3]
> +        vmul.s16        \tmp2, \src5, d0[3]
> +.else
> +        vmul.s16        \tmp1, \src5, d1[0]
> +        vmul.s16        \tmp2, \src6, d1[0]
> +.endif
> +        vqadd.s16       \dst1, \dst1, \tmp1
> +        vqadd.s16       \dst2, \dst2, \tmp2
> +.endm

the negative terms could be accumulated in tmp1/2 I doubt it makes a
difference in practice but reduces data dependencies.

...

> +@ Instantiate a vertical filter function for filtering a 4 pixels 
> wide
> +@ slice. The first half of the registers contain one row, while the second
> +@ half of a register contains the second-next row (also stored in the first
> +@ half of the register two steps ahead). The convolution does two outputs
> +@ at a time; the output of q5-q12 into one, and q4-q13 into another one.
> +@ The first half of first output is the first output row, the first half
> +@ of the other output is the second output row. The second halves of the
> +@ registers are rows 3 and 4.
> +@ This only is designed to work for 4 or 8 output lines.
> +.macro do_8tap_4v type, idx1, idx2
> +function \type\()_8tap_4v_\idx1\idx2
> +        sub             r2,  r2,  r3, lsl #1
> +        sub             r2,  r2,  r3
> +        vld1.16         {q0},  [r12, :128]
> +
> +        vld1.32         {d2[]},   [r2], r3
> +        vld1.32         {d3[]},   [r2], r3
> +        vld1.32         {d4[]},   [r2], r3
> +        vld1.32         {d5[]},   [r2], r3
> +        vld1.32         {d6[]},   [r2], r3
> +        vld1.32         {d7[]},   [r2], r3
> +        vld1.32         {d8[]},   [r2], r3
> +        vld1.32         {d9[]},   [r2], r3
> +        vld1.32         {d28[]},  [r2]
> +        sub             r2,  r2,  r3, lsl #2
> +        sub             r2,  r2,  r3, lsl #1
> +        vld1.32         {d2[1]},  [r2], r3

vext.8 d2,  d2,  d4,  #4 should be faster than loading. it works since
we loaded data to all lanes

> +        vld1.32         {d3[1]},  [r2], r3
> +        vmovl.u8        q5,  d2
> +        vld1.32         {d4[1]},  [r2], r3
> +        vmovl.u8        q6,  d3
> +        vld1.32         {d5[1]},  [r2], r3
> +        vmovl.u8        q7,  d4
> +        vld1.32         {d6[1]},  [r2], r3
> +        vmovl.u8        q8,  d5
> +        vld1.32         {d7[1]},  [r2], r3
> +        vmovl.u8        q9,  d6
> +        vld1.32         {d8[1]},  [r2], r3
> +        vmovl.u8        q10, d7
> +        vld1.32         {d9[1]},  [r2], r3
> +        vmovl.u8        q11, d8
> +        vld1.32         {d28[1]}, [r2]
> +        sub             r2,  r2,  r3
> +
> +        vmovl.u8        q12, d9
> +        vmovl.u8        q13, d28
> +
> +        convolve        q1,  q2,  q5,  q6,  q7,  q8,  q9,  q10, q11, q12, 
> q13, \idx1, \idx2, q4, q3
> +        do_store4       q1,  d2,  q2,  d4,  d3,  d5,  \type
> +        subs            r4,  r4,  #4
> +        beq             9f
> +
> +        vld1.32         {d2[]},   [r2], r3
> +        vld1.32         {d3[]},   [r2], r3
> +        vld1.32         {d4[]},   [r2], r3
> +        vld1.32         {d5[]},   [r2], r3
> +        sub             r2,  r2,  r3, lsl #1
> +        vld1.32         {d2[1]},  [r2], r3
> +        vld1.32         {d3[1]},  [r2], r3
> +        vmovl.u8        q14, d2
> +        vld1.32         {d4[1]},  [r2], r3
> +        vmovl.u8        q15, d3
> +        vld1.32         {d5[1]},  [r2], r3
> +        vmovl.u8        q5,  d4
> +        vmovl.u8        q6,  d5
> +
> +        convolve        q1,  q2,  q9,  q10, q11, q12, q13, q14, q15, q5,  
> q6,  \idx1, \idx2, q4, q3
> +        do_store4       q1,  d2,  q2,  d4,  d3,  d5,  \type
> +
> +9:
> +        vpop            {q4-q7}
> +        pop             {r4-r5}
> +        bx              lr
> +endfunc
> +.endm
> +
> +do_8tap_4v put, 3, 4
> +do_8tap_4v put, 4, 3
> +do_8tap_4v avg, 3, 4
> +do_8tap_4v avg, 4, 3
> +
> +.macro do_8tap_v_func type, filter, size
> +function ff_vp9_\type\()_\filter\()\size\()_v_neon, export=1
> +        push            {r4-r5}
> +        vpush           {q4-q7}
> +        ldr             r4,  [sp, #72]
> +        ldr             r5,  [sp, #80]
> +        movrel          r12, \filter\()_filter-16
> +        add             r12,  r12, r5, lsl #4
> +        cmp             r5,  #8
> +        mov             r5,  #\size

a little confusing that this changes r5 and r12 compared to the
horizontal mc, changing the horizontal mc so that r5 holds the width
and r12 the filter constants/tmp reg would work if you care

Skipped parts are ok and the asm looks already good. Sorry the review 
too so long.

Janne
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to