On 4/21/2025 4:50 PM, Mark Thompson wrote:
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_shiftdIf 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.
You're not lying. That value is to tell x86inc to load x arguments onto gprs in the prologue if they are in stack. If they are not (As is the case with the first six arguments on Unix64, first four on Win64), qp_shiftm will be equivalent of qp_shiftd, and if they are, it will be the stack memory address.
So with my suggestion, on Win64 you get movd xmm8, [rsp]; qp_shiftm points to memory instead of mov r11, [rsp] ; prologue loads argument into what will be qp_shiftd movd xmm8, r11d ; qp_shiftd is an alias of r11d It's ricing, yes, but it's free.
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?
You could do the lea several instructions earlier, so the dependency wouldn't matter, but unless you can measure a difference in speed, then maybe don't bother.
+ + 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%1vpaddd 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:
It may be better just for having six crosslane instructions instead of eight.
%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]
Maybe instead load pitch*3 onto tmpq outside of the macro lea tmpq, [pitchq+pitchq*2] Then you can do: vmovq [outputq], xm%1 vmovq [outputq+pitchq], xm%2 vmovq [outputq+pitchq*2], xm%3 vmovq [outputq+tmpq], xm%4 lea outputq, [outputq+pitchq*4] Inside it.
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, xm14Same.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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
