perf(cambi): dispatch AVX-512 and NEON for every CAMBI stage, and scan c-values columns on AVX2 - #1479
Merged
Conversation
lusoris
force-pushed
the
perf/cambi-spatial-mask-simd
branch
from
September 19, 2026 09:57
ccda800 to
5940e14
Compare
added 7 commits
September 19, 2026 13:26
Since the upstream optimisation batch was ported in e3fd1c8, which took upstream's AVX2-only dispatch block and dropped the macro that built the AVX-512 and NEON c-values drivers, the AVX-512 and NEON derivative rows, range updaters and c-values row kernels were compiled but never called, and neither ISA had a decimate, anti-dithering filter or mode filter at all. No correctness reason was recorded for the loss. This adds the missing kernels: AVX-512 decimate (vpermt2w), anti-dithering filter and mode filter, all with masked row tails, and NEON decimate (ld2), anti-dithering filter (widening adds, vhadd for the last row) and mode filter. The AVX-512 derivative row is rewritten with a masked tail; the old one left up to 32 scalar columns per row and was slower than AVX2 at 576x324. Both ISAs get a frame-level c-values driver on a shared walk (cambi_c_values_frame.h). It calls the same cambi.h update helpers as the scalar walk, but only for the columns a vector scan flags as needing a histogram update. On flat, banding-prone content almost every column of the scalar walk is masked out, out of band or cancels, so that per-column bookkeeping was the cost. Updates within a row commute and all land before the row's c-values are computed, so the output is byte-identical. The AVX-512 scans stay out of line so the walk keeps no vector value across its row-kernel calls; inlined, Clang spilled the hoisted broadcasts (ADR-1254). Dispatch follows the measure-first rule of ADR-1256 (Research-2063). On a Zen 5 host every AVX-512 stage beats AVX2 under GCC, Clang and icx: anti-dithering 1.2-2.5x, derivative 1.1-1.8x, decimate 1.25-1.5x, mode filter 1.08-1.4x, c-values 2.0-8.8x; a whole CAMBI frame runs 1.23-1.32x faster than the previous default under GCC and about 2x faster than AVX2 under Clang and icx. On aarch64, measured as qemu instruction counts, NEON removes 76-91 % of the instructions of anti-dithering, derivative and decimate and 56-79 % of c-values. The NEON mode filter removes none (0.93x GCC, 1.02x Clang: both compilers vectorise the scalar loop), so it stays undispatched and parity-tested, like compute_mask_row_neon. cambi_increment_range_neon / cambi_decrement_range_neon are retired: plain C loops in the driver compile to the same NEON adds, and the intrinsics measured 0.3-0.9 % more instructions. CAMBI JSON output at --precision max is byte-identical across default, AVX2-only and scalar dispatch, across GCC, Clang and icx builds, between NEON and scalar under qemu, and against the previous binary, on 18 inputs and option sets.
…er dispatch level test_cambi_stage_simd compares each per-stage SIMD kernel (decimate, anti-dithering filter, mode filter, derivative row, histogram range updates, c-values row and the frame-level c-values driver) on AVX2, AVX-512 and NEON with the shipped scalar stage. It includes cambi.c, as test_cambi.c does, so the reference is the static scalar code the extractor falls back to rather than a copy. Every buffer carries a sentinel guard and pictures get a wider stride and extra rows filled with a value no sample takes, and the whole allocation is compared, so a kernel that writes past its region, or reads past it and uses the value, fails. Widths cover every tail residue of the 8-, 16- and 32-lane loops; values cover 8-bit input as CAMBI converts it, 10-bit, full-range uint16 and low-entropy content; the c-values sweeps use the production TVI and visibility tables for max_log_contrast 0 to 5, BT.1886 and PQ, windows 3 to 65 and heights down to the smallest the walk supports, and compare the final histogram as well as the c-values. Deliberately broken kernels (one-element tail overruns, a dropped remainder, a wrong mode tie-break, a scan that misses columns) all fail it. test_cambi_dispatch_invariance drives the extractor through the public API at each dispatch level the host offers (host, AVX2 only and scalar on x86; host and scalar on aarch64) on banded fixtures covering 8- and 10-bit input, full-reference mode, max_log_contrast 5, the 1080p high-res speedup, window 33 and odd 4:4:4 sizes, and requires bit-identical per-frame scores. test_cambi's AVX2 c-values parity check gated on vmaf_get_cpu_flags(), which is 0 until vmaf_init_cpu() runs, and nothing in that binary runs it: the AVX2 branch never executed and the test passed without comparing anything (confirmed with a breakpoint on calculate_c_values_avx2, which was never hit). It now gates on CPUID directly.
…rements The CAMBI metric page's CPU SIMD section now has a per-stage table for AVX2, AVX-512 and NEON and says why the two aarch64 stages that stay scalar (the mask row and the mode filter) do: the compilers already vectorise those loops, so a NEON kernel removes no work. It explains why the AVX-512 and NEON c-values stage is so much faster than AVX2 (it skips the pixels that leave the histogram unchanged), gives the measured per-stage and whole-frame speed-ups, adds the aarch64 --cpumask example, and replaces the claim that the AVX-512 and NEON kernels predate the c-values layout, which was never the reason they were undispatched. The arm backend overview lists CAMBI's NEON coverage as full. Research-2065 holds the method (real frames, interleaved best-of-N timing under GCC, Clang and icx; qemu instruction counts for NEON), the per-stage and whole-frame numbers, why the c-values walk was bound by per-column bookkeeping rather than vector width, the register-pressure reason the AVX-512 scans stay out of line, and the decision for each previously dead kernel. docs/state.md closes T-CAMBI-SIMD-DEAD-KERNELS and T-CAMBI-AVX2-PARITY-TEST-NOOP and opens T-CAMBI-AVX2-CVALUES-LLVM: in Clang and icx builds, icx being the published container's compiler, the AVX2 c-values driver is about 0.8x of scalar. The two local-gate defects hit while validating this branch (check_exported_symbols in an ASan build, and preflight's 32-bit sweep on NEON sources) are fixed, with their own state rows, in #1475. The changelog fragment, the rebase note (all of this is fork-local; the dispatch additions must survive the next upstream rewrite of init()) and the x86 and arm64 AGENTS.md invariants complete the set.
The AVX2 dispatch bound upstream's calculate_c_values_avx2, which visits every column of the sliding-histogram walk. On flat, banding-prone content almost every column leaves the histogram unchanged, so that walk is per-column bookkeeping, and in icx builds (icx builds the published container) it ran at 0.81-0.83x of the scalar code. On a CPU with AVX2 but no AVX-512, the c-values stage was slower than scalar. calculate_c_values_scan_avx2 is the AVX2 twin of the AVX-512 and NEON drivers: the shared scanned walk in cambi_c_values_frame.h, with AVX2 column scans (16 uint16 lanes per compare, packed and movemasked into the 32-bit column masks, a scalar tail so no row is read past its last column) and the existing AVX2 row kernel and range updaters. The unsigned band test is spelled with min_epu16, since AVX2 has no unsigned 16-bit compare. The scans stay out of line and build their constants per call, so the walk keeps no ymm value across its row-kernel calls (ADR-1254). It beats both the upstream walk and scalar on all seven inputs under every compiler (c-values stage, Zen 5, min of 3 runs): GCC 2.09-2.78x scalar and 1.93-2.85x upstream AVX2, Clang 3.50-5.52x and 3.14-5.21x, icx 2.87-4.48x and 3.43-5.38x. A whole CAMBI frame at AVX2 only runs 1.21-1.23x (GCC), 1.58-1.65x (Clang) and 1.71-1.78x (icx) faster than before. So the AVX2 block now binds it; upstream's calculate_c_values_avx2 stays built because test_cambi and test_cambi_stage_simd still check it. The per-column scan predicates move into cambi_c_values_frame.h so the AVX2 and NEON row tails share one copy. CAMBI JSON at --precision max stays byte-identical to scalar and to the pre-change binary on the 18 inputs and option sets, at default, AVX2-only and scalar dispatch under GCC, Clang and icx, and NEON vs scalar under qemu.
…nd's edges test_cambi_stage_simd now checks every frame-level c-values driver an ISA carries: on AVX2 both the dispatched calculate_c_values_scan_avx2 and the upstream calculate_c_values_avx2 it replaced. A second frame fixture uses only the scored band's edge values and their neighbours (just below, first, second, last, just past) with random masks, so equal pixels in the two rows a slide compares, masked-out pixels equal to an unmasked one, and values one step outside the band are common. That is where a scan's skip test can go wrong: excluding the band's last value went unnoticed by the ramp fixture. test_cambi_dispatch_invariance gains two 10-bit cases on the band's edges: a one-code ramp whose top band is 563, the last value of the default BT.1886 band, with sparse 564s just past it, and a ramp across 67, the first value of the band once cambi_vis_lum_threshold is 0.06. Its AVX2-only level now runs the scanned driver. Planted faults in the AVX2 scans all fail test_cambi_stage_simd: a slide cancel that ignores the band test or needs only one side in band, the band's last or first value excluded, a slide scan that flags only the subtracted row, a row or slide tail that drops the last column, swapped pack halves, a dropped second 32-column mask, an inverted mask test, a missing slide scan (crash). All but the missing scan also fail test_cambi_dispatch_invariance at its AVX2-only level. The same cancel and band-top faults in the AVX-512 and NEON scans fail test_cambi_stage_simd too; before the edge fixture the band-top one did not.
…ate row docs/state.md moves T-CAMBI-AVX2-CVALUES-LLVM to Recently closed with the measurements: upstream's AVX2 c-values walk ran at 0.81-0.83x of scalar in icx builds (and 0.80-1.11x under Clang depending on the build), the scanned AVX2 driver runs at 2.09-5.52x scalar and 1.93-5.38x upstream across GCC, Clang and icx, and a whole frame at AVX2 only is 1.21-1.78x faster than the pre-change binary. Research-2065 gains the AVX2 section (method, per-compiler c-values and whole-frame tables, the Win64 stack check) and the planted-fault list, and drops the AVX2 gap from its open items. The CAMBI metric page explains that all three SIMD c-values drivers skip unchanged pixels, gives the c-values speed-ups against scalar, corrects the AVX-512 against AVX2 c-values row (1.04-1.28x against the scanned driver) and replaces the whole-frame table with default and AVX2-only numbers for each compiler. The rebase note records the one diverging line in upstream's AVX2 dispatch block and why upstream's calculate_c_values_avx2 stays built but unbound. The x86 AGENTS.md gains the AVX2 scan invariants, and the changelog fragment is rewritten to describe the final state.
…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.
lusoris
force-pushed
the
perf/cambi-simd-gaps-2
branch
from
September 19, 2026 11:34
afeda25 to
18f52fb
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /* The reference must be the shipped file-static scalar stages, not a copy | ||
| * (ADR-1207), so the TU is included, as test_cambi.c does. */ | ||
| // NOLINTNEXTLINE(bugprone-suspicious-include) — ADR-0141 / ADR-1207: white-box reference to the static scalar stages. | ||
| #include "feature/cambi.c" |
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
2 participants
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
AVX-512 and NEON now cover every CAMBI stage that AVX2 does, and AVX2's c-values stage runs through a faster fork-local walk. CAMBI scores are unchanged on every path (byte-identical
--precision maxoutput). On a Zen 5 host a whole frame runs 1.23–1.32× faster than before under GCC with default dispatch, and 1.21–1.78× faster at AVX2 only, depending on the compiler. Stacked on #1472; review #1472 first.Why kernels were dead.
e3fd1c88a, the port of upstream's CAMBI optimisation batch, took upstream's AVX2-only dispatch block and dropped the macro that built the AVX-512 and NEON c-values drivers. No reason is recorded, and those kernels already used the new histogram layout.The main change. The c-values stage was slow because of per-pixel bookkeeping, not vector width. On flat content almost every pixel leaves the column histogram unchanged, yet the walk visits each one. The new shared walk,
core/src/feature/cambi_c_values_frame.h:cambi.hupdate helpers only for columns that change something.The output is byte-identical, because updates within a row commute and every updating column runs the same helper with the same range. Simply re-wiring the old kernels into the per-pixel walk would have gained 1.05–1.17× on AVX-512 and cost NEON 31–36% more instructions than scalar.
AVX2 gets the same walk with 16-lane scans (
calculate_c_values_scan_avx2), replacing upstream'scalculate_c_values_avx2in dispatch. Upstream's driver stays built, becausetest_cambiandtest_cambi_stage_simdstill use it.c-values stage, 7 inputs × 3 runs, single thread, Zen 5:
icx builds the published container, so on CPUs with AVX2 but no AVX-512 that stage used to run slower than scalar.
Per stage, AVX-512 and NEON. AVX-512 speed-ups are over AVX2. NEON figures are qemu instruction counts as a fraction of scalar (no aarch64 hardware here).
Each kernel is dispatched only where it measured faster (ADR-1256's rule).
Bugs found on the way (none changes scores):
test_cambi's AVX2 c-values parity check never ran. It gated onvmaf_get_cpu_flags(), which is 0 untilvmaf_init_cpu()runs. It now reads CPUID; the AVX2 path had been bit-exact all along.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: 0 clang-tidy findings and 0 uncited NOLINTs on every touched file (x86 and aarch64); cppcheck clean; pre-commit and pre-push hooks pass.meson test -C build: fast suite 140/140 on x86 (GCC and Clang), 138/138 on aarch64 under qemu (GCC and Clang); CAMBI tests pass on icx and under ASan+UBSan./cross-backend-diffand the worst ULP is ≤ 2: 0. Byte-identical output on 18 input and option sets, covering default, AVX2-only and scalar dispatch under GCC, Clang and icx, the pre-change binary, and NEON vs scalar 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: no ADR; dispatch follows ADR-1256's measure-first rule from perf(cambi): vectorize the spatial-mask dp and mask rows on AVX2, AVX-512 and NEON #1472.Bug-status hygiene (ADR-0165)
docs/state.md:T-CAMBI-SIMD-DEAD-KERNELS-2026-09-18,T-CAMBI-AVX2-PARITY-TEST-NOOP-2026-09-18andT-CAMBI-AVX2-CVALUES-LLVM-2026-09-18closed.Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests.Deep-dive deliverables (ADR-0108)
docs/research/2065-cambi-simd-gaps.md.core/src/feature/x86/AGENTS.md,core/src/feature/arm64/AGENTS.md.changelog.d/changed/perf-cambi-simd-gaps.md.docs/rebase-notes.md: why upstream's AVX2 c-values driver is no longer dispatched, and the shared walk upstream's copies must stay in step with.Reproducer
Known follow-ups
ADR-1256 (in perf(cambi): vectorize the spatial-mask dp and mask rows on AVX2, AVX-512 and NEON #1472, decision pending) needs a note:
Both points should cite Research-2065; left untouched here while its decision is open.
No real aarch64 timing (qemu instruction counts only) and no Intel AVX-512 timing (Zen 5 only). The MinGW object was not built locally.