| Message ID | 20210110232453.52663-1-Reimar.Doeffinger@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libswscale/aarch64/hscale.S: Support more bit-depth variants. | expand |
| 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 |
On Mon, 11 Jan 2021, Reimar.Doeffinger@gmx.de wrote: > From: Reimar Döffinger <Reimar.Doeffinger@gmx.de> > > Trivially expand hscale assembler to support > 8 bit formats > both for input and output. > 16-bit input is not supported as I am not certain how to > get sufficient test coverage. > --- > libswscale/aarch64/hscale.S | 53 ++++++++++++++++++++++++++---------- > libswscale/aarch64/swscale.c | 49 +++++++++++++++++++++++++++++++-- > 2 files changed, 85 insertions(+), 17 deletions(-) > > diff --git a/libswscale/aarch64/hscale.S b/libswscale/aarch64/hscale.S > index af55ffe2b7..3b42d39dac 100644 > --- a/libswscale/aarch64/hscale.S > +++ b/libswscale/aarch64/hscale.S > @@ -20,7 +20,11 @@ > > #include "libavutil/aarch64/asm.S" > > -function ff_hscale_8_to_15_neon, export=1 > +.macro hscale srcbits, dstbits, ldt, lds, c > +function ff_hscale_\srcbits\()_to_\dstbits\()_neon, export=1 > +.if \dstbits >= 16 > + movi v20.4S, #(0x1 << (\dstbits - 16)), msl #16 > +.endif > sbfiz x7, x6, #1, #32 // filterSize*2 (*2 because int16) > 1: ldr w8, [x5], #4 // filterPos[idx] > ldr w0, [x5], #4 // filterPos[idx + 1] > @@ -34,30 +38,30 @@ function ff_hscale_8_to_15_neon, export=1 > movi v1.2D, #0 // val sum part 2 (for dst[1]) > movi v2.2D, #0 // val sum part 3 (for dst[2]) > movi v3.2D, #0 // val sum part 4 (for dst[3]) > - add x17, x3, w8, UXTW // srcp + filterPos[0] > - add x8, x3, w0, UXTW // srcp + filterPos[1] > - add x0, x3, w11, UXTW // srcp + filterPos[2] > - add x11, x3, w9, UXTW // srcp + filterPos[3] > + add x17, x3, w8, UXTW #!!(\srcbits > 8) // srcp + filterPos[0] > + add x8, x3, w0, UXTW #!!(\srcbits > 8) // srcp + filterPos[1] > + add x0, x3, w11, UXTW #!!(\srcbits > 8) // srcp + filterPos[2] > + add x11, x3, w9, UXTW #!!(\srcbits > 8) // srcp + filterPos[3] This construct breaks with llvm's assembler and armasm64 - they don't evaluate !! here, ending up with errors like these: <instantiation>:19:31: error: expected integer shift amount add x8, x3, w0, UXTW #!!(8 > 8) libswscale\aarch64\hscale.o.asm(3093) : error A2007: wrong operand type for :LNOT: add x17, x3, w8, UXTW #!!0 While less elegant, I guess this can be handled easily by adding a macro parameter that represents the evaluated value of \srcbits > 8. > mov w15, w6 // filterSize counter > -2: ld1 {v4.8B}, [x17], #8 // srcp[filterPos[0] + {0..7}] > +2: ld1 {v4.\ldt}, [x17], \lds // srcp[filterPos[0] + {0..7}] > ld1 {v5.8H}, [x16], #16 // load 8x16-bit filter values, part 1 > - ld1 {v6.8B}, [x8], #8 // srcp[filterPos[1] + {0..7}] > + ld1 {v6.\ldt}, [x8], \lds // srcp[filterPos[1] + {0..7}] > ld1 {v7.8H}, [x12], #16 // load 8x16-bit at filter+filterSize > - uxtl v4.8H, v4.8B // unpack part 1 to 16-bit > +\c\c uxtl v4.8H, v4.8B // unpack part 1 to 16-bit With gas-preprocessor and armasm64, // isn't considered a comment char by armasm64, only by the C preprocessor invoked by gas-preprocessor, so it doesn't strip out these lines properly. I guess one could add more code to gas-preprocessor to handle that, but that's probably a bit overkill... Adding .if .endif around these instances isn't entirely elegant, but you can also make e.g. a macro uxtl_if, which takes a third parameter which is a predicate for whether the instruction should be included or not. > smlal v0.4S, v4.4H, v5.4H // v0 accumulates srcp[filterPos[0] + {0..3}] * filter[{0..3}] > smlal2 v0.4S, v4.8H, v5.8H // v0 accumulates srcp[filterPos[0] + {4..7}] * filter[{4..7}] > - ld1 {v16.8B}, [x0], #8 // srcp[filterPos[2] + {0..7}] > + ld1 {v16.\ldt}, [x0], \lds // srcp[filterPos[2] + {0..7}] LLVM's assembler had a bug regarding passing parameters like "8h" as a parameter to a macro, which was only fixed in LLVM 8, see https://bugs.llvm.org/show_bug.cgi?id=32973. That bug is easy to avoid by moving the dot into the macro argument, i.e. making this "ld1 {v16\ldt}, ..." and passing ".8h" as macro argument. Additionally - we do have a checkasm test for this function, but not for these bitdepth combinations. It'd be great to extend the test to test these combinations as well - I'd prefer to not add much more assembly without a corresponding checkasm test. // Martin
> On 15 Jan 2021, at 22:10, Martin Storsjö <martin@martin.st> wrote: > > On Mon, 11 Jan 2021, Reimar.Doeffinger@gmx.de wrote: > >> From: Reimar Döffinger <Reimar.Doeffinger@gmx.de> >> >> Trivially expand hscale assembler to support > 8 bit formats >> both for input and output. >> 16-bit input is not supported as I am not certain how to >> get sufficient test coverage. >> --- >> libswscale/aarch64/hscale.S | 53 ++++++++++++++++++++++++++---------- >> libswscale/aarch64/swscale.c | 49 +++++++++++++++++++++++++++++++-- >> 2 files changed, 85 insertions(+), 17 deletions(-) >> >> diff --git a/libswscale/aarch64/hscale.S b/libswscale/aarch64/hscale.S >> index af55ffe2b7..3b42d39dac 100644 >> --- a/libswscale/aarch64/hscale.S >> +++ b/libswscale/aarch64/hscale.S >> @@ -20,7 +20,11 @@ >> #include "libavutil/aarch64/asm.S" >> -function ff_hscale_8_to_15_neon, export=1 >> +.macro hscale srcbits, dstbits, ldt, lds, c >> +function ff_hscale_\srcbits\()_to_\dstbits\()_neon, export=1 >> +.if \dstbits >= 16 >> + movi v20.4S, #(0x1 << (\dstbits - 16)), msl #16 >> +.endif >> sbfiz x7, x6, #1, #32 // filterSize*2 (*2 because int16) >> 1: ldr w8, [x5], #4 // filterPos[idx] >> ldr w0, [x5], #4 // filterPos[idx + 1] >> @@ -34,30 +38,30 @@ function ff_hscale_8_to_15_neon, export=1 >> movi v1.2D, #0 // val sum part 2 (for dst[1]) >> movi v2.2D, #0 // val sum part 3 (for dst[2]) >> movi v3.2D, #0 // val sum part 4 (for dst[3]) >> - add x17, x3, w8, UXTW // srcp + filterPos[0] >> - add x8, x3, w0, UXTW // srcp + filterPos[1] >> - add x0, x3, w11, UXTW // srcp + filterPos[2] >> - add x11, x3, w9, UXTW // srcp + filterPos[3] >> + add x17, x3, w8, UXTW #!!(\srcbits > 8) // srcp + filterPos[0] >> + add x8, x3, w0, UXTW #!!(\srcbits > 8) // srcp + filterPos[1] >> + add x0, x3, w11, UXTW #!!(\srcbits > 8) // srcp + filterPos[2] >> + add x11, x3, w9, UXTW #!!(\srcbits > 8) // srcp + filterPos[3] > > This construct breaks with llvm's assembler and armasm64 - they don't evaluate !! here, ending up with errors like these: > > <instantiation>:19:31: error: expected integer shift amount > add x8, x3, w0, UXTW #!!(8 > 8) > > libswscale\aarch64\hscale.o.asm(3093) : error A2007: wrong operand type for :LNOT: > add x17, x3, w8, UXTW #!!0 > > While less elegant, I guess this can be handled easily by adding a macro parameter that represents the evaluated value of \srcbits > 8. It’s actually easier. I just need 0 or 1 here, the thing I was missing that caused me to add the !! is that a true condition does not evaluate to 1 but to -1, so (\srcbits <= 8) + 1 works with both. >> mov w15, w6 // filterSize counter >> -2: ld1 {v4.8B}, [x17], #8 // srcp[filterPos[0] + {0..7}] >> +2: ld1 {v4.\ldt}, [x17], \lds // srcp[filterPos[0] + {0..7}] >> ld1 {v5.8H}, [x16], #16 // load 8x16-bit filter values, part 1 >> - ld1 {v6.8B}, [x8], #8 // srcp[filterPos[1] + {0..7}] >> + ld1 {v6.\ldt}, [x8], \lds // srcp[filterPos[1] + {0..7}] >> ld1 {v7.8H}, [x12], #16 // load 8x16-bit at filter+filterSize >> - uxtl v4.8H, v4.8B // unpack part 1 to 16-bit >> +\c\c uxtl v4.8H, v4.8B // unpack part 1 to 16-bit > > With gas-preprocessor and armasm64, // isn't considered a comment char by armasm64, only by the C preprocessor invoked by gas-preprocessor, so it doesn't strip out these lines properly. I guess one could add more code to gas-preprocessor to handle that, but that's probably a bit overkill... That is a whole another fun topic, I’ll be sending patches related to gas-preprocessor. Admittedly might consider some of the issues my fault for not cross-compiling and also preferring WSL over msys2 or such... And then I might get around to addressing the rest.
diff --git a/libswscale/aarch64/hscale.S b/libswscale/aarch64/hscale.S index af55ffe2b7..3b42d39dac 100644 --- a/libswscale/aarch64/hscale.S +++ b/libswscale/aarch64/hscale.S @@ -20,7 +20,11 @@ #include "libavutil/aarch64/asm.S" -function ff_hscale_8_to_15_neon, export=1 +.macro hscale srcbits, dstbits, ldt, lds, c +function ff_hscale_\srcbits\()_to_\dstbits\()_neon, export=1 +.if \dstbits >= 16 + movi v20.4S, #(0x1 << (\dstbits - 16)), msl #16 +.endif sbfiz x7, x6, #1, #32 // filterSize*2 (*2 because int16) 1: ldr w8, [x5], #4 // filterPos[idx] ldr w0, [x5], #4 // filterPos[idx + 1] @@ -34,30 +38,30 @@ function ff_hscale_8_to_15_neon, export=1 movi v1.2D, #0 // val sum part 2 (for dst[1]) movi v2.2D, #0 // val sum part 3 (for dst[2]) movi v3.2D, #0 // val sum part 4 (for dst[3]) - add x17, x3, w8, UXTW // srcp + filterPos[0] - add x8, x3, w0, UXTW // srcp + filterPos[1] - add x0, x3, w11, UXTW // srcp + filterPos[2] - add x11, x3, w9, UXTW // srcp + filterPos[3] + add x17, x3, w8, UXTW #!!(\srcbits > 8) // srcp + filterPos[0] + add x8, x3, w0, UXTW #!!(\srcbits > 8) // srcp + filterPos[1] + add x0, x3, w11, UXTW #!!(\srcbits > 8) // srcp + filterPos[2] + add x11, x3, w9, UXTW #!!(\srcbits > 8) // srcp + filterPos[3] mov w15, w6 // filterSize counter -2: ld1 {v4.8B}, [x17], #8 // srcp[filterPos[0] + {0..7}] +2: ld1 {v4.\ldt}, [x17], \lds // srcp[filterPos[0] + {0..7}] ld1 {v5.8H}, [x16], #16 // load 8x16-bit filter values, part 1 - ld1 {v6.8B}, [x8], #8 // srcp[filterPos[1] + {0..7}] + ld1 {v6.\ldt}, [x8], \lds // srcp[filterPos[1] + {0..7}] ld1 {v7.8H}, [x12], #16 // load 8x16-bit at filter+filterSize - uxtl v4.8H, v4.8B // unpack part 1 to 16-bit +\c\c uxtl v4.8H, v4.8B // unpack part 1 to 16-bit smlal v0.4S, v4.4H, v5.4H // v0 accumulates srcp[filterPos[0] + {0..3}] * filter[{0..3}] smlal2 v0.4S, v4.8H, v5.8H // v0 accumulates srcp[filterPos[0] + {4..7}] * filter[{4..7}] - ld1 {v16.8B}, [x0], #8 // srcp[filterPos[2] + {0..7}] + ld1 {v16.\ldt}, [x0], \lds // srcp[filterPos[2] + {0..7}] ld1 {v17.8H}, [x13], #16 // load 8x16-bit at filter+2*filterSize - uxtl v6.8H, v6.8B // unpack part 2 to 16-bit +\c\c uxtl v6.8H, v6.8B // unpack part 2 to 16-bit smlal v1.4S, v6.4H, v7.4H // v1 accumulates srcp[filterPos[1] + {0..3}] * filter[{0..3}] - uxtl v16.8H, v16.8B // unpack part 3 to 16-bit +\c\c uxtl v16.8H, v16.8B // unpack part 3 to 16-bit smlal v2.4S, v16.4H, v17.4H // v2 accumulates srcp[filterPos[2] + {0..3}] * filter[{0..3}] smlal2 v2.4S, v16.8H, v17.8H // v2 accumulates srcp[filterPos[2] + {4..7}] * filter[{4..7}] - ld1 {v18.8B}, [x11], #8 // srcp[filterPos[3] + {0..7}] + ld1 {v18.\ldt}, [x11], \lds // srcp[filterPos[3] + {0..7}] smlal2 v1.4S, v6.8H, v7.8H // v1 accumulates srcp[filterPos[1] + {4..7}] * filter[{4..7}] ld1 {v19.8H}, [x4], #16 // load 8x16-bit at filter+3*filterSize subs w15, w15, #8 // j -= 8: processed 8/filterSize - uxtl v18.8H, v18.8B // unpack part 4 to 16-bit +\c\c uxtl v18.8H, v18.8B // unpack part 4 to 16-bit smlal v3.4S, v18.4H, v19.4H // v3 accumulates srcp[filterPos[3] + {0..3}] * filter[{0..3}] smlal2 v3.4S, v18.8H, v19.8H // v3 accumulates srcp[filterPos[3] + {4..7}] * filter[{4..7}] b.gt 2b // inner loop if filterSize not consumed completely @@ -73,8 +77,29 @@ function ff_hscale_8_to_15_neon, export=1 zip1 v2.4S, v2.4S, v3.4S // part23 = zip values from part2 and part3 mov v0.d[1], v2.d[0] // part0123 = zip values from part01 and part23 subs w2, w2, #4 // dstW -= 4 - sqshrn v0.4H, v0.4S, #7 // shift and clip the 2x16-bit final values +.if \dstbits < 16 + sqshrn v0.4H, v0.4S, #(14 + \srcbits - \dstbits) // shift and clip the 2x16-bit final values st1 {v0.4H}, [x1], #8 // write to destination part0123 +.else + srshr v0.4S, v0.4S, #(14 + \srcbits - \dstbits) + smin v0.4S, V0.4S, V20.4S + st1 {v0.4S}, [x1], #16 +.endif b.gt 1b // loop until end of line ret endfunc +.endm + +hscale 8, 15, 8B, #8 +hscale 9, 15, 8H, #16, / +hscale 10, 15, 8H, #16, / +hscale 12, 15, 8H, #16, / +hscale 14, 15, 8H, #16, / +// hscale 16, 15, 8H, #16, / // needs testing and possibly some special-cases + +hscale 8 19, 8B, #8 +hscale 9, 19, 8H, #16, / +hscale 10, 19, 8H, #16, / +hscale 12, 19, 8H, #16, / +hscale 14, 19, 8H, #16, / +// hscale 16, 19, 8H, #16, / // needs testing and possibly some special-cases diff --git a/libswscale/aarch64/swscale.c b/libswscale/aarch64/swscale.c index eecbea88ca..4c88f929d7 100644 --- a/libswscale/aarch64/swscale.c +++ b/libswscale/aarch64/swscale.c @@ -24,6 +24,33 @@ void ff_hscale_8_to_15_neon(SwsContext *c, int16_t *dst, int dstW, const uint8_t *src, const int16_t *filter, const int32_t *filterPos, int filterSize); +void ff_hscale_9_to_15_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); +void ff_hscale_10_to_15_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); +void ff_hscale_12_to_15_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); +void ff_hscale_14_to_15_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); +void ff_hscale_8_to_19_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); +void ff_hscale_9_to_19_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); +void ff_hscale_10_to_19_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); +void ff_hscale_12_to_19_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); +void ff_hscale_14_to_19_neon(SwsContext *c, int16_t *dst, int dstW, + const uint8_t *src, const int16_t *filter, + const int32_t *filterPos, int filterSize); void ff_yuv2planeX_8_neon(const int16_t *filter, int filterSize, const int16_t **src, uint8_t *dest, int dstW, @@ -34,11 +61,27 @@ av_cold void ff_sws_init_swscale_aarch64(SwsContext *c) int cpu_flags = av_get_cpu_flags(); if (have_neon(cpu_flags)) { - if (c->srcBpc == 8 && c->dstBpc <= 14 && - (c->hLumFilterSize % 8) == 0 && + if ((c->hLumFilterSize % 8) == 0 && (c->hChrFilterSize % 8) == 0) { - c->hyScale = c->hcScale = ff_hscale_8_to_15_neon; + int largedst = c->dstBpc > 14; + switch (c->srcBpc) { + case 8: + c->hyScale = c->hcScale = largedst ? ff_hscale_8_to_19_neon : ff_hscale_8_to_15_neon; + break; + case 9: + c->hyScale = c->hcScale = largedst ? ff_hscale_9_to_19_neon : ff_hscale_9_to_15_neon; + break; + case 10: + c->hyScale = c->hcScale = largedst ? ff_hscale_10_to_19_neon : ff_hscale_10_to_15_neon; + break; + case 12: + c->hyScale = c->hcScale = largedst ? ff_hscale_12_to_19_neon : ff_hscale_12_to_15_neon; + break; + case 14: + c->hyScale = c->hcScale = largedst ? ff_hscale_14_to_19_neon : ff_hscale_14_to_15_neon; + break; + } } if (c->dstBpc == 8) { c->yuv2planeX = ff_yuv2planeX_8_neon;
From: Reimar Döffinger <Reimar.Doeffinger@gmx.de> Trivially expand hscale assembler to support > 8 bit formats both for input and output. 16-bit input is not supported as I am not certain how to get sufficient test coverage. --- libswscale/aarch64/hscale.S | 53 ++++++++++++++++++++++++++---------- libswscale/aarch64/swscale.c | 49 +++++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 17 deletions(-)