| Message ID | 20220913115824.60792-2-hum@semihalf.com |
|---|---|
| State | New |
| Headers | show |
| Series | Provide arm64 neon implementations | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Tue, 13 Sep 2022, Hubert Mazur wrote: > Provide optimized implementation for pix_median_abs16 function. > > Performance comparison tests are shown below. > - median_sad_0_c: 722.0 > - median_sad_0_neon: 144.7 > > Benchmarks and tests run with checkasm tool on AWS Graviton 3. > > Signed-off-by: Hubert Mazur <hum@semihalf.com> > --- > libavcodec/aarch64/me_cmp_init_aarch64.c | 4 ++ > libavcodec/aarch64/me_cmp_neon.S | 81 ++++++++++++++++++++++++ > libavcodec/me_cmp.c | 5 +- > 3 files changed, 88 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/aarch64/me_cmp_init_aarch64.c b/libavcodec/aarch64/me_cmp_init_aarch64.c > index ade3e9a4c1..fb51a833be 100644 > --- a/libavcodec/aarch64/me_cmp_init_aarch64.c > +++ b/libavcodec/aarch64/me_cmp_init_aarch64.c > @@ -53,6 +53,8 @@ int nsse16_neon(int multiplier, const uint8_t *s, const uint8_t *s2, > ptrdiff_t stride, int h); > int nsse16_neon_wrapper(MpegEncContext *c, const uint8_t *s1, const uint8_t *s2, > ptrdiff_t stride, int h); > +int pix_median_abs16_neon(MpegEncContext *v, const uint8_t *pix1, const uint8_t *pix2, > + ptrdiff_t stride, int h); > > av_cold void ff_me_cmp_init_aarch64(MECmpContext *c, AVCodecContext *avctx) > { > @@ -78,6 +80,8 @@ av_cold void ff_me_cmp_init_aarch64(MECmpContext *c, AVCodecContext *avctx) > c->vsse[4] = vsse_intra16_neon; > > c->nsse[0] = nsse16_neon_wrapper; > + > + c->median_sad[0] = pix_median_abs16_neon; > } > } > > diff --git a/libavcodec/aarch64/me_cmp_neon.S b/libavcodec/aarch64/me_cmp_neon.S > index f8998749a5..a4a4344f42 100644 > --- a/libavcodec/aarch64/me_cmp_neon.S > +++ b/libavcodec/aarch64/me_cmp_neon.S > @@ -969,3 +969,84 @@ function nsse16_neon, export=1 > > ret > endfunc > + > +function pix_median_abs16_neon, export=1 > + // x0 unused > + // x1 uint8_t *pix1 > + // x2 uint8_t *pix2 > + // x3 ptrdiff_t stride > + // w4 int h > + > + ld1 {v2.16b}, [x1], x3 > + ld1 {v3.16b}, [x2], x3 > + movi v31.8h, #0 > + movi v16.8h, #0 > + ext v0.16b, v2.16b, v2.16b, #1 > + ext v1.16b, v3.16b, v3.16b, #1 > + usubl v28.8h, v2.8b, v3.8b > + usubl2 v27.8h, v2.16b, v3.16b > + usubl v26.8h, v0.8b, v1.8b > + usubl2 v25.8h, v0.16b, v1.16b > + sub w4, w4, #1 // we need to make h-1 iterations > + saba v31.8h, v26.8h, v28.8h > + saba v16.8h, v25.8h, v27.8h > + mov h18, v28.h[0] > + cmp w4, #1 > + sqabs h18, h18 > + > + b.lt 2f > +1: > + > + ld1 {v6.16b}, [x1], x3 // pix1 vector for V(j-1) > + ld1 {v7.16b}, [x2], x3 // pix2 vector for V(j-1) > + subs w4, w4, #1 > + mov v2.16b, v6.16b > + mov v3.16b, v7.16b These two mov instructions seem unnecessary? > + ext v4.16b, v6.16b, v6.16b, #1 // pix1 vector for V(j) > + ext v5.16b, v7.16b, v7.16b, #1 // pix2 vector for V(j) > + > + // protected registers: v30, v29, v28, v27, v26, v25, v24, v23 > + // scratch registers: v22, v21, v20, v19, v17 > + > + // To find median of three values, calculate sum of them > + // and subtract max and min value from it. > + usubl v30.8h, v6.8b, v7.8b // V(j-1) > + usubl2 v29.8h, v6.16b, v7.16b // V(j-1) > + usubl v24.8h, v4.8b, v5.8b // V(j) > + usubl2 v23.8h, v4.16b, v5.16b // V(j) > + mov v0.16b, v4.16b > + mov v1.16b, v5.16b These two movs are unused, too, right? > + sabd v20.8h, v30.8h, v28.8h > + mov h17, v20.h[0] > + add d18, d18, d17 These are quite suboptimally scheduled here. However, we shouldn't need them. In general, try to avoid these single-element calculations if not strictly necessary. You can just keep using both the input (here, v20) and the accumulator (v18) as a .4h vector, where you only care about the first element. Then at the very end you can extract the individual first element from it, instead of doing it every round in the loop. Then you can potentially change sabd into saba too, unless the non-accumulated result is needed too. > + add v22.8h, v26.8h, v30.8h > + smin v20.8h, v26.8h, v30.8h > + add v21.8h, v25.8h, v29.8h > + smax v19.8h, v26.8h, v30.8h > + sub v22.8h, v22.8h, v28.8h > + sub v21.8h, v21.8h, v27.8h > + smin v17.8h, v19.8h, v22.8h > + smin v22.8h, v25.8h, v29.8h > + mov v28.16b, v30.16b > + smax v20.8h, v20.8h, v17.8h // median values lower half > + smax v19.8h, v25.8h, v29.8h > + saba v31.8h, v24.8h, v20.8h > + mov v27.16b, v29.16b > + smin v19.8h, v19.8h, v21.8h > + mov v26.16b, v24.16b > + smax v17.8h, v22.8h, v19.8h // median values upper half > + mov v25.16b, v23.16b > + saba v16.8h, v23.8h, v17.8h > + > + b.ne 1b > + > +2: > + ins v16.h[7], wzr > + add v31.8h, v31.8h, v16.8h > + uaddlv s17, v31.8h > + add d18, d18, d17 > + fmov w0, s18 > + > + ret > + > +endfunc > diff --git a/libavcodec/me_cmp.c b/libavcodec/me_cmp.c > index 4242fbc6e4..230e7ea54a 100644 > --- a/libavcodec/me_cmp.c > +++ b/libavcodec/me_cmp.c > @@ -1048,6 +1048,9 @@ av_cold void ff_me_cmp_init(MECmpContext *c, AVCodecContext *avctx) > ff_dsputil_init_dwt(c); > #endif > > +c->median_sad[0] = pix_median_abs16_c; > +c->median_sad[1] = pix_median_abs8_c; > + These are incorrectly indented. Other than that, this seems reasonable I think. // Martin
diff --git a/libavcodec/aarch64/me_cmp_init_aarch64.c b/libavcodec/aarch64/me_cmp_init_aarch64.c index ade3e9a4c1..fb51a833be 100644 --- a/libavcodec/aarch64/me_cmp_init_aarch64.c +++ b/libavcodec/aarch64/me_cmp_init_aarch64.c @@ -53,6 +53,8 @@ int nsse16_neon(int multiplier, const uint8_t *s, const uint8_t *s2, ptrdiff_t stride, int h); int nsse16_neon_wrapper(MpegEncContext *c, const uint8_t *s1, const uint8_t *s2, ptrdiff_t stride, int h); +int pix_median_abs16_neon(MpegEncContext *v, const uint8_t *pix1, const uint8_t *pix2, + ptrdiff_t stride, int h); av_cold void ff_me_cmp_init_aarch64(MECmpContext *c, AVCodecContext *avctx) { @@ -78,6 +80,8 @@ av_cold void ff_me_cmp_init_aarch64(MECmpContext *c, AVCodecContext *avctx) c->vsse[4] = vsse_intra16_neon; c->nsse[0] = nsse16_neon_wrapper; + + c->median_sad[0] = pix_median_abs16_neon; } } diff --git a/libavcodec/aarch64/me_cmp_neon.S b/libavcodec/aarch64/me_cmp_neon.S index f8998749a5..a4a4344f42 100644 --- a/libavcodec/aarch64/me_cmp_neon.S +++ b/libavcodec/aarch64/me_cmp_neon.S @@ -969,3 +969,84 @@ function nsse16_neon, export=1 ret endfunc + +function pix_median_abs16_neon, export=1 + // x0 unused + // x1 uint8_t *pix1 + // x2 uint8_t *pix2 + // x3 ptrdiff_t stride + // w4 int h + + ld1 {v2.16b}, [x1], x3 + ld1 {v3.16b}, [x2], x3 + movi v31.8h, #0 + movi v16.8h, #0 + ext v0.16b, v2.16b, v2.16b, #1 + ext v1.16b, v3.16b, v3.16b, #1 + usubl v28.8h, v2.8b, v3.8b + usubl2 v27.8h, v2.16b, v3.16b + usubl v26.8h, v0.8b, v1.8b + usubl2 v25.8h, v0.16b, v1.16b + sub w4, w4, #1 // we need to make h-1 iterations + saba v31.8h, v26.8h, v28.8h + saba v16.8h, v25.8h, v27.8h + mov h18, v28.h[0] + cmp w4, #1 + sqabs h18, h18 + + b.lt 2f +1: + + ld1 {v6.16b}, [x1], x3 // pix1 vector for V(j-1) + ld1 {v7.16b}, [x2], x3 // pix2 vector for V(j-1) + subs w4, w4, #1 + mov v2.16b, v6.16b + mov v3.16b, v7.16b + ext v4.16b, v6.16b, v6.16b, #1 // pix1 vector for V(j) + ext v5.16b, v7.16b, v7.16b, #1 // pix2 vector for V(j) + + // protected registers: v30, v29, v28, v27, v26, v25, v24, v23 + // scratch registers: v22, v21, v20, v19, v17 + + // To find median of three values, calculate sum of them + // and subtract max and min value from it. + usubl v30.8h, v6.8b, v7.8b // V(j-1) + usubl2 v29.8h, v6.16b, v7.16b // V(j-1) + usubl v24.8h, v4.8b, v5.8b // V(j) + usubl2 v23.8h, v4.16b, v5.16b // V(j) + mov v0.16b, v4.16b + mov v1.16b, v5.16b + sabd v20.8h, v30.8h, v28.8h + mov h17, v20.h[0] + add d18, d18, d17 + add v22.8h, v26.8h, v30.8h + smin v20.8h, v26.8h, v30.8h + add v21.8h, v25.8h, v29.8h + smax v19.8h, v26.8h, v30.8h + sub v22.8h, v22.8h, v28.8h + sub v21.8h, v21.8h, v27.8h + smin v17.8h, v19.8h, v22.8h + smin v22.8h, v25.8h, v29.8h + mov v28.16b, v30.16b + smax v20.8h, v20.8h, v17.8h // median values lower half + smax v19.8h, v25.8h, v29.8h + saba v31.8h, v24.8h, v20.8h + mov v27.16b, v29.16b + smin v19.8h, v19.8h, v21.8h + mov v26.16b, v24.16b + smax v17.8h, v22.8h, v19.8h // median values upper half + mov v25.16b, v23.16b + saba v16.8h, v23.8h, v17.8h + + b.ne 1b + +2: + ins v16.h[7], wzr + add v31.8h, v31.8h, v16.8h + uaddlv s17, v31.8h + add d18, d18, d17 + fmov w0, s18 + + ret + +endfunc diff --git a/libavcodec/me_cmp.c b/libavcodec/me_cmp.c index 4242fbc6e4..230e7ea54a 100644 --- a/libavcodec/me_cmp.c +++ b/libavcodec/me_cmp.c @@ -1048,6 +1048,9 @@ av_cold void ff_me_cmp_init(MECmpContext *c, AVCodecContext *avctx) ff_dsputil_init_dwt(c); #endif +c->median_sad[0] = pix_median_abs16_c; +c->median_sad[1] = pix_median_abs8_c; + #if ARCH_AARCH64 ff_me_cmp_init_aarch64(c, avctx); #elif ARCH_ALPHA @@ -1062,6 +1065,4 @@ av_cold void ff_me_cmp_init(MECmpContext *c, AVCodecContext *avctx) ff_me_cmp_init_mips(c, avctx); #endif - c->median_sad[0] = pix_median_abs16_c; - c->median_sad[1] = pix_median_abs8_c; }
Provide optimized implementation for pix_median_abs16 function. Performance comparison tests are shown below. - median_sad_0_c: 722.0 - median_sad_0_neon: 144.7 Benchmarks and tests run with checkasm tool on AWS Graviton 3. Signed-off-by: Hubert Mazur <hum@semihalf.com> --- libavcodec/aarch64/me_cmp_init_aarch64.c | 4 ++ libavcodec/aarch64/me_cmp_neon.S | 81 ++++++++++++++++++++++++ libavcodec/me_cmp.c | 5 +- 3 files changed, 88 insertions(+), 2 deletions(-)