Add NEON for fwd_txfm - #5409
Conversation
|
Please note, this PR only partially covers the various transform sizes and uses many c-fallbacks still. |
| const int32x4_t t0 = vaddq_s32(vmulq_s32(cos_t, c1), vmulq_s32(sin_t, c2)); | ||
| const int32x4_t t1 = vsubq_s32(vmulq_s32(cos_t, c2), vmulq_s32(sin_t, c1)); |
There was a problem hiding this comment.
Use fused multiply-accumulate (vmlaq_s32 / vmlsq_s32):
Instead of 4 separate vmulq_s32 + vaddq_s32 + vsubq_s32, you can use vmlaq_s32 (MLA) and vmlsq_s32 (MLS) compute t0 and t1.
There was a problem hiding this comment.
Done. This micro benchmarks out to roughly the same speed as this function is memory bound on my machine.
| vst1q_s32(&src_c1[i], r0); | ||
| vst1q_s32(&src_c2[i], r1); | ||
| } | ||
| for (; i < ncoeffs; i++) { |
There was a problem hiding this comment.
ncoeffs is always multiple of 16, so this block is dead code.
There was a problem hiding this comment.
Another miss on my end, removed.
| transpose_4x8_s32(res_lo, fin_lo); | ||
| transpose_4x8_s32(res_hi, fin_hi); |
There was a problem hiding this comment.
Same here - reuse transpose_arrays_s32_4x4
| #include "av2/encoder/fwd_txfm_internal.h" | ||
| #include "avm_dsp/txfm_common.h" | ||
|
|
||
| static INLINE void transpose_store_4x4_s32(int32x4_t r0, int32x4_t r1, |
There was a problem hiding this comment.
You can reuse transpose_elems_inplace_s32_4x4 function in avm_dsp/arm/transpose_neon.h
There was a problem hiding this comment.
Done, I replaced an additional transpose too that required some more setup, but the disassembly is the same.
jjustiss-apple
left a comment
jjustiss-apple
left a comment
There was a problem hiding this comment.
I believe I've addressed all your concerns, Jerome. Thank you for the very thorough review!
| const int32x4_t t0 = vaddq_s32(vmulq_s32(cos_t, c1), vmulq_s32(sin_t, c2)); | ||
| const int32x4_t t1 = vsubq_s32(vmulq_s32(cos_t, c2), vmulq_s32(sin_t, c1)); |
There was a problem hiding this comment.
Done. This micro benchmarks out to roughly the same speed as this function is memory bound on my machine.
| #include "av2/encoder/fwd_txfm_internal.h" | ||
| #include "avm_dsp/txfm_common.h" | ||
|
|
||
| static INLINE void transpose_store_4x4_s32(int32x4_t r0, int32x4_t r1, |
There was a problem hiding this comment.
Done, I replaced an additional transpose too that required some more setup, but the disassembly is the same.
| vst1q_s32(&src_c1[i], r0); | ||
| vst1q_s32(&src_c2[i], r1); | ||
| } | ||
| for (; i < ncoeffs; i++) { |
There was a problem hiding this comment.
Another miss on my end, removed.
| transpose_4x8_s32(res_lo, fin_lo); | ||
| transpose_4x8_s32(res_hi, fin_hi); |
e1ab6a4 to
045f6d4
Compare
4 participants
The forward transform pipeline (fwd_txfm) is a significant encoder hotspot
with no NEON specialization. All 1D kernels run scalar C on ARM.
Add NEON intrinsics for the full fwd_txfm pipeline: all 1D kernels
(DCT2, ADST, FDST, IDTX, DDTX, FDDT) for sizes 4-32, matrix-multiply
NEON kernels, a cross-chroma transform (CCTX) NEON path, and
avm_highbd_fdct8x8.
Micro-benchmark results (Apple M3 Pro P-core, 99% CI):
CTC results (RA, cpu-used=1, 33 frames, A5):
Unit tests: FwdTxfmVariantTest (5160 parameterized C-vs-RTCD cases),
HighbdFdct8x8Test (bit-exact + boundary), FwdCctxTest (54 combinations).