Repository navigation
perf(cambi): vectorize the spatial-mask dp and mask rows on AVX2, AVX-512 and NEON - #1472
Merged
Merged
Conversation
lusoris
force-pushed
the
perf/cambi-spatial-mask-simd
branch
from
September 18, 2026 12:13
6bddcaa to
ccda800
Compare
This was referenced Sep 18, 2026
…rnel calculate_c_values_row_neon carried the scalar per-pixel c-value body twice, once for eight-column blocks with an active mask and once for the tail, which put the function over the readability-function-size budget and five levels deep. The body now lives in one static helper that takes the row-invariant inputs as a struct, and both loops call it. The helper keeps the scalar operation order, so the output is unchanged; test_cambi_simd passes under qemu-aarch64. The row pointer arithmetic now widens to ptrdiff_t before multiplying, the file includes its own header so its definitions are checked against their prototypes, and test_cambi.c declares the AVX2-only histogram buffer inside the ARCH_X86 guard, which removes an unused-variable warning on aarch64.
…-512 and NEON
CAMBI builds its spatial mask once per frame from two row kernels,
compute_dp_row (one summed-area-table row) and compute_mask_row (the box sum
compared against mask_index), and both ran scalar on every ISA. This adapts
upstream Netflix/vmaf 86da14d03 ("feature/cambi: AVX2 vectorize spatial-mask
dp row and mask row"), whose AVX2 kernels land in cambi_avx2.c under the
file's existing Netflix notice, and adds AVX-512 and NEON twins.
The port differs from upstream in two places. Upstream's AVX2 dp row carries
its running prefix through a lane broadcast of the already-carried scan, which
measured 0.64-0.74x of scalar when built with Clang or icx. The fork's version
adds the block total to the carry, so a single add is loop-carried, and runs
1.8-2.3x faster than scalar at 1080p on Zen 5 across GCC, Clang and icx. The
AVX2 mask row biases both compare operands by 2^31, which makes the signed
vpcmpgtd equal to the scalar unsigned compare for every input rather than only
for box sums below 2^31. AVX-512 uses the same structure with an unsigned
compare and runs 2.6-3.2x (dp) and 2.1-2.3x (mask) faster than scalar.
Dispatch follows the existing setup_callbacks pattern. The scalar kernels are
the default binding, AVX2 upgrades both, AVX-512 (under HAVE_AVX512) upgrades
both, and aarch64 upgrades the dp row to NEON. The NEON mask row is built and
tested but not dispatched, because GCC and Clang already compile the scalar
loop to the same NEON instructions (ADR-1256). The scalar kernels become the
non-static reference functions declared in cambi.h, as upstream made them, and
the GPU twins' spatial-mask trampoline keeps passing the scalar kernels.
test_cambi_spatial_mask_simd checks every twin byte for byte against the
production scalar. It covers the dp row over every tail residue of both vector
widths, random widths, pad sizes 1 to 8 and wrapping uint32 inputs; the mask
row with box sums planted around thresholds that include 2^31; and the whole
row recurrence for several heights and filter sizes. CAMBI scores at
--precision max are identical to master for 8-bit and 10-bit inputs, odd
widths and full-reference mode, under default, AVX2-only and scalar dispatch,
and on aarch64 under qemu.
…ispatch decision The CAMBI metric page gains a CPU SIMD section. It lists, per stage, which AVX2, AVX-512 and NEON kernels the extractor dispatches, gives the measured speed-ups of the spatial-mask row kernels, and shows how to compare paths with --cpumask. It also states plainly that several older AVX-512 and NEON CAMBI kernels are built but not dispatched, and the arm backend overview no longer claims a full NEON CAMBI path. ADR-1256 records the rule applied here: a spatial-mask row kernel is dispatched on an ISA only when it beats scalar on a measured run, or, where it cannot be timed, when its instruction count clearly drops. Research-2062 holds the benchmark method and numbers for GCC, Clang and icx, the reason upstream's AVX2 dp row loses to scalar under Clang and icx, and the NEON instruction counts. The changelog fragment, the rebase note for upstream 86da14d03, and the x86 and arm64 AGENTS.md rows mark what a future upstream sync must keep.
…onto master Recorded from a clean clone with the pinned engine. The branch predates the governance adoption, so its changes to cambi.c shifted the lines the baseline keys on. 1,623 to 1,622; no finding is new.
The maintainer accepted the measure-before-dispatch rule for CAMBI's spatial-mask row kernels on 2026-09-19 (popup Q3.5). Status, index row and tag pages updated.
lusoris
force-pushed
the
perf/cambi-spatial-mask-simd
branch
from
September 19, 2026 09:57
ccda800 to
5940e14
Compare
lusoris
added a commit
that referenced
this pull request
Sep 19, 2026
…limit clang-format spreads a braced initializer over one line per field, so the eight-case table was a 96-line brace block, which the HISS-04 size check counts as a function. One case per line inside a format-off region, as the HIP option tables already do. The data is unchanged. Also drops the pre-correction copy of the CAMBI state row that the keep-both rebase resolution kept twice, and re-records the standards baseline from a clean clone with the pinned engine: #1472's squash moved the lines the line-keyed file tracks in cambi.c. Total is unchanged at 1,622.
11 of 18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CAMBI's spatial-mask
compute_dp_rowandcompute_mask_rowran scalar on every ISA. They now have AVX2, AVX-512 and NEON kernels, each dispatched only where it measurably beats the scalar it replaces (ADR-1256, Proposed). CAMBI scores are byte-identical at--precision max.Upstream Netflix/vmaf added AVX2 versions in
86da14d03. Taken as-is, upstream's AVX2 dp row is slower than scalar under Clang (0.74x) and icx (0.64x), and icx builds the published container. So the dp row is rewritten to carry one add per block instead of a serial chain. The AVX2 mask row biases its signed compare so it matches scalar for every input, where upstream's plain signed compare breaks near 2^31.Speed-up vs the production scalar at 1080p on Zen 5:
The NEON dp row is wired: 8 columns go from roughly 56–80 scalar instructions to 26, with one loop-carried add instead of eight. The NEON mask row is not wired, because GCC and Clang already compile the scalar loop to the same instruction count. Its kernel stays in the tree and in the parity test. The two row kernels are about 2% of a 1080p CAMBI frame, so the end-to-end gain is small. The point of the change is closing the backend gap.
Reviewer decision: ADR-1256, the measure-before-dispatch rule.
Type
feat— new featurefix— bug fixperf— performance improvementrefactor— no behavior changedocs— documentation onlytest— test-onlybuild/ci— tooling / infraport— cherry-pick from upstream Netflix/vmafsycl/cuda/simd— backend-specificChecklist
make format && make lintis green locally.meson test -C build./cross-backend-diffand the worst ULP is ≤ 2: integer kernels, bit-exact to scalar. 21 cambi JSONs are byte-identical toorigin/masterunder default dispatch, AVX2-only and scalar, on x86 and on the aarch64 cross build under qemu..c/.cpp/.cu/.h/.hpp, it has the appropriate license header (seeCONTRIBUTING.md).!orBREAKING CHANGE:and the migration path is documented below: not breaking.docs/adr/_index_fragments/<NNNN-slug>.mdand the slug is appended todocs/adr/_index_fragments/_order.txt.Bug-status hygiene (ADR-0165)
no state delta: a performance change; no tracked bug is opened, closed or re-scoped. The dead AVX-512/NEON CAMBI kernels found along the way are listed under Known follow-ups and get their own row in the follow-up that re-wires them.Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests.Deep-dive deliverables (ADR-0108)
docs/research/2062-cambi-spatial-mask-simd.md: per-compiler measurements, the upstream comparison and the NEON instruction counts.docs/adr/1256-cambi-spatial-mask-simd-dispatch.md,## Alternatives considered.core/src/feature/x86/AGENTS.mdandcore/src/feature/arm64/AGENTS.md.changelog.d/changed/perf-cambi-spatial-mask-simd.md.docs/rebase-notes.md: upstream86da14d03adapted, and what differs.Reproducer
Known follow-ups
e3fd1c88a, so they are dead code and CAMBI runs AVX2 or scalar there.docs/backends/arm/overview.mdclaimed NEON CAMBI; that claim is corrected here. Re-wiring or retiring them is the next CAMBI change.