diff --git a/.github/workflows/lint-and-format.yml b/.github/workflows/lint-and-format.yml index 2f7cd9425..9111a7f98 100644 --- a/.github/workflows/lint-and-format.yml +++ b/.github/workflows/lint-and-format.yml @@ -383,8 +383,8 @@ jobs: # replaces icpx with clang++ and drops -fsycl so the entry is # consumable by stock clang-tidy. # 2. scripts/ci/clang-tidy-sycl.sh injects -isystem - # + -D__SYCL_DEVICE_ONLY__=0 (guards __spirv_ControlBarrier / ocl_event) - # + -Wno-unknown-warning-option + -Wno-unknown-pragmas. + # + -extra-arg-before=-std=c++20 + -Wno-unknown-warning-option + # + -Wno-unknown-pragmas. # # Advisory (continue-on-error: true) until one green master run confirms # the wrapper holds across all current SYCL TUs; gate tightens after that. @@ -514,9 +514,10 @@ jobs: # so it has nothing to gain from LTO anyway. CC=icx CXX=icpx meson setup build-sycl core \ -Denable_sycl=true -Denable_cuda=false -Db_lto=false - # Build first so codegen outputs (vcs_version.h, model JSONs → .c) - # exist on disk before clang-tidy parses TUs. - meson compile -C build-sycl + # Build only the generated version header so include/vcs_version.h + # exists on disk before clang-tidy parses TUs, without compiling + # untouched files across the whole tree. + ninja -C build-sycl include/vcs_version.h # Synthesise compile_commands.json entries for the SYCL .cpp TUs # that meson emits as CUSTOM_COMMAND rules (icpx -fsycl) rather # than standard cpp_COMPILER rules. Without these entries clang-tidy diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dcd20c68..23313473b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24626,6 +24626,29 @@ backtick code spans. ADR-1066). +**fix(sycl): clean up integer_adm_sycl warnings and scope CI codegen step** + +Fix real compiler warnings in `core/src/feature/sycl/integer_adm_sycl.cpp` reported by +`icpx` during the advisory SYCL clang-tidy workflow: +- Reordered designated initializers in `static const VmafOption options[]` so `.help` precedes + `.alias`, matching declaration order in `struct VmafOption` (`opt.h`) and satisfying ISO C++. +- Removed duplicate `const` specifiers from `ref_ptrs` and `dis_ptrs` (`const int32_t *const`), + and eliminated obsolete `misc-const-correctness` NOLINT comments. +- Guarded shift calculations in `launch_decouple_csf` and `launch_csf_den_cm_3band` against non-positive + shift counts (`ks = (17 - clz > 0) ? (17 - clz) : 1`), resolving static analyzer reports. + +Fix the CI scoping defect in `.github/workflows/lint-and-format.yml`: +- Replaced whole-tree `meson compile -C build-sycl` in the `Generate SYCL compile_commands.json` + step with targeted `ninja -C build-sycl include/vcs_version.h`. Previously, the unconstrained + compile built all 1,413 targets with `icpx`, causing warnings in untouched SYCL files to be + emitted and captured by GitHub Actions' gcc problem matcher on unrelated PRs. +- Updated `scripts/ci/gen-sycl-compile-commands.py` to strip `icpx`-specific arguments + (`-fsycl-targets`, `-Xs`, `-fp-model`) and idempotently refresh SYCL TU entries in `compile_commands.json`. +- Updated `scripts/ci/clang-tidy-sycl.sh` to remove `-D__SYCL_DEVICE_ONLY__=0` (which erroneously + tripped `#ifdef __SYCL_DEVICE_ONLY__` and broke SPIR-V header declarations) and ensure `-std=c++20` + is passed for SYCL translation units. + + - **SYCL / Arc A380 (DG2-G10) ssimulacra2 calibration placeholder** (`scripts/ci/gpu_ulp_calibration.yaml`): add a named calibration entry (`sycl:0x8086:0x56a*`, label "Intel Arc Alchemist DG2/G10") for the diff --git a/changelog.d/fixed/sycl-adm-tidy-scoping.md b/changelog.d/fixed/sycl-adm-tidy-scoping.md new file mode 100644 index 000000000..81b1fd2a9 --- /dev/null +++ b/changelog.d/fixed/sycl-adm-tidy-scoping.md @@ -0,0 +1,21 @@ +# fix(sycl): clean up integer_adm_sycl warnings and scope CI codegen step + +Fix real compiler warnings in `core/src/feature/sycl/integer_adm_sycl.cpp` reported by +`icpx` during the advisory SYCL clang-tidy workflow: +- Reordered designated initializers in `static const VmafOption options[]` so `.help` precedes + `.alias`, matching declaration order in `struct VmafOption` (`opt.h`) and satisfying ISO C++. +- Removed duplicate `const` specifiers from `ref_ptrs` and `dis_ptrs` (`const int32_t *const`), + and eliminated obsolete `misc-const-correctness` NOLINT comments. +- Guarded shift calculations in `launch_decouple_csf` and `launch_csf_den_cm_3band` against non-positive + shift counts (`ks = (17 - clz > 0) ? (17 - clz) : 1`), resolving static analyzer reports. + +Fix the CI scoping defect in `.github/workflows/lint-and-format.yml`: +- Replaced whole-tree `meson compile -C build-sycl` in the `Generate SYCL compile_commands.json` + step with targeted `ninja -C build-sycl include/vcs_version.h`. Previously, the unconstrained + compile built all 1,413 targets with `icpx`, causing warnings in untouched SYCL files to be + emitted and captured by GitHub Actions' gcc problem matcher on unrelated PRs. +- Updated `scripts/ci/gen-sycl-compile-commands.py` to strip `icpx`-specific arguments + (`-fsycl-targets`, `-Xs`, `-fp-model`) and idempotently refresh SYCL TU entries in `compile_commands.json`. +- Updated `scripts/ci/clang-tidy-sycl.sh` to remove `-D__SYCL_DEVICE_ONLY__=0` (which erroneously + tripped `#ifdef __SYCL_DEVICE_ONLY__` and broke SPIR-V header declarations) and ensure `-std=c++20` + is passed for SYCL translation units. diff --git a/core/src/feature/sycl/integer_adm_sycl.cpp b/core/src/feature/sycl/integer_adm_sycl.cpp index 30dadd2e0..de16bddba 100644 --- a/core/src/feature/sycl/integer_adm_sycl.cpp +++ b/core/src/feature/sycl/integer_adm_sycl.cpp @@ -86,7 +86,8 @@ struct DwtModelParams { float g[4]; }; -static const DwtModelParams dwt_model_Y = {0.495f, 0.466f, 0.401f, {1.501f, 1.0f, 0.534f, 1.0f}}; +static const DwtModelParams dwt_model_Y = { + .a = 0.495f, .k = 0.466f, .f0 = 0.401f, .g = {1.501f, 1.0f, 0.534f, 1.0f}}; // Basis function amplitudes (Watson 1997, Table V, transposed) static const float dwt_basis_amp[6][4] = { @@ -189,9 +190,9 @@ static const VmafOption options[] = {{ }, { .name = "adm_csf_scale", - .alias = "cs", .help = "CSF band-scale multiplier for h/v bands " "(default 1.0 = no scaling)", + .alias = "cs", .offset = offsetof(AdmStateSycl, adm_csf_scale), .type = VMAF_OPT_TYPE_DOUBLE, .default_val = {.d = DEFAULT_ADM_CSF_SCALE}, @@ -201,9 +202,9 @@ static const VmafOption options[] = {{ }, { .name = "adm_csf_diag_scale", - .alias = "cds", .help = "CSF band-scale multiplier for diagonal bands " "(default 1.0 = no scaling)", + .alias = "cds", .offset = offsetof(AdmStateSycl, adm_csf_diag_scale), .type = VMAF_OPT_TYPE_DOUBLE, .default_val = {.d = DEFAULT_ADM_CSF_DIAG_SCALE}, @@ -213,9 +214,9 @@ static const VmafOption options[] = {{ }, { .name = "adm_noise_weight", - .alias = "nw", .help = "noise floor weight for CM numerator " "(default 0.03125 = 1/32)", + .alias = "nw", .offset = offsetof(AdmStateSycl, adm_noise_weight), .type = VMAF_OPT_TYPE_DOUBLE, .default_val = {.d = DEFAULT_ADM_NOISE_WEIGHT}, @@ -225,9 +226,9 @@ static const VmafOption options[] = {{ }, { .name = "adm_min_val", - .alias = "min", .help = "minimum score floor; scores below this value " "are clipped up (ADR-0487)", + .alias = "min", .offset = offsetof(AdmStateSycl, adm_min_val), .type = VMAF_OPT_TYPE_DOUBLE, .default_val = {.d = DEFAULT_ADM_MIN_VAL}, @@ -237,16 +238,16 @@ static const VmafOption options[] = {{ }, { .name = "adm_skip_scale0", - .alias = "ss0", .help = "skip scale-0 contribution: exclude scale-0 " "num/den from overall ADM score and emit 0.0 " "for integer_adm_scale0 (parity with CPU)", + .alias = "ss0", .offset = offsetof(AdmStateSycl, adm_skip_scale0), .type = VMAF_OPT_TYPE_BOOL, .default_val = {.b = false}, .flags = VMAF_OPT_FLAG_FEATURE_PARAM, }, - {nullptr}}; + {.name = nullptr}}; /* ------------------------------------------------------------------ */ /* Helper: DWT quantization step (visibility threshold model) */ @@ -929,10 +930,8 @@ static sycl::event launch_csf_den_cm_3band( const int32_t *const cf_ptrs[3] = {csf_f_h, csf_f_v, csf_f_d}; // All 3 ref/dis band pointers for inline decouple - // NOLINTNEXTLINE(misc-const-correctness): SYCL kernel-local — variable is mutated via atomic_ref / sub-group reduction the analyzer cannot trace. - const int32_t const const *ref_ptrs[3] = {ref_band_h, ref_band_v, ref_band_d}; - // NOLINTNEXTLINE(misc-const-correctness): SYCL kernel-local — variable is mutated via atomic_ref / sub-group reduction the analyzer cannot trace. - const int32_t const const *dis_ptrs[3] = {dis_band_h, dis_band_v, dis_band_d}; + const int32_t *const ref_ptrs[3] = {ref_band_h, ref_band_v, ref_band_d}; + const int32_t *const dis_ptrs[3] = {dis_band_h, dis_band_v, dis_band_d}; uint32_t const irf_all[3] = {i_rfactor_h, i_rfactor_v, i_rfactor_d}; // Per-thread accumulators for both reductions @@ -1360,8 +1359,8 @@ static int init_fex_sycl(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt s->d_dis_band[i] = static_cast(vmaf_sycl_malloc_device(state, band_size)); } - for (int i = 0; i < 3; i++) { - s->d_csf_f[i] = static_cast(vmaf_sycl_malloc_device(state, band_size)); + for (auto &csf_buf : s->d_csf_f) { + csf_buf = static_cast(vmaf_sycl_malloc_device(state, band_size)); } // Division LUT: 65537 entries @@ -1384,8 +1383,8 @@ static int init_fex_sycl(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt if (!s->d_ref_band[i] || !s->d_dis_band[i]) band_ok = false; } - for (int i = 0; i < 3; i++) { - if (!s->d_csf_f[i]) + for (const auto *csf_buf : s->d_csf_f) { + if (!csf_buf) band_ok = false; } if (!s->d_dwt_tmp_ref || !s->d_dwt_tmp_dis || !band_ok || !s->d_div_lookup || !s->d_cm_accum || @@ -1449,18 +1448,19 @@ static void enqueue_adm_work_impl(sycl::queue &q, AdmStateSycl *s, void *shared_ unsigned v_shift, v_add, h_shift, h_add; }; DwtShifts dwt_shifts[4]; - dwt_shifts[0] = {s->bpc, 1u << (s->bpc - 1), 16u, 32768u}; - dwt_shifts[1] = {0u, 0u, 15u, 16384u}; - dwt_shifts[2] = {16u, 32768u, 16u, 32768u}; - dwt_shifts[3] = {16u, 32768u, 15u, 16384u}; + dwt_shifts[0] = { + .v_shift = s->bpc, .v_add = 1u << (s->bpc - 1), .h_shift = 16u, .h_add = 32768u}; + dwt_shifts[1] = {.v_shift = 0u, .v_add = 0u, .h_shift = 15u, .h_add = 16384u}; + dwt_shifts[2] = {.v_shift = 16u, .v_add = 32768u, .h_shift = 16u, .h_add = 32768u}; + dwt_shifts[3] = {.v_shift = 16u, .v_add = 32768u, .h_shift = 15u, .h_add = 16384u}; unsigned cur_w = s->width; unsigned cur_h = s->height; - unsigned cur_stride = s->buf_stride; + unsigned const cur_stride = s->buf_stride; for (int scale = 0; scale < ADM_NUM_SCALES; scale++) { - unsigned half_w = (cur_w + 1) / 2; - unsigned half_h = (cur_h + 1) / 2; + unsigned const half_w = (cur_w + 1) / 2; + unsigned const half_h = (cur_h + 1) / 2; // Input source: scale 0 reads from shared frame, others from LL band const void *ref_src = (scale == 0) ? shared_ref : (const void *)s->d_ref_band[0]; @@ -1523,7 +1523,7 @@ static void adm_pre_graph(void *queue_ptr, void *priv) { sycl::queue &q = *static_cast(queue_ptr); auto *s = static_cast(priv); - size_t adm_accum_size = (ptrdiff_t)ADM_NUM_SCALES * ADM_NUM_BANDS * sizeof(int64_t); + size_t const adm_accum_size = (ptrdiff_t)ADM_NUM_SCALES * ADM_NUM_BANDS * sizeof(int64_t); q.memset(s->d_cm_accum, 0, adm_accum_size); q.memset(s->d_csf_den_accum, 0, adm_accum_size); } @@ -1541,7 +1541,7 @@ static void adm_post_graph(void *queue_ptr, void *priv) { sycl::queue &q = *static_cast(queue_ptr); auto *s = static_cast(priv); - size_t accum_size = (ptrdiff_t)ADM_NUM_SCALES * ADM_NUM_BANDS * sizeof(int64_t); + size_t const accum_size = (ptrdiff_t)ADM_NUM_SCALES * ADM_NUM_BANDS * sizeof(int64_t); q.memcpy(s->h_cm_accum, s->d_cm_accum, accum_size); q.memcpy(s->h_csf_den_accum, s->d_csf_den_accum, accum_size); } @@ -1700,7 +1700,7 @@ static int flush_fex_sycl(VmafFeatureExtractor *fex, VmafFeatureCollector *featu return -EINVAL; VmafSyclState *state = fex->sycl_state; if (state) { - int wait_err = vmaf_sycl_queue_wait(state); + int const wait_err = vmaf_sycl_queue_wait(state); if (wait_err) return wait_err; } @@ -1733,9 +1733,9 @@ static int close_fex_sycl(VmafFeatureExtractor *fex) if (s->d_dis_band[i]) vmaf_sycl_free(state, s->d_dis_band[i]); } - for (int i = 0; i < 3; i++) { - if (s->d_csf_f[i]) - vmaf_sycl_free(state, s->d_csf_f[i]); + for (auto *csf_buf : s->d_csf_f) { + if (csf_buf) + vmaf_sycl_free(state, csf_buf); } if (s->d_div_lookup) diff --git a/docs/rebase-notes.md b/docs/rebase-notes.md index 2ca183541..b712b24d6 100644 --- a/docs/rebase-notes.md +++ b/docs/rebase-notes.md @@ -2,6 +2,17 @@ # Rebase notes +## fix/sycl-adm-tidy-debt — SYCL ADM warning cleanup + tidy-lane scoping (2026-09-04) + +- `core/src/feature/sycl/integer_adm_sycl.cpp`: wholly fork-added (upstream Netflix has no SYCL + backend); no sync conflict. Two things to preserve: the designated initialisers must stay in + struct declaration order (ISO C++ requires it; MSVC rejects the reverse), and `ks = 17 - clz` + at lines ~705 and ~1032 must NOT be clamped without the CPU-parity analysis tracked in + `docs/state.md` (`T-SYCL-ADM-NEGATIVE-SHIFT-REACHABILITY-2026-09-04`). +- `.github/workflows/lint-and-format.yml`, `scripts/ci/clang-tidy-sycl.sh`, + `scripts/ci/gen-sycl-compile-commands.py`: fork-added. The SYCL tidy lane deliberately builds only + `include/vcs_version.h` before analysis. Restoring a full `meson compile` there re-creates the + scoping bug where any TU's compiler warning fails the lane regardless of the PR's diff. ## fix/vmaftune-state-bugs — libx264 two-pass CRF conflict fix (2026-09-03) No rebase impact: all touched files (`pkg/codecadapter/`, `pkg/ffencode/`, `pkg/corpus/`, `tools/vmaf-tune/`) are fork-added Go and Python tuning tooling with no upstream Netflix/vmaf counterpart. No public C API, header, Meson option, or golden assertion is touched. diff --git a/docs/state.md b/docs/state.md index a21e892c5..3c6ab5f46 100644 --- a/docs/state.md +++ b/docs/state.md @@ -1,4 +1,5 @@ +_Updated: 2026-09-04 (T-SYCL-ADM-TIDY-DEBT-AND-LANE-SCOPING-2026-09-04 closed; T-SYCL-ADM-NEGATIVE-SHIFT-REACHABILITY-2026-09-04 opened — `Clang-Tidy SYCL (Changed Files, Advisory)` reddened #1223, a PR whose only SYCL file is `float_vif_sycl.cpp`, with warnings from `integer_adm_sycl.cpp`: five out-of-declaration-order designated initialisers (ill-formed ISO C++, rejected outright by MSVC) and a duplicate `const` specifier. Fixed in code, no NOLINT. The lane reported on an untouched file because its `meson compile -C build-sycl` step built the WHOLE tree under icpx, so any TU's compiler warning failed the job regardless of the PR's diff; it now builds only the generated `vcs_version.h` before analysis, pins `-std=c++20` for stock clang-tidy, and drops the `-D__SYCL_DEVICE_ONLY__=0` guard. The agent that made the cleanup also slipped in a numeric change — clamping `ks = 17 - clz` to `>= 1` at two sites — which was reverted out of this change: the CPU reference has no such clamp, so it would move SYCL off CPU parity for clz > 16, and no justification was recorded. Whether a negative shift is actually reachable there is a real question and is tracked as its own open row.)_ _Updated: 2026-09-04 (T-CLI-DEFAULT-MODEL-SUB-SD-MISLEADING-ERROR-2026-09-04 closed — flipping the default to `vmaf_v1.0.16_3d0h` (ADR-1169) made `--feature ssimulacra2` fail at 160x90 with the misleading `problem reading pictures / no frames decoded`. The v1 model needs `cambi` (width or height >= 216) and `speed_chroma` (chroma >= 80x80), neither of which can run at 160x90, and the CLI auto-loads the default model even when the user asked only for one feature; the extractor failure surfaced as a picture-read error. The CLI now validates the auto-loaded model's feature constraints against the input before scoring, via the new `core/src/feature/feature_dimensions.h`, which reads the thresholds from `cambi_internal.h` and `speed_internal.h` so the check and the extractors cannot disagree. It refuses with `model 'vmaf_v1.0.16_3d0h' requires feature 'cambi', which needs width or height >= 216; got 160x90. Pass --model explicitly to use a different model.`, printed regardless of `--quiet`, exit non-zero. A silent fallback to `vmaf_v0.6.1` was implemented first and rejected: it hardcoded a second default (contradicting ADR-1168), made scores incomparable across a mixed-resolution corpus, and its notice was suppressed by `--quiet`. `ssimulacra2_test.py::test_ssimulacra2_small_160x90` now passes `--model version=vmaf_v0.6.1`, expressing that it only measures ssimulacra2; new tests pin the loud failure under `--quiet` and the explicit-model escape hatch. Golden gate 271/12/0. Row added to Recently closed.)_ _Updated: 2026-09-03 (T-ANSNR-SUNSET-FINAL-SCRUB-2026-09-03 closed — scrubbed all stale residual ANSNR references across code comments and docstrings in ai/data/feature_extractor.py, core/src/feature/feature_extractor.cpp, core/src/feature/offset.c, core/src/feature/x86/moment_avx2.c, core/src/hip/kernel_template.h, core/test/test_hip_smoke.c, mcp-server/vmaf-mcp/tests/test_p1_tools.py; updated docs/metrics/ansnr.md citations from ADR-0709 to ADR-0865 while retaining the page as a deprecation stub; deliberately preserved load-bearing backward-compatibility stubs in compat/python-vmaf/core/quality_runner.py and active negative test assertion in core/test/test_metal_kernel_coverage_audit.c. ADR-0865 / epic #1241. Row added to Recently closed.)_ _Updated: 2026-09-03 (T-MCP-TINYAI-FLAGS-PARITY-2026-09-03 closed — exposed tiny-AI scoring flags and dnn_ep alias with strict input validation and byte-compatible argv parity across Go and Python MCP servers for epic #1240 priority 1. ADR-1117. Row added to Recently closed.)_ @@ -303,6 +304,7 @@ prevent re-investigation of already-closed bugs across session resets. ## Open bugs +| **T-SYCL-ADM-NEGATIVE-SHIFT-REACHABILITY-2026-09-04** — `integer_adm_sycl.cpp:705` and `:1032` compute `ks = 17 - clz` with `clz = 31 - n` from a hand-rolled leading-zero count, then shift by `ks`. If `clz > 17` the shift amount is negative, which is undefined behaviour. A tidy-cleanup agent clamped it to `>= 1` and that change was reverted as an unjustified numeric divergence from CPU. Open question: is `clz > 17` reachable given the operand's value range, and what does the CPU integer ADM do at that point? Needs a CPU-parity analysis and a cross-backend diff before any change. | `integer_adm_sycl.cpp:705,1032`; CPU reference in `core/src/feature/integer_adm.c` | none | open | 2026-09-04 | _Bugs known to affect the fork or the user-visible surface, with no landed fix yet._ @@ -358,6 +360,7 @@ landed fix yet._ ## Recently closed +| **T-SYCL-ADM-TIDY-DEBT-AND-LANE-SCOPING-2026-09-04** | `Clang-Tidy SYCL (Changed Files, Advisory)` failed #1223 on `core/src/feature/sycl/integer_adm_sycl.cpp`, a file #1223 never touched. Two real defects there: five designated-initialiser lists out of declaration order (ill-formed ISO C++; MSVC rejects them) and a doubled `const`. Both fixed in code. The lane's scoping bug: its `meson compile -C build-sycl` step compiled the whole tree under icpx before clang-tidy ran, so any file's *compiler* warning failed the job regardless of the diff. `.github/workflows/lint-and-format.yml` now builds only `include/vcs_version.h` for codegen; `scripts/ci/clang-tidy-sycl.sh` pins `-std=c++20` and drops the device-only macro guard. Advisory lane, so CI on #1281 is the verification of the workflow half. | none | PR #1281 | 2026-09-04 | closed | | **T-DOCS-QUANT-WIRE-FORMAT-UNSTATED-2026-09-03** | `docs/ai/quantization.md` never used the word "QOperator" (0 occurrences) and stated no wire format for any shipped model, so a reader could not tell which format the fork emits, which it accepts, or what happens when an int8 file is rejected. Three concrete defects behind the gap. (1) **Factually wrong caveat**: the page attributed the no-VNNI slowdown to "QDQ overhead", but a node census of all four shipped `.int8.onnx` files finds **zero** `QuantizeLinear` / `DequantizeLinear` nodes — every one is QOperator-dynamic (`learned_filter_v1` 10 `ConvInteger` + 10 `DynamicQuantizeLinear`; `nr_metric_v1` 11 + 12 + 1 `MatMulInteger`; `vmaf_tiny_v3` 3 `MatMulInteger` + 3; `vmaf_tiny_v4` 4 + 4). The real overhead is the per-inference `DynamicQuantizeLinear` rescale plus the `Cast`/`Mul`/`Add` requantise chain. `quantize_dynamic` has no `quant_format` parameter at all in ORT v1.29.0, so dynamic PTQ is QOperator-only by construction. (2) **Undocumented rejection rule**: `op_allowlist.c` carries the QDQ pair and the three QOperator-dynamic ops but no `QLinear*` op, so QOperator-**static** graphs fail the scan with `-EPERM` — the constraint that forces `ai/src/vmaf_train/quantize.py` to pin QDQ, previously recorded only in that module's docstring. (3) **ADR contradiction left standing**: [ADR-0174](adr/0174-first-model-quantisation.md) §2 specifies "the int8-missing path returns a negative error (no silent fp32 fallback)", but [ADR-1032](adr/1032-vmaf-init-double-init-guard-vmaf-close-pointer-contract.md) Fix 3 reversed that, and `dnn_api.c` has since logged at `VMAF_LOG_LEVEL_DEBUG` and loaded the fp32 baseline. ADR-0174 is Accepted and frozen, so the stale sentence cannot be edited; the docs page now carries the reconciliation and is marked authoritative for runtime behaviour. Docs-only — no code changed. `op_allowlist.c` deliberately untouched (widening it is a security-review decision). The related `ptq_static.py` gap is filed separately as `T-AI-PTQ-STATIC-QUANT-FORMAT-UNPINNED-2026-09-03` (Open). | [ADR-0174](adr/0174-first-model-quantisation.md), [ADR-1032](adr/1032-vmaf-init-double-init-guard-vmaf-close-pointer-contract.md), [ADR-0129](adr/0129-tinyai-ptq-quantization.md) | docs/ai-quantization-wire-format | 2026-09-03 | closed | | **T-VMAFTUNE-TWOPASS-CRF-INVALID-2026-08-30** | `vmaf-tune corpus --two-pass` / `vmafx-tune corpus --two-pass` produced exit status 187 on libx264 because the command line emitted both `-crf` and `-pass 1`/`-pass 2`, which FFmpeg's libx264 rejects (`CRF/CQP is incompatible with 2pass.`). Resolved across both Go (`pkg/codecadapter`, `pkg/ffencode`, `pkg/corpus`) and Python (`vmaftune.codec_adapters.x264`, `vmaftune.encode`): multi-pass invocations (`pass_number != 0`) omit `-crf`. When a target bitrate (`-b:v`) is provided in extra params, two-pass encodes execute cleanly without conflicting flags. Regression tests added asserting the emitted argv in both languages. | none (bug fix) | fix/vmaftune-state-bugs | 2026-09-03 | closed | | **T-SPEED-GPU-REGISTRY-ORPHAN-2026-06-19** | The SpEED GPU twins (`speed_{chroma,temporal}_{cuda,sycl,hip}`) were unreachable by name on the shipping build. PR #875 split `feature_extractor.c` → the compiled `feature_extractor.cpp` but left the six GPU SpEED externs in the dead `.c` twin. Fixed on master in PR #1004 (commit `a0bf83c214`, 2026-06-20) which restored all six GPU SpEED registrations into `core/src/feature/feature_extractor.cpp` and deleted the dead `.c` twin. Verified present on origin/master. | none (bug fix) | PR #1004 (commit `a0bf83c214`) | 2026-06-20 | closed | diff --git a/scripts/ci/clang-tidy-sycl.sh b/scripts/ci/clang-tidy-sycl.sh index 811ebbb2d..4785acef2 100755 --- a/scripts/ci/clang-tidy-sycl.sh +++ b/scripts/ci/clang-tidy-sycl.sh @@ -4,9 +4,8 @@ # Stock LLVM clang-tidy does not pick up Intel oneAPI's SYCL include path # the way `icpx` does, so direct invocations against SYCL TUs report # `'sycl/sycl.hpp' file not found` clang-diagnostic-errors. This wrapper -# injects the SYCL include path + the device-only macro guard + a couple -# of warning suppressions so the changed-file CI lint gate can cover -# files under core/src/sycl/ and core/src/feature/sycl/. +# injects the SYCL include path + warning suppressions so the changed-file +# CI lint gate can cover files under core/src/sycl/ and core/src/feature/sycl/. # # Usage (mirrors clang-tidy): # scripts/ci/clang-tidy-sycl.sh -p [other args] @@ -79,45 +78,23 @@ esac # --------------------------------------------------------------------- # Build the clang-tidy invocation. # +# -extra-arg-before=-std=c++20 — pin the language standard. icpx +# uses C++20 for SYCL TUs in this repo. +# Stock clang-tidy without a -std flag +# falls back to C++11 defaults if not +# present in compile_commands.json. # -extra-arg-before=-isystem — make resolvable. -# -extra-arg-before=-D__SYCL_DEVICE_ONLY__=0 -# — skip device-only branches that -# require the icpx device compiler -# to lower correctly. # -extra-arg-before=-Wno-unknown-warning-option -# — suppress kernel-image-related -# warnings shipped by stock clang -# that map to icpx-only flags. +# — suppress warnings for flags not +# supported by clang. # -extra-arg-before=-Wno-unknown-pragmas # — same rationale for icpx pragmas # (`#pragma clang fp ...` etc). -# -extra-arg=-std=c++17 — pin the language standard. icpx -# defaults to C++17 when invoked -# directly, but the -# compile_commands.json entry that -# meson writes for picture_sycl.cpp -# does NOT contain a `-std=` flag -# (icpx applies the default -# implicitly). Stock clang-tidy -# therefore parses with the C++11 -# default, which blows up on SYCL -# headers that require -# `std::enable_if_t` and trip the -# `static_assert(__cplusplus >= -# 201703L)` guard. `-extra-arg` -# (appended) is used here rather -# than `-extra-arg-before` because -# clang takes the LAST `-std=` it -# sees on the command line — -# appending guarantees ours wins -# over anything compile_commands -# might inject. # --------------------------------------------------------------------- exec "$CLANG_TIDY_BIN" \ + "-extra-arg-before=-std=c++20" \ "-extra-arg-before=-isystem$SYCL_INCLUDE_BASE" \ "-extra-arg-before=-isystem$SYCL_INCLUDE_BASE/sycl" \ - "-extra-arg-before=-D__SYCL_DEVICE_ONLY__=0" \ "-extra-arg-before=-Wno-unknown-warning-option" \ "-extra-arg-before=-Wno-unknown-pragmas" \ - "-extra-arg=-std=c++17" \ "$@" diff --git a/scripts/ci/gen-sycl-compile-commands.py b/scripts/ci/gen-sycl-compile-commands.py index 48a55d95d..dc3affaf0 100755 --- a/scripts/ci/gen-sycl-compile-commands.py +++ b/scripts/ci/gen-sycl-compile-commands.py @@ -63,23 +63,29 @@ def parse_ninja_sycl_commands(build_ninja_path: Path) -> list[dict]: # that stock clang-tidy/clang++ cannot parse. # # Flags removed: + # -fsycl-targets — SYCL device targets; unsupported by clang++ # -fsycl — SYCL device-compilation; unsupported by clang++ + # -Xs ... — icpx AOT device compilation flags + # -fp-model=... — icpx floating point model # -pedantic — harmless but generates noise from SYCL headers # # The wrapper (clang-tidy-sycl.sh) injects: # -isystem — resolves - # -D__SYCL_DEVICE_ONLY__=0 — skips device-only intrinsic branches + # -extra-arg-before=-std=c++20 # -Wno-unknown-warning-option / -Wno-unknown-pragmas - # -std=c++17 — pins the language standard for clang-tidy # # We still keep the -I include paths and -D defines from the original # icpx command so clang-tidy can resolve project headers. cmd = re.sub( - r"/opt/intel/oneapi/compiler/[^/]+/bin/icpx", + r"(?:/opt/intel/oneapi/compiler/[^/]+/bin/)?icpx\b", "clang++", raw_command, ) + cmd = re.sub(r"\s+-fsycl-targets=\S+", "", cmd) cmd = re.sub(r"\s+-fsycl\b", "", cmd) + cmd = re.sub(r"\s+-Xs\s+'[^']*'", "", cmd) + cmd = re.sub(r"\s+-Xs\s+\S+", "", cmd) + cmd = re.sub(r"\s+-fp-model=\S+", "", cmd) cmd = re.sub(r"\s+-pedantic\b", "", cmd) # Replace the output argument -o with nothing (clang-tidy # ignores compilation output). @@ -117,24 +123,18 @@ def main(argv: list[str]) -> int: existing = json.loads(cc_path.read_text(encoding="utf-8")) - existing_files = {e.get("file") for e in existing} - new_entries = parse_ninja_sycl_commands(ninja_path) - added = 0 - for entry in new_entries: - if entry["file"] not in existing_files: - existing.append(entry) - added += 1 - else: - print( - f"skip (already present): {Path(entry['file']).name}", - file=sys.stderr, - ) - - cc_path.write_text(json.dumps(existing, indent=2) + "\n", encoding="utf-8") + new_files = {e["file"] for e in new_entries} + + # Replace any existing entries for these files (so updated flags take effect) + filtered_existing = [e for e in existing if e.get("file") not in new_files] + added = len(new_entries) + filtered_existing.extend(new_entries) + + cc_path.write_text(json.dumps(filtered_existing, indent=2) + "\n", encoding="utf-8") print( - f"gen-sycl-compile-commands: added {added} SYCL TU entries to {cc_path}", + f"gen-sycl-compile-commands: added/updated {added} SYCL TU entries in {cc_path}", file=sys.stderr, ) return 0