Skip to content

docs(sycl): prove the integer_adm shift amount cannot go negative - #1284

Merged
lusoris merged 1 commit into
masterfrom
fix/sycl-adm-shift-reachability
Sep 5, 2026
Merged

docs(sycl): prove the integer_adm shift amount cannot go negative#1284
lusoris merged 1 commit into
masterfrom
fix/sycl-adm-shift-reachability

Conversation

@lusoris

@lusoris lusoris commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Settles the open question T-SYCL-ADM-NEGATIVE-SHIFT-REACHABILITY-2026-09-04: at integer_adm_sycl.cpp lines ~705 and ~1032 the kernel computes clz = 31 - n from a hand-rolled leading-zero count and then shifts by ks = 17 - clz. If clz > 17 that shift would be negative — undefined behaviour. A tidy-cleanup agent had clamped it to >= 1; #1281 reverted that as an unjustified divergence from the CPU. This PR is the analysis that decides who was right.

Verdict: NOT-A-BUG — the negative shift is algebraically unreachable. Both sites sit in the else arm of an abs_oh >= 32768 guard, so tmp >= 2^15, the MSB index n >= 15, clz = 31 - n <= 16, and ks = 17 - clz lies in [1, 17]. The CPU reference has the identical contract: integer_adm.c:589 calls get_best15_from32(abs_o, …) only when abs_o >= 32768, giving __builtin_clz(temp) <= 16 and k = 17 - clz >= 1 with no clamp (integer_adm.c:556-563). CUDA's __clz twin relies on the same guard. The reverted clamp was therefore dead code — but it would also have masked the invariant had it ever been violated, which is worse than a crash.

Both sites now carry an INVARIANT comment stating the guard and the derivation, so the next reader does not re-open this. No numeric change.

Live check on the Arc

SYCL vs CPU on the Netflix src01_hrc00/hrc01 pair for integer_adm: max abs delta 3.1e-05 (integer_adm_scale1), within the places=4 tolerance (5e-05) — unchanged by this PR, since nothing numeric moved; recorded here as the baseline the reverted clamp would have disturbed for clz > 16 inputs, which do not exist.

Type

  • docs — comments + state ledger; no code-path change

Checklist

  • Commits follow Conventional Commits.
  • make format && make lint is green locally.
  • Unit tests pass — no compiled change (comments only).
  • SIMD/GPU numeric paths — no numeric change; the cross-backend diff above is the evidence. Twins, new sources, breaking, ADR — n/a.
  • Docs — no docs needed: invariant comments in internal SYCL kernel with no user-visible delta

Bug-status hygiene (ADR-0165)

  • docs/state.md — the open row moves to Recently closed with the NOT-A-BUG verdict and the derivation.

Netflix golden-data gate (ADR-0024)

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: trivial. The whole analysis is the value-range derivation above and fits in the state.md row.
  • Decision matrix — no alternatives: only-one-way fix. Once the guard is shown, the only defensible change is a comment recording it; a clamp would hide a violated invariant.
  • AGENTS.md invariant note — no rebase-sensitive invariants beyond what docs/rebase-notes.md now records (do not clamp ks; the abs_oh >= 32768 guard is the invariant).
  • Reproducer / smoke-test command — below.
  • CHANGELOG fragment — no changelog needed: no user-visible change (comments and the bug ledger only).
  • Rebase notedocs/rebase-notes.md.

Reproducer

# CPU: the same shift, guarded the same way, no clamp
git show origin/master:core/src/feature/integer_adm.c | sed -n '556,563p;589p'
# SYCL: both sites, with the guard a few lines above each
grep -n 'int const ks = 17 - clz' core/src/feature/sycl/integer_adm_sycl.cpp

🤖 Generated with Claude Code

@lusoris lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026
Document algebraic invariant proof showing clz > 17 (negative shift) is
unreachable in core/src/feature/sycl/integer_adm_sycl.cpp (T-SYCL-ADM-NEGATIVE-SHIFT-REACHABILITY-2026-09-04).
Because normalization is guarded by abs_oh >= 32768 (2^15), MSB index
n >= 15, so leading zeros clz = 31 - n <= 16 and ks = 17 - clz >= 1.
Matches unclamped CPU reference and CUDA twin.
Move task row to Recently closed in docs/state.md as NOT-A-BUG.
@lusoris
lusoris force-pushed the fix/sycl-adm-shift-reachability branch from 3d74569 to 8cf3be7 Compare September 5, 2026 10:35
@lusoris
lusoris marked this pull request as ready for review September 5, 2026 10:35
@lusoris
lusoris merged commit f9ee83c into master Sep 5, 2026
129 of 134 checks passed
@lusoris
lusoris deleted the fix/sycl-adm-shift-reachability branch September 5, 2026 11:23
@lusoris lusoris added the type:docs Documentation updates label Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:docs Documentation updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant