Skip to content

ci: keep the fork 64-bit only and record the build matrix as it runs (ADR-1258, ADR-1259) - #1485

Closed
lusoris wants to merge 15 commits into
masterfrom
ci/retire-i686-lane
Closed

lusoris wants to merge 15 commits into
masterfrom
ci/retire-i686-lane

Conversation

@lusoris

@lusoris lusoris commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR brings the CI records in line with the matrix that actually runs, and carries out the maintainer's decision to keep the fork 64-bit only.

ADR-1258: the fork stays 64-bit only.

  • History. ADR-0691 (9aa008e70, #1564) removed the Ubuntu i686 gcc lane. The libvmaf/core/ rename squash (384d97d03) restored it an hour later by accident. It then ran compile-only with -Denable_asm=false and no tests; ADR-1234's preflight m32 stage was built on it.
  • What a native i686 run showed: 7 x87 test failures, and 3 x86-64-only intrinsics the lane never exercised.
  • Removed: the lane, its dependency step, its matrix.i686 conditions, and preflight's m32 stage. This amends ADR-1234's stage list.
  • Kept as hygiene, not a support promise: the intrinsics now have 32-bit-safe forms. That is Netflix#1481's cause: extract_epi64_128() beside the existing extract_epi64(), and _mm_storel_epi64 in psnr_avx2.c. x86-64 code is unchanged; the fast suite passes 137/137.

ADR-1259: the CI build matrix as it runs. It supersedes ADR-0689, ADR-0691, ADR-0710 and ADR-0728.

  • ADR-0728 was never implemented. Its merge (bfd4c436b, feat!: sunset legacy native build modes (BREAKING, ADR-0728) #52) changed only its own ADR, a changelog fragment and deprecations.md. Its 14 lane removals and aggregator changes never happened.
  • Later ADRs rely on those lanes (ADR-1225, ADR-1234, ADR-1253, ADR-1254).
  • The ADR tabulates all 20 lanes with their required or advisory status and owning ADR, and records the maintainer's decisions: keep every lane, and keep Cppcheck and the three Sanitizers required.

Records corrected to match.

  • deprecations.md; release.md (40 required checks); ci-job-names.md (65 wrong "Previous Name" rows); ci.md.
  • The build.yml and sanitizers.yml headers; windows-d3d11-import.md; metal/index.md.
  • .github/AGENTS.md; core/src/hip/AGENTS.md; the backend pages.
  • Changelog fragments that announced removals that never happened are deleted or rewritten; the whole changelog is still Unreleased.

Also:

Found, recorded, not fixed here. Windows MSVC+CUDA is reported by both libvmaf-build-matrix.yml (required) and build.yml, so the aggregator's newest-run rule can count the wrong job. The state row is T-CI-MSVC-CUDA-SHARED-CHECK-NAME-2026-09-18. Fixing it renames a required check, which is a maintainer decision.

no docs needed: CI and decision records only; the docs changes above correct existing pages, and no user-facing option or output changes.

Type

  • feat — new feature
  • fix — bug fix
  • perf — performance improvement
  • refactor — no behavior change
  • docs — documentation only
  • test — test-only
  • build / ci — tooling / infra
  • port — cherry-pick from upstream Netflix/vmaf
  • sycl / cuda / simd — backend-specific

Checklist

  • Commits follow Conventional Commits (the commit-msg hook enforces this).
  • make format && make lint is green locally: pre-commit, both praetor audits, compile-context --verify, actionlint on the three touched workflows, check-aggregator-names.sh (40 match), mkdocs build --strict (0 warnings). clang-tidy and cppcheck report 0 findings on the three x86 files.
  • Unit tests pass: meson test -C build: x86-64 fast suite 137/137. scripts/dev/preflight.sh passes all six stages.
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2: 0. On x86-64 the ADM change maps back to _mm_extract_epi64 through the macro, and the PSNR change stores the same low 64 bits.
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap: not applicable.
  • If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header: no new files.
  • If this is a breaking change, the commit message uses ! or BREAKING CHANGE:: not breaking; 32-bit x86 was already unsupported per ADR-0691.
  • If this PR adds an ADR, the ADR row lives in docs/adr/_index_fragments/<NNNN-slug>.md and the slug is appended to docs/adr/_index_fragments/_order.txt: ADR-1258 and ADR-1259.

Bug-status hygiene (ADR-0165)

  • docs/state.md: T-CI-I686-LANE-RESURRECTED-2026-09-18 and T-X86-64-ONLY-INTRINSICS-2026-09-18 closed; T-CI-MSVC-CUDA-SHARED-CHECK-NAME-2026-09-18 opened.

Netflix golden-data gate (ADR-0024)

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.
  • If I believe a golden value must change, I have explained why below: no score changes.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: ADR-1258 and ADR-1259 carry the measurements and the commit evidence in their Context.
  • Decision matrix## Alternatives considered in docs/adr/1258-keep-64-bit-only-retire-i686-lane.md and docs/adr/1259-ci-build-matrix-as-it-runs.md.
  • AGENTS.md invariant notecore/src/feature/x86/AGENTS.md (no x86-64-only intrinsics), .github/AGENTS.md, core/src/hip/AGENTS.md.
  • Reproducer / smoke-test command — below.
  • CHANGELOG fragmentchangelog.d/removed/ci-i686-lane-retired-again.md, changelog.d/fixed/x86-64-only-intrinsics.md.
  • Rebase notedocs/rebase-notes.md: no i686 row (a merge that restores one is the bug that happened); keep extract_epi64_128() and the _mm_storel_epi64 read.

Reproducer

# the x86 sources compile for 32-bit x86 with asm (Netflix#1481)
meson setup build-i686 core --cross-file build-aux/i686-linux-gnu.ini -Denable_asm=true \
  -Denable_cuda=false -Denable_sycl=false -Db_lto=false && ninja -C build-i686
# CI records agree with the workflows
bash scripts/ci/check-aggregator-names.sh
python -m mkdocs build --strict

Known follow-ups

  • T-CI-MSVC-CUDA-SHARED-CHECK-NAME-2026-09-18: rename one of the two Windows MSVC+CUDA jobs (a required check name).
  • Five lanes that ADR-0689/0710 removed only on paper (Ubuntu gcc, Ubuntu clang, macOS clang, Ubuntu ARM clang, Ubuntu CUDA) are recorded as running; the maintainer's keep-decision covered the advisory lanes it listed, and these five want an explicit confirmation.
  • The three Windows lanes skip the impact planner and build on every non-draft PR.

Lusoris added 14 commits September 19, 2026 13:15
…lane (ADR-1258)

ADR-0728 removed the i686 lane; the libvmaf/ -> core/ rename merge brought it
back the same day and it ran compile-only since. ADR-1258 upholds ADR-0728:
the lane and preflight's m32 stage go, and the x86-64-only intrinsics get
32-bit-safe forms as hygiene. ADR-0151 is marked superseded by ADR-0728.
…sources

adm_avx2.c and adm_avx512.c called _mm_extract_epi64 on 12 lines each, next
to an extract_epi64() fallback that covered only the 256-bit form, and
psnr_avx2.c called _mm_cvtsi128_si64. GCC declares both only on x86-64, which
is Netflix#1481: no 32-bit x86 build with asm. extract_epi64_128() maps to
_mm_extract_epi64 on x86-64 and to a 32-bit fallback elsewhere; the PSNR sum
is read with _mm_storel_epi64. x86-64 code is unchanged. The fork stays
64-bit only (ADR-1258); this keeps the sources portable.
…lines

The 32-bit-safe extract_epi64_128() helper in adm_avx2.c and adm_avx512.c
moves 19 baselined HISS-04 functions down by 10 lines. Their sizes are
unchanged and the total stays at 1627 infractions, so only the recorded
line numbers change. Recorded with the pinned engine
(~/.cache/vmafx-praetor/praetorctl baseline --record, e4b35cb3c7fe) in a
fresh clone of the rebased branch, per the ADR-1249 rebase note.
ADR-0691 retired 32-bit x86; the libvmaf/ -> core/ rename merge (384d97d)
restored the lane the same day. It ran compile-only with -Denable_asm=false
and no tests. Per ADR-1258 the fork stays 64-bit only: the matrix row, its
dependency step and its matrix.i686 conditions go, and preflight drops the
m32 stage that mirrored the lane. AGENTS.md (recompiled into CLAUDE.md and
the other agent context files), the preflight page and the ADM page no
longer describe 32-bit as covered.

The shellcheck findings actionlint reports in this workflow are fixed on the
way: quoted redirections and substitutions, and pkg-config flags read into
arrays instead of relying on word splitting.
State rows T-CI-I686-LANE-RESURRECTED-2026-09-18 and
T-X86-64-ONLY-INTRINSICS-2026-09-18 (closed), changelog fragments, a rebase
note (no i686 row; keep extract_epi64_128() and the _mm_storel_epi64 read)
and an x86 AGENTS invariant.
#1384 committed both .md.stub reservation files alongside the finished ADRs.
A stub is a working-tree marker that the real ADR replaces (ADR-0535); these
two only kept numbers 1223 and 1224 looking claimed twice.
9aa008e (#1564), the commit that removed the i686 lane, implemented
ADR-0691. ADR-0728 (bfd4c43, #52) landed after the rename merge had
already restored the lane and changed only its own ADR, a changelog
fragment and deprecations.md. ADR-1258, its index row, the state row, the
changelog fragment and the rebase note now say so, and the preflight page,
preflight.sh and the ADM page cite ADR-1258 for the 64-bit-only rule.

ADR-0151 is marked superseded by ADR-1258 instead of ADR-0728: ADR-0691's
removal was undone the same day, so the lane ADR-0151 added ran until
ADR-1258 removed it, and ADR-1258 is the record that is in force.
Four Accepted ADRs described a matrix CI does not run. ADR-0689 (0ab991a)
and ADR-0691 (9aa008e) removed lanes that the libvmaf/ -> core/ rename
merge 384d97d restored about an hour later. ADR-0710 (4e21173) added
build.yml and sanitizers.yml but removed nothing and left the aggregator
alone. ADR-0728 (bfd4c43) changed no workflow at all.

ADR-1259 lists every lane in libvmaf-build-matrix.yml and build.yml with
its required status and owning ADR, records the maintainer's three
decisions (64-bit only via ADR-1258; the five lanes without a later ADR
stay, not required; Cppcheck and the three Sanitizers jobs stay
required) and supersedes all four ADRs. Their status lines and index rows
are flipped, and the index rows for ADR-0691 and ADR-0710 now describe
what those ADRs decided.

It also records that two jobs report the required check name
"Windows MSVC+CUDA", so the aggregator counts only whichever started
last; that is opened as T-CI-MSVC-CUDA-SHARED-CHECK-NAME-2026-09-18.
ADR-1259 records the build matrix as it runs; these pages and fragments
still described ADR-0689, ADR-0691, ADR-0710 and ADR-0728 as carried out.

- deprecations.md: the ADR-0728 entry is marked withdrawn and says what
  became of each configuration it listed (MinGW64, Cppcheck and the three
  sanitizers are required checks); a 2026-09-18 entry records the i686
  lane; tox does not run in the Linux all-backends lane.
- changelog.d: the unreleased native-build-sunset, 0691 and 0689 fragments
  announced removals that never happened and are deleted; ci-slim-down-v2
  now says build.yml and sanitizers.yml were added alongside the matrix.
  CHANGELOG.md is re-rendered.
- release.md: the required-check inventory has 40 names, not 34
  (including the Standards & Invariant Verification Gate from ADR-1249),
  and Semgrep is required.
- ci-job-names.md: the "Previous Name" column now holds the names #1286
  actually replaced (from the diff of f93a003), the eleven non-required
  matrix lanes and six later required checks are added, and the count
  is corrected to 40 names across eleven workflows.
- ci.md: lists build.yml and sanitizers.yml and points at ADR-1259.
- windows-d3d11-import.md: Windows CI is not "MinGW + no-SYCL"; the
  required Windows MSVC+SYCL lane compiles d3d11_import.cpp.
- metal/index.md: the Metal lanes are not required checks.
- .github/AGENTS.md: the MoltenVK lane section is replaced by the ADR-1259
  invariant; macOS clang and macOS clang+DNN are the continue-on-error
  rows; current Tidy SYCL job name.
- core/src/hip/AGENTS.md: the lane is "Ubuntu HIP" and installs ROCm 10.
- rebase-notes.md: do not restore the deleted fragments or drop lanes in
  a merge resolution.
build.yml's header said it supersedes libvmaf-build-matrix.yml and that
the ARM, static and DNN legs were retired; sanitizers.yml's said it
replaced the per-PR sanitizer matrix. Neither happened (ADR-1259): both
workflows run alongside the ones they were meant to replace, and none of
their jobs is a required check. The headers now say so, and build.yml's
notes that its Windows row shares the required name Windows MSVC+CUDA.

actionlint findings in sanitizers.yml are fixed on the way: the
workflow_dispatch trigger now declares the max_total_time input that
fuzz-nightly already reads (default 60, as before, mirroring fuzz.yml),
and the -max_total_time argument is quoted. No job, trigger or required
check changes.
lanes that no longer exist: "Build — Ubuntu ARM clang (CPU)",
"Build — Ubuntu HIP", "Build — macOS clang (CPU)", "Build — Windows MSVC +
CUDA (build only)", "Build — Ubuntu SYCL". They now use the names the
checks report under. Facts in the same sentences are corrected against
the workflows: the ARM lane runs on ubuntu-24.04-arm, the Windows CUDA
leg installs 13.3.1 (packages *_13.3, CUDA_PATH_V13_3), and oneAPI comes
from Intel's apt repository at ONEAPI_VERSION (Linux) and a pinned
offline installer (Windows), not from the intel/oneapi-runtime-toolkit
action.
origin/master moved past 7cc0cc9 while ADR-1259 was written, so the
evidence sentence in ADR-1259 and the T-CI-MSVC-CUDA-SHARED-CHECK-NAME
state row name the master commit instead of calling it origin/master.
…ared names

libvmaf-build-matrix.yml's required lane and build.yml's Windows row both
reported as 'Windows MSVC+CUDA'. The required checks aggregator keeps one run
per name, the newest, so either job could mask the other's failure. The
build.yml job becomes 'Windows MSVC+CUDA (full)' (the maintainer's choice; the
required name and the ruleset stay as they are).

check-aggregator-names.sh compared sets of names and could not see the
duplicate. It now also fails when more than one job reports a required name,
ignoring step names and workflow titles. A fixture test covers the shared-name
case and runs in rule-enforcement.yml. ADR-1259 and the CI pages record the
rename; T-CI-MSVC-CUDA-SHARED-CHECK-NAME-2026-09-18 is closed.
The union resolution left three copies: the pre-correction wording that credited ADR-0728, and two copies of the corrected one that credits ADR-0691. Keeps the corrected row. Also carries the HISS baseline re-recorded from a clean clone with the pinned engine: the CAMBI SIMD dispatch merged in the meantime and moved the lines the line-keyed file tracks in cambi.c; no finding is new.
@lusoris
lusoris force-pushed the ci/retire-i686-lane branch from 35f3820 to 4937dd3 Compare September 19, 2026 11:21
The rebase left the rendered README carrying both the pre-correction ADR-1258 row and the corrected one, while _index_fragments/ held only the corrected fragment. Regenerated with concat-adr-index.sh and generate-adr-by-tag.sh, which the docs freshness gate compares against.
@lusoris

lusoris commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

Folded into #1497 together with the other two, and closing here.

The three kept invalidating each other: each one touches docs/state.md, CHANGELOG.md, the ADR index and .standards-baseline.json, so landing any one made the other two conflict and need a rebase, a fresh baseline record and a fresh CI run. #1497 carries all three resolved once, with one CI run.

Nothing was dropped. Every source path of this branch is byte-identical in #1497, verified with git diff over the branch's own files. The baseline there was re-recorded from a clean clone and agreed with the folded value exactly.

@lusoris lusoris closed this Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:ci CI and infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant