[FFmpeg-devel] [PATCH v2 5/6] lavc/apv: AVX2 transquant for x86-64
Mark Thompson
sw at jkqxz.net
Mon Apr 21 22:50:15 EEST 2025
On 21/04/2025 17:53, James Almer wrote:
> On 4/21/2025 12:24 PM, Mark Thompson wrote:
>> Typical checkasm result on Alder Lake:
>>
>> decode_transquant_8_c: 461.1 ( 1.00x)
>> decode_transquant_8_avx2: 97.5 ( 4.73x)
>> decode_transquant_10_c: 483.9 ( 1.00x)
>> decode_transquant_10_avx2: 91.7 ( 5.28x)
>> ---
>> libavcodec/apv_dsp.c | 4 +
>> libavcodec/apv_dsp.h | 2 +
>> libavcodec/x86/Makefile | 2 +
>> libavcodec/x86/apv_dsp.asm | 279 ++++++++++++++++++++++++++++++++++
>> libavcodec/x86/apv_dsp_init.c | 40 +++++
>> tests/checkasm/Makefile | 1 +
>> tests/checkasm/apv_dsp.c | 109 +++++++++++++
>> tests/checkasm/checkasm.c | 3 +
>> tests/checkasm/checkasm.h | 1 +
>> tests/fate/checkasm.mak | 1 +
>> 10 files changed, 442 insertions(+)
>> create mode 100644 libavcodec/x86/apv_dsp.asm
>> create mode 100644 libavcodec/x86/apv_dsp_init.c
>> create mode 100644 tests/checkasm/apv_dsp.c
>>
>> ...
>> diff --git a/libavcodec/x86/apv_dsp.asm b/libavcodec/x86/apv_dsp.asm
>> new file mode 100644
>> index 0000000000..6b045e989a
>> --- /dev/null
>> +++ b/libavcodec/x86/apv_dsp.asm
>> @@ -0,0 +1,279 @@
>> +;************************************************************************
>> +;* 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
>> +;* 51, Inc., Foundation Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> +;******************************************************************************
>> +
>> +%include "libavutil/x86/x86util.asm"
>> +
>> +SECTION_RODATA 32
>> +
>> +; Full matrix for row transform.
>> +const tmatrix_row
>> + dw 64, 89, 84, 75, 64, 50, 35, 18
>> + dw 64, -18, -84, 50, 64, -75, -35, 89
>> + dw 64, 75, 35, -18, -64, -89, -84, -50
>> + dw 64, -50, -35, 89, -64, -18, 84, -75
>> + dw 64, 50, -35, -89, -64, 18, 84, 75
>> + dw 64, -75, 35, 18, -64, 89, -84, 50
>> + dw 64, 18, -84, -50, 64, 75, -35, -89
>> + dw 64, -89, 84, -75, 64, -50, 35, -18
>> +
>> +; Constant pairs for broadcast in column transform.
>> +const tmatrix_col_even
>> + dw 64, 64, 64, -64
>> + dw 84, 35, 35, -84
>> +const tmatrix_col_odd
>> + dw 89, 75, 50, 18
>> + dw 75, -18, -89, -50
>> + dw 50, -89, 18, 75
>> + dw 18, -50, 75, -89
>> +
>> +; Memory targets for vpbroadcastd (register version requires AVX512).
>> +cextern pd_1
>> +const sixtyfour
>> + dd 64
>> +
>> +SECTION .text
>> +
>> +; void ff_apv_decode_transquant_avx2(void *output,
>> +; ptrdiff_t pitch,
>> +; const int16_t *input,
>> +; const int16_t *qmatrix,
>> +; int bit_depth,
>> +; int qp_shift);
>> +
>> +INIT_YMM avx2
>> +
>> +cglobal apv_decode_transquant, 6, 7, 16, output, pitch, input, qmatrix, bit_depth, qp_shift, tmp
>> +
>> + ; Load input and dequantise
>> +
>> + vpbroadcastd m10, [pd_1]
>> + lea tmpq, [bit_depthq - 2]
>
> lea tmpd, [bit_depthd - 2]
>
> The upper 32 bits of the register may have garbage.
Ah, I was assuming that lea had to be pointer-sized, but apparently it doesn't. Changed.
>> + movd xm8, qp_shiftd
>
> If you declare the function as 5, 7, 16, then qp_shift will not be loaded into a gpr on ABIs where it's on stack (Win64, and x86_32 if it was supported), and then you can do
>
> movd xm8, qp_shiftm
>
> Which will load it directly to the simd register from memory, saving one instruction in the prologue.
This seems like highly dubious magic since it is lying about the number of arguments.
I've changed it, but I want to check a Windows machine as well.
>> + movd xm9, tmpd
>> + vpslld m10, m10, xm9
>> + vpsrld m10, m10, 1
>> +
>> + ; m8 = scalar qp_shift
>> + ; m9 = scalar bd_shift
>> + ; m10 = vector 1 << (bd_shift - 1)
>> + ; m11 = qmatrix load
>> +
>> +%macro LOAD_AND_DEQUANT 2 ; (xmm input, constant offset)
>> + vpmovsxwd m%1, [inputq + %2]
>> + vpmovsxwd m11, [qmatrixq + %2]
>> + vpmaddwd m%1, m%1, m11
>> + vpslld m%1, m%1, xm8
>> + vpaddd m%1, m%1, m10
>> + vpsrad m%1, m%1, xm9
>> + vpackssdw m%1, m%1, m%1
>> +%endmacro
>> +
>> + LOAD_AND_DEQUANT 0, 0x00
>> + LOAD_AND_DEQUANT 1, 0x10
>> + LOAD_AND_DEQUANT 2, 0x20
>> + LOAD_AND_DEQUANT 3, 0x30
>> + LOAD_AND_DEQUANT 4, 0x40
>> + LOAD_AND_DEQUANT 5, 0x50
>> + LOAD_AND_DEQUANT 6, 0x60
>> + LOAD_AND_DEQUANT 7, 0x70
>> +
>> + ; mN = row N words 0 1 2 3 0 1 2 3 4 5 6 7 4 5 6 7
>> +
>> + ; Transform columns
>> + ; This applies a 1-D DCT butterfly
>> +
>> + vpunpcklwd m12, m0, m4
>> + vpunpcklwd m13, m2, m6
>> + vpunpcklwd m14, m1, m3
>> + vpunpcklwd m15, m5, m7
>> +
>> + ; m12 = rows 0 and 4 interleaved
>> + ; m13 = rows 2 and 6 interleaved
>> + ; m14 = rows 1 and 3 interleaved
>> + ; m15 = rows 5 and 7 interleaved
>> +
>> + vpbroadcastd m0, [tmatrix_col_even + 0x00]
>> + vpbroadcastd m1, [tmatrix_col_even + 0x04]
>> + vpbroadcastd m2, [tmatrix_col_even + 0x08]
>> + vpbroadcastd m3, [tmatrix_col_even + 0x0c]
>
> Maybe do
>
> lea tmpq, [tmatrix_col_even]
> vpbroadcastd m0, [tmpq + 0x00]
> vpbroadcastd m1, [tmpq + 0x04]
> ...
>
> To emit smaller instructions. Same for tmatrix_col_odd and tmatrix_row below.
150: 48 8d 05 00 00 00 00 lea 0x0(%rip),%rax # 157 <ff_apv_decode_transquant_avx2+0x157>
157: c4 e2 7d 58 00 vpbroadcastd (%rax),%ymm0
15c: c4 e2 7d 58 48 04 vpbroadcastd 0x4(%rax),%ymm1
162: c4 e2 7d 58 50 08 vpbroadcastd 0x8(%rax),%ymm2
168: c4 e2 7d 58 58 0c vpbroadcastd 0xc(%rax),%ymm3
18e: c4 e2 7d 58 05 00 00 vpbroadcastd 0x0(%rip),%ymm0 # 197 <ff_apv_decode_transquant_avx2+0x197>
195: 00 00
197: c4 e2 7d 58 0d 00 00 vpbroadcastd 0x0(%rip),%ymm1 # 1a0 <ff_apv_decode_transquant_avx2+0x1a0>
19e: 00 00
1a0: c4 e2 7d 58 15 00 00 vpbroadcastd 0x0(%rip),%ymm2 # 1a9 <ff_apv_decode_transquant_avx2+0x1a9>
1a7: 00 00
1a9: c4 e2 7d 58 1d 00 00 vpbroadcastd 0x0(%rip),%ymm3 # 1b2 <ff_apv_decode_transquant_avx2+0x1b2>
1b0: 00 00
Saves 6 bytes, but there is now a dependency which wasn't there before. Is it really better?
>> +
>> + vpmaddwd m4, m12, m0
>> + vpmaddwd m5, m12, m1
>> + vpmaddwd m6, m13, m2
>> + vpmaddwd m7, m13, m3
>> + vpaddd m8, m4, m6
>> + vpaddd m9, m5, m7
>> + vpsubd m10, m5, m7
>> + vpsubd m11, m4, m6
>> +
>> + vpbroadcastd m0, [tmatrix_col_odd + 0x00]
>> + vpbroadcastd m1, [tmatrix_col_odd + 0x04]
>> + vpbroadcastd m2, [tmatrix_col_odd + 0x08]
>> + vpbroadcastd m3, [tmatrix_col_odd + 0x0c]
>> +
>> + vpmaddwd m4, m14, m0
>> + vpmaddwd m5, m15, m1
>> + vpmaddwd m6, m14, m2
>> + vpmaddwd m7, m15, m3
>> + vpaddd m12, m4, m5
>> + vpaddd m13, m6, m7
>> +
>> + vpbroadcastd m0, [tmatrix_col_odd + 0x10]
>> + vpbroadcastd m1, [tmatrix_col_odd + 0x14]
>> + vpbroadcastd m2, [tmatrix_col_odd + 0x18]
>> + vpbroadcastd m3, [tmatrix_col_odd + 0x1c]
>> +
>> + vpmaddwd m4, m14, m0
>> + vpmaddwd m5, m15, m1
>> + vpmaddwd m6, m14, m2
>> + vpmaddwd m7, m15, m3
>> + vpaddd m14, m4, m5
>> + vpaddd m15, m6, m7
>> +
>> + vpaddd m0, m8, m12
>> + vpaddd m1, m9, m13
>> + vpaddd m2, m10, m14
>> + vpaddd m3, m11, m15
>> + vpsubd m4, m11, m15
>> + vpsubd m5, m10, m14
>> + vpsubd m6, m9, m13
>> + vpsubd m7, m8, m12
>> +
>> + ; Mid-transform normalisation
>> + ; Note that outputs here are fitted to 16 bits
>> +
>> + vpbroadcastd m8, [sixtyfour]
>> +
>> +%macro NORMALISE 1
>> + vpaddd m%1, m%1, m8
>> + vpsrad m%1, m%1, 7
>> + vpackssdw m%1, m%1, m%1
>> + vpermq m%1, m%1, q3120
>> +%endmacro
>> +
>> + NORMALISE 0
>> + NORMALISE 1
>> + NORMALISE 2
>> + NORMALISE 3
>> + NORMALISE 4
>> + NORMALISE 5
>> + NORMALISE 6
>> + NORMALISE 7
>> +
>> + ; mN = row N words 0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7
>> +
>> + ; Transform rows
>> + ; This multiplies the rows directly by the transform matrix,
>> + ; avoiding the need to transpose anything
>> +
>> + mova m12, [tmatrix_row + 0x00]
>> + mova m13, [tmatrix_row + 0x20]
>> + mova m14, [tmatrix_row + 0x40]
>> + mova m15, [tmatrix_row + 0x60]
>> +
>> +%macro TRANS_ROW_STEP 1
>> + vpmaddwd m8, m%1, m12
>> + vpmaddwd m9, m%1, m13
>> + vpmaddwd m10, m%1, m14
>> + vpmaddwd m11, m%1, m15
>> + vphaddd m8, m8, m9
>> + vphaddd m10, m10, m11
>> + vphaddd m%1, m8, m10
>> +%endmacro
>> +
>> + TRANS_ROW_STEP 0
>> + TRANS_ROW_STEP 1
>> + TRANS_ROW_STEP 2
>> + TRANS_ROW_STEP 3
>> + TRANS_ROW_STEP 4
>> + TRANS_ROW_STEP 5
>> + TRANS_ROW_STEP 6
>> + TRANS_ROW_STEP 7
>> +
>> + ; Renormalise, clip and store output
>> +
>> + vpbroadcastd m14, [pd_1]
>> + mov tmpd, 20
>> + sub tmpd, bit_depthd
>> + movd xm9, tmpd
>> + dec tmpd
>> + movd xm13, tmpd
>> + movd xm15, bit_depthd
>> + vpslld m8, m14, xm13
>> + vpslld m12, m14, xm15
>> + vpsrld m10, m12, 1
>> + vpsubd m12, m12, m14
>> + vpxor m11, m11, m11
>> +
>> + ; m8 = vector 1 << (bd_shift - 1)
>> + ; m9 = scalar bd_shift
>> + ; m10 = vector 1 << (bit_depth - 1)
>> + ; m11 = zero
>> + ; m12 = vector (1 << bit_depth) - 1
>> +
>> + cmp bit_depthd, 8
>> + jne store_10
>> +
>> +%macro NORMALISE_AND_STORE_8 1
>> + vpaddd m%1, m%1, m8
>> + vpsrad m%1, m%1, xm9
>> + vpaddd m%1, m%1, m10
>> + vextracti128 xm13, m%1, 0
>> + vextracti128 xm14, m%1, 1
>> + vpackusdw xm%1, xm13, xm14
>> + vpackuswb xm%1, xm%1, xm%1
>
> vpaddd m%1, m%1, m10
> vextracti128 xm14, m%1, 1
> vpackusdw xm%1, xm%1, xm14
> vpackuswb xm%1, xm%1, xm%1
>
> vextracti128 with 0 as third argument is the same as a mova for the lower 128 bits, so it's not needed.
Thinking about this a bit more makes me want to combine rows to not waste elements. It's not obvious that this is better, but how about:
%macro NORMALISE_AND_STORE_8 4
vpaddd m%1, m%1, m8
vpaddd m%2, m%2, m8
vpaddd m%3, m%3, m8
vpaddd m%4, m%4, m8
vpsrad m%1, m%1, xm9
vpsrad m%2, m%2, xm9
vpsrad m%3, m%3, xm9
vpsrad m%4, m%4, xm9
vpaddd m%1, m%1, m10
vpaddd m%2, m%2, m10
vpaddd m%3, m%3, m10
vpaddd m%4, m%4, m10
; m%1 = 32x4 A0-3 A4-7
; m%2 = 32x4 B0-3 B4-7
; m%3 = 32x8 C0-3 C4-7
; m%4 = 32x8 D0-3 D4-7
vpackusdw m%1, m%1, m%2
vpackusdw m%3, m%3, m%4
; m%1 = 16x16 A0-3 B0-3 A4-7 B4-7
; m%2 = 16x16 C0-3 D0-3 C4-7 D4-7
vpermq m%1, m%1, q3120
vpermq m%2, m%3, q3120
; m%1 = 16x16 A0-3 A4-7 B0-3 B4-7
; m%2 = 16x16 C0-3 C4-7 D0-3 D4-7
vpackuswb m%1, m%1, m%2
; m%1 = 32x8 A0-3 A4-7 C0-3 C4-7 B0-3 B4-7 D0-3 D4-7
vextracti128 xm%2, m%1, 1
vpsrldq xm%3, xm%1, 8
vpsrldq xm%4, xm%2, 8
vmovq [outputq], xm%1
vmovq [outputq + pitchq], xm%2
lea outputq, [outputq + 2*pitchq]
vmovq [outputq], xm%3
vmovq [outputq + pitchq], xm%4
lea outputq, [outputq + 2*pitchq]
%endmacro
NORMALISE_AND_STORE_8 0, 1, 2, 3
NORMALISE_AND_STORE_8 4, 5, 6, 7
>> + movq [outputq], xm%1
>> + add outputq, pitchq
>> +%endmacro
>> +
>> + NORMALISE_AND_STORE_8 0
>> + NORMALISE_AND_STORE_8 1
>> + NORMALISE_AND_STORE_8 2
>> + NORMALISE_AND_STORE_8 3
>> + NORMALISE_AND_STORE_8 4
>> + NORMALISE_AND_STORE_8 5
>> + NORMALISE_AND_STORE_8 6
>> + NORMALISE_AND_STORE_8 7
>> +
>> + RET
>> +
>> +store_10:
>> +
>> +%macro NORMALISE_AND_STORE_10 1
>> + vpaddd m%1, m%1, m8
>> + vpsrad m%1, m%1, xm9
>> + vpaddd m%1, m%1, m10
>> + vpmaxsd m%1, m%1, m11
>> + vpminsd m%1, m%1, m12
>> + vextracti128 xm13, m%1, 0
>> + vextracti128 xm14, m%1, 1
>> + vpackusdw xm%1, xm13, xm14
>
> Same.
A similar method for pairs applies here as well.
>> + mova [outputq], xm%1
>> + add outputq, pitchq
>> +%endmacro
>> +
>> + NORMALISE_AND_STORE_10 0
>> + NORMALISE_AND_STORE_10 1
>> + NORMALISE_AND_STORE_10 2
>> + NORMALISE_AND_STORE_10 3
>> + NORMALISE_AND_STORE_10 4
>> + NORMALISE_AND_STORE_10 5
>> + NORMALISE_AND_STORE_10 6
>> + NORMALISE_AND_STORE_10 7
>> +
>> + RET
>> ...
Thanks,
- Mark
More information about the ffmpeg-devel
mailing list