cuda: keep exact Q8 MMA loads within tensor bounds - #978
Open
JordiPosthumus wants to merge 1 commit into
Open
Conversation
JordiPosthumus
force-pushed
the
codex/fix-q8-mma-bounded-loads
branch
from
September 5, 2026 20:40
e6f4700 to
5c21574
Compare
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
Read each Q8 code word as two 16-bit loads, rather than reading two aligned
32-bit words and combining them with a funnel shift. Add a model-free CUDA
regression for exactly-sized weight allocations.
Why
ldu32_unaligned()can read beyond a raw tensor's allocation. For an alignedfour-byte address, its second load reads an entirely unused next word. For a
halfword-aligned address, that load can include two bytes beyond the tensor.
Padding in a larger model allocation can hide this; an exact tensor allocation
exposes it under Compute Sanitizer and can produce an illegal-access error.
The Q8 MMA caller already requires halfword-aligned weights, and every Q8 code
offset is even. Two 16-bit loads therefore reconstruct exactly the requested
four bytes without touching neighboring storage. The MMA instructions,
activation quantization, scales and floating-point reduction are unchanged.
There are no new options, allocations, cache formats or sampling changes.
Validation
5c21574c248408d4d8752b0fc9a94cd72813f9c3, one commiton
f4d03f6c. Runtime and fixtures are unchanged by the rebase; Makefilecleanup was reconciled with upstream.
ds4-server,ds4_testandtests/test_cuda_q8_mma_boundswithCUDA_ARCH=sm_121,./ds4_test --server, the focused test and memcheck (zero errors).No engine-source adapter was required on this base.
make -B -j2 all ds4_test,./ds4_test --serverandmake -B -j2 cpupassed (CPU compile/link only). SDK 27 emits 27 existingMetal deprecation warnings. The red-to-green and model/timing results below
are earlier unchanged-runtime measurements against
b0a147a7, not newlyrepeated whole-model comparisons for this rebase.
prefill tokens, widths 32/96/4096, ranks 1/65/128, and odd/even tensor ends.
first fixture. Patched kernel: all three fixtures pass with exact expected
outputs and zero memory errors.
--report-api-errors noonly to suppress the engine's handledcudaHostRegister-unsupported fallback notices. Invalid memory accessesremain checked and cause failure.
remained approximately 1.92–1.98 ms versus 2.02 ms before; no observed speed
regression. These are bounded measurements, not a claimed end-to-end gain.
prefill optimization matched the old binary on 8,403,200 float logits:
32768 -> 36864 context, then 64 teacher-forced steps, Vision-Exp
IQ2XXS-w2Q2K-AProjQ8-SExpQ8-OutQ8, 262144 allocation, 2048 chunks, two
resident sessions. This is combined integration evidence, not a bounds-only
whole-model A/B. The standalone red-to-green memory test is above.
make cpu, and./ds4_test --serverpassed on M3 Ultra;no Mac model was loaded. The full model-backed aggregate suite was not run.
make cuda-regression CUDA_ARCH=sm_121passed after supplying the existingtests/cuda_long_context_smokelink target's missingds4_image.odependency.The earlier
b0a147a7native build also needed the unrelated non-Appleg_tp_block_ctxplatform guards, now fixed in the refreshed upstream base.The old smoke-target link prerequisite is not included in this PR; the new
focused target links its own dependencies.
This was discovered while testing the separate GB10 Q8 prefill optimization #979.
The bounds fix does not require that optimization or any model/launcher change.