Skip to content

Add NEON for cwp_convolve_x, cwp_convolve_y, cwp_convolve_2d_copy - #5508

Open
jjustiss-apple wants to merge 2 commits into
AOMediaCodec:av2-encfrom
jjustiss-apple:jjustiss/neon-cwp-convolve
Open

jjustiss-apple wants to merge 2 commits into
AOMediaCodec:av2-encfrom
jjustiss-apple:jjustiss/neon-cwp-convolve

Conversation

@jjustiss-apple

Copy link
Copy Markdown
Contributor

Adds optimized NEON intrinsics for av2_highbd_cwp_convolve_x,
av2_highbd_cwp_convolve_y, and av2_highbd_cwp_convolve_2d_copy.
Also improves the existing 2D path (landed in #5425) with symmetric
coefficient folding and MODE/RBITS dispatch.

Optimization techniques: immediate-shift post-filter, symmetric
coefficient folding (6-tap), MODE/RBITS compile-time dispatch,
s32 halving add for compound avg, tap-count specialized kernels
(2/4/6/8/12-tap horizontal, 4/6/8-tap vertical).

Micro-benchmarks (Apple M2 Ultra P-core):

Kernel Speedup vs C
conv_y 2-tap 39.9x
conv_y 4-tap 28.4x
conv_y 6-tap sym 26.3x
conv_y 8-tap 21.6x
conv_x 2-tap 21.6x
conv_x 4-tap 15.9x
conv_x 6-tap sym 13.2x
conv_x 8-tap 10.8x
conv_2d 18.5x
conv_2d 8x8 12.8x
conv_2d_copy 26.3x

CTC Results (RA, cpu-used=1, 33 frames, A5):

Metric Delta
Encode time -1.2%
BD-rate Y 0.000%
BD-rate Cb 0.000%
BD-rate Cr 0.000%

@jjustiss-apple

Copy link
Copy Markdown
Contributor Author

@jianj-g This one is ready for review, it extends our previous work on the CWP _2d function PR #5425

// constant 3 or 5 and BITS is 0. Falls back to the original 4-op sequence
// for non-standard values.
// Captures round_const, round_shift, bits_shift from enclosing scope.
#define CWP_X_POST_FILTER(sum, ROUND0, BITS, offset) \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the callsites to this macro have BITS as 0 (also only called by other macros)

  1. (BITS) == 4 and (BITS) == 3 branches are dead code.
  2. at compile time, the branch else if ((BITS) > 0) will be ignored. I'm not sure what the consequences are from this... if BITS will always be 0, it can be removed from this macro.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right it's all dead code (now removed). The compiler was removing it which is why it didn't affect the speed.

I found that I can run clang -E to unfold the macros and then use regex to find literal comparisons, which flagged all these.

A challenge I'm learning with macros is that -Wunreachable-code is suppressed in clang, which requires extra handling.

Thank you for flagging this!

ConvolveParams *conv_params, int bd) {
const int tap_y = get_filter_tap(filter_params_y, subpel_y_qn);

(void)tap_y;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tap_y is used in line 3980

@jjustiss-apple jjustiss-apple Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

s8 = s9; \
s9 = s10; \
s10 = s11; \
height--; \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can unroll 4 rows each iteration here (same for CONV_Y_12TAP_8)

@jjustiss-apple
jjustiss-apple force-pushed the jjustiss/neon-cwp-convolve branch from 4b04cd0 to ef68e0e Compare October 9, 2026 15:22
@jjustiss-apple
jjustiss-apple requested a review from jianj-g October 9, 2026 15:57
@jjustiss-apple

Copy link
Copy Markdown
Contributor Author

@jianj-g everything should be addressed, thanks again for your thorough review. I'm tightening my own self-review processes and hopefully will have fewer of these dead code issues going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants