Repository navigation
fix(sycl): give every SYCL feature kernel the CPU's fp32 arithmetic - #1630
Merged
Merged
Conversation
lusoris
changed the base branch from
master
to
perf/sycl-ssimulacra2-msssim-device-resident
September 29, 2026 16:04
lusoris
force-pushed
the
fix/sycl-fp-contract-all-tus
branch
from
September 29, 2026 16:06
8031efc to
ba1b66e
Compare
lusoris
force-pushed
the
perf/sycl-ssimulacra2-msssim-device-resident
branch
from
September 29, 2026 16:50
229f072 to
0f5c8ed
Compare
lusoris
force-pushed
the
fix/sycl-fp-contract-all-tus
branch
2 times, most recently
from
September 30, 2026 07:37
fe2f653 to
a73f3d0
Compare
lusoris
force-pushed
the
perf/sycl-ssimulacra2-msssim-device-resident
branch
from
September 30, 2026 12:15
31f8535 to
416e73c
Compare
Base automatically changed from
perf/sycl-ssimulacra2-msssim-device-resident
to
master
September 30, 2026 14:05
This was referenced Sep 30, 2026
lusoris
force-pushed
the
fix/sycl-fp-contract-all-tus
branch
from
September 30, 2026 23:06
a73f3d0 to
b03fcbd
Compare
lusoris
added a commit
that referenced
this pull request
Sep 30, 2026
`core/test/sycl_fp_arith_probe.cpp` fell between the two clang-tidy jobs. Tidy Changed excludes SYCL test sources by the `core/test/test_sycl` prefix and parsed this one with the CPU build's flags, where `<sycl/sycl.hpp>` does not exist, so it failed on #1630; Tidy SYCL selects `core/test/test_sycl*` and never measured it. As `test_sycl_fp_arith_probe.cpp` it is excluded from the first and linted by the second, where `scripts/ci/clang-tidy-sycl.sh` reports nothing for it.
Every SYCL feature translation unit now compiles with `-fp-model=precise -ffp-contract=off -foffload-fp32-prec-div -foffload-fp32-prec-sqrt` (ADR-1367). `-fp-model=precise` alone still fused `a * b + c` into one FMA inside kernel lambdas and left fp32 `/` and `sqrt` approximate, while the SYCL guide described the kernels as IEEE-754 strict. Only the SpEED and ssimulacra2 TUs had contraction off. - `sycl_strict_fp_args` is defined once, between new policy markers, and goes to every feature TU; ADR-1363's `sycl_exact_fp_args` and `sycl_exact_fp_sources` fold into it. `sycl_dependency` carries the precision pair to non-Windows links, because the SPIR-V JIT image is still device-linked there under `-fno-sycl-rdc`. - `integer_vif_sycl` wrote its one fp64-approximating multiply-subtract as an implicit contraction; it is now an explicit `sycl::fma`. - New `test_sycl_fp_arith_contract` checks contraction and rounding on the device for both image paths; `test_strict_fp_compiler_args.py` executes the policy for icpx and AdaptiveCpp. - `gen-sycl-compile-commands.py` drops the icpx-only precision flags so stock clang-tidy can still parse SYCL TUs. Nine twins' outputs move, each inside its ADR-0214 tolerance (float_adm 2.5e-5 -> 2.5e-6 on the Netflix pair); bit-identical twins stay bit identical. No twin's 4K cost on the Arc B580 moved beyond run-to-run spread. docs/state.md closes T-SYCL-FP-MODEL-PRECISE-CONTRACTS-2026-09-29 and opens CUDA, HIP, float_moment-routing and parity-gate rows.
`core/test/sycl_fp_arith_probe.cpp` fell between the two clang-tidy jobs. Tidy Changed excludes SYCL test sources by the `core/test/test_sycl` prefix and parsed this one with the CPU build's flags, where `<sycl/sycl.hpp>` does not exist, so it failed on #1630; Tidy SYCL selects `core/test/test_sycl*` and never measured it. As `test_sycl_fp_arith_probe.cpp` it is excluded from the first and linted by the second, where `scripts/ci/clang-tidy-sycl.sh` reports nothing for it.
lusoris
force-pushed
the
fix/sycl-fp-contract-all-tus
branch
from
October 1, 2026 06:48
3811833 to
98dedac
Compare
This was referenced Oct 1, 2026
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
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
Every SYCL feature kernel now does fp32 arithmetic the way the CPU reference does. Each SYCL feature TU compiles with
-fp-model=precise -ffp-contract=off -foffload-fp32-prec-div -foffload-fp32-prec-sqrt(ADR-1367). Soa * b + cis never fused into an FMA, and/andsqrtare correctly rounded, on the AOT images and on the SPIR-V image other devices compile at first launch.Before this,
-fp-model=precisealone left 12%, 29% and 8% of random multiply-adds, divisions and square roots different from the host, while the SYCL guide called the kernels IEEE-754 strict. This closesT-SYCL-FP-MODEL-PRECISE-CONTRACTS-2026-09-29.The branch is one commit on master
10f27efe2; #1627, which it was stacked on, is merged. On the Arc A380 this change fails the same tests as master, and its own contract test passes on the JIT image and on adg2-g11AOT image (A380 section below).What changes:
sycl_strict_fp_argsis defined once incore/src/meson.build, between new policy markers, and reaches every feature TU. perf(sycl): run ssimulacra2 on the device and wait once per frame in ms_ssim #1627'ssycl_exact_fp_args/sycl_exact_fp_sourcesfold into it;sycl_exact_fp.his unchanged apart from its comments.-fno-sycl-rdc. The AOT images are now finished per TU at compile time and need the pair there, while the SPIR-V JIT image is still device-linked at the final icpx link and needs it there.sycl_strict_fp_argswhole.link.exenever wraps device code.integer_vif_syclwrites its fused operation explicitly. It approximated the CPU's fp64sigma2_sq - g * sigma12through an implicit contraction; with contraction off it drifted 6x further from the CPU. It is nowsycl::fma(-g, sigma12, sigma2_sq)and ends up closer than before.test_sycl_fp_arith_contract(new) compiles a probe kernel with the extractors' own arguments and checks 1,048,576 random and 2,197 boundary operands on the device.test_strict_fp_compiler_args.pyexecutes the policy for icpx and AdaptiveCpp.test_sycl_kernel_source_contract.pynow checks that no TU gets a private FP list.gen-sycl-compile-commands.pydrops the icpx-only precision flags, which stock clang rejects as unknown arguments. Without this, Tidy SYCL would fail on every SYCL TU.float_ssim's formula. These are corrected to match:docs/development/sycl-toolchains.md,docs/metrics/features.mdand the AGENTS files;core/src/feature/cuda/AGENTS.md, which overstated which CUDA kernels use--fmad=false.Type
fix— bug fixsycl/cuda/simd— backend-specificChecklist
make format && make lintis green locally. Partial, run on the rebased head on Linux:pre-commit run --from-ref origin/master --to-ref HEAD: all 39 applicable hooks pass.make lint-py lint-md docs-fragments-check lint-reuse python-locks-checkpasses.make lint-shpasses every step exceptscripts/githooks/tests/test_install.py. That test fails only while a.venvwithoutpre_commitis first on PATH, and it fails the same way on master. It is a hook-installer bug outside this change; the test passes on this branch without that.venvon PATH.make verify-allpasses with the enginestandards-gate.ymlpins (f41e74d8).speed_sycl_pipeline.cpp(11) andssimulacra2_sycl.cpp(13), allmisc-static-assert. Master measures the same counts in the same configuration, which is the driftT-TIDY-RATCHET-GPU-LANES-UNREPRODUCIBLE-2026-09-22tracks.make lint-cwas not run.--suite sycl52/52 on the Arc B580 and 52/52 on the UHD 770 (office box, before the rebase).test_strict_fp_compiler_args,test_sycl_kernel_source_contractandtest_gen_sycl_compile_commandspass.test_sycl_fp_arith_contractpasses on both image paths. The A380's 16 failing tests fail identically on master; see below./cross-backend-diffand the worst ULP is ≤ 2. I ran the per-twin CPU comparison and the ADR-0214 gate instead (tables below). Several twins were never within 2 ULP; none moved outside its tolerance..c/.cpp/.cu/.h/.hpp, it has the appropriate license header (seeCONTRIBUTING.md).docs/adr/_index_fragments/<NNNN-slug>.mdand the slug is appended todocs/adr/_index_fragments/_order.txt— do not editdocs/adr/README.mddirectly.Bug-status hygiene (ADR-0165)
docs/state.mdupdated:T-SYCL-FP-MODEL-PRECISE-CONTRACTS-2026-09-29.ryzen-4090-arc:T-CUDA-FP-CONTRACT-DEFAULT-2026-09-29andT-HIP-FP-CONTRACT-DEFAULT-2026-09-29.T-CLI-FLOAT-MOMENT-NO-TWIN-2026-09-29.T-CI-PARITY-GATE-STALE-METRIC-KEYS-2026-09-29.Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests.Cross-backend numerical results
Arc B580 and UHD 770 (office box, measured before the rebase)
Max abs diff against
--backend cpu --precision max, before (#1627 head) -> after. The Arc B580 and the UHD 770 give the same result unless noted. 576x324 is the Netflix pair (48 frames); 4K is BBB 3840x2160 (22 frames).scripts/ci/cross_backend_parity_gate.py --backends cpu syclon the Netflix pair, one feature per run: the 15 runnable cells pass on both GPUs before and after. Thecambiandmotioncells abort on stale metric names in the gate itself; that isT-CI-PARITY-GATE-STALE-METRIC-KEYS-2026-09-29, and both twins are covered by the table above. Where the maximum grew, the residual comes from operations the flags do not control, such assycl::log2, the summation order, or fp64 terms on the CPU; see Research-1367 §6.Arc A380 (
dg2-g11, zeus, 2026-09-30)Setup. Kernel 7.2.8 with the A380 on the xe driver (
xe.force_probe=56a5), NEO 26.35.39758.10, IGC 2.41.5, icpx 2026.0. Builds were--buildtype=release -Db_lto=false, run withONEAPI_DEVICE_SELECTOR=level_zero:0.meson test --suite sycl:The 16 failures are the same tests in all three builds. They cover:
_parity,_parity_large,_tiny_frames)_parity,_parity_large)_parity,_parity_large)_parity,_parity_large,_parity_simd32)_parity,_parity_large)speed_singular_parity,speed_temporal_parity_large,shared_planesandmotion_tiny_framesThis change does not cause them. The xe driver returns wrong values from GPU kernels that use scratch memory:
Two standalone SYCL kernels with no vmafx code fail on this device:
They fail as JIT and as AOT, on Level Zero and on OpenCL, and both are correct on the OpenCL CPU device.
IGC's zeinfo shows that each of the 16 failing tests runs a kernel with a non-zero
private_sizeorspill_size.On 2026-09-25, under i915 with the same NEO and IGC, 13 of these 16 tests passed on this box. The other three (
psnr_hvs_parity_simd32,shared_planes,motion_tiny_frames) did not exist yet.Removing the scratch use from the SYCL kernels is separate work, branched from master.
On the scratch-free twins, per-frame CPU parity via
scripts/dev/speed_gpu_parity.py --backend sycllooks like the B580's (max abs diff, master -> this branch):AOT image check. A
dg2-g11-only AOT build stops atsycl_aot_image_check, on master as on this branch. With a single target, ocloc stores each TU's image as a plain zebin ELF (31/31 images here), andcheck_aot_image.pyonly acceptsarfat binaries. The AOT row above was built withninja -k 0. A fix is in progress on a separate branch from master,fix/sycl-aot-check-single-target.Performance (if
perforfeat)Arc B580, 3840x2160, ms per frame, before -> after. Frames were held in memory (8 frames loaded once, 50 fed through
vmaf_read_pictures()), median of 5 interleaved runs, or 7 for the last three:Every twin is within -8% to +3.1%, inside the spread of kernels whose code did not change, so no twin crossed the 10% bar. The prescribed CLI method,
(t(22) - t(2)) / 20with a median of 3, was run too. It is in the research digest, but on the shared office host it swung ±50% for unchanged twins, so it cannot resolve 10%. The A380 was not timed: under xe, half its twins compute wrong values.Deep-dive deliverables (ADR-0108)
docs/research/1367-sycl-strict-fp-every-feature-tu.md## Alternatives consideredAGENTS.mdinvariant note —core/src/sycl/AGENTS.md,core/src/feature/sycl/AGENTS.md,docs/development/rebase-sensitive-invariants.mdchangelog.d/fixed/sycl-strict-fp-every-feature-tu.mddocs/rebase-notes.md,fix/sycl-fp-contract-all-tusentryReproducer
Known follow-ups
T-CUDA-FP-CONTRACT-DEFAULT-2026-09-29,T-HIP-FP-CONTRACT-DEFAULT-2026-09-29). Their division and sqrt are already IEEE. Neither is measured here.div_rn/sqrt_rnduplicate what the line now gives on icpx. Whether plain/is as fast in the SpEED eigenvalue kernel is unmeasured.--feature float_momentnever selects a GPU twin (T-CLI-FLOAT-MOMENT-NO-TWIN-2026-09-29).-Woverriding-optionnow appears for 60 SYCL compiles instead of 12. It is the intended override, and the icx x86 strict libraries already print it 43 times.