Store pre_att_rms_out as BF16 - #987
Open
Mikyx-1 wants to merge 1 commit into
Open
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
pre_att_rms_out is only ever the A operand of the QKV MatMuls, and there is no f32 MatMul kernel: MaybeDecompressA in ops/matmul-inl.h converts it to BF16 on every call, with no caching across calls. Storing it as BF16 lets RMSNormBatched write the final type directly and removes that pass. Its siblings pre_ffw_rms_out, x_bf and att_sums are already BF16, so this applies the "change most activations to bf16" roadmap item from google#164 to one more buffer. Same change proposed in google#560, rebased onto the current templated activations, where every consumer (RMSNormBatched, LayerNormBatched, CallMatMul) is already generic over the element type. This adds no hardware bf16 requirement. MaybeDecompressA returns StridedViewBF in both branches, so the kernel sees the same type and runs the same instructions either way; targets without HWY_NATIVE_DOT_BF16 widen to f32 via PromoteEvenTo exactly as they did before. What goes away is one full pass over M x K, on every target. Measured on Apple M2, gemma2-2b-sfp-pt, 926-token prefill + 64 generated, profiler build: MM.DecompressA: 50,930 calls / 11.9 ms -> 14,816 calls / 3.5 ms The binary also shrinks 656 KB because the f32-A MatMul instantiations for these call sites are no longer emitted. End to end the win is real but humble: ~8 ms of a ~34 s run, below run-to-run noise here, because the conversion is O(M*K) in front of an O(M*K*N) matmul. Output is unchanged. 96 generated tokens are byte-identical before and after, gemma_test's cross entropy is identical to every printed digit (1.108557), and the attention_test goldens pass unchanged on NEON_BF16, NEON_WITHOUT_AES and EMU128. FillRandom in attention_test.cc is templated on the element type so it can still fill the buffer.
Mikyx-1
force-pushed
the
bf16-pre-att-rms-out-dev
branch
from
August 22, 2026 12:53
e7dee50 to
6769a7a
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.
pre_att_rms_outis only ever the A operand of the QKV MatMuls, and there is no f32 MatMul kernel —MaybeDecompressAconverts it to BF16 on every call, with no caching across calls. Storing it as BF16 letsRMSNormBatchedwrite the final type directly and removes that pass.Its siblings
pre_ffw_rms_out,x_bfandatt_sumsare already BF16, so this applies the "change most activations to bf16" item from the #164 roadmap to one more buffer.This is the same change proposed in #560 by Fabian Schuetze, rebased onto the current activations. That patch no longer applies (it predates
MatStorageT, and had to hand-fix aconst float* x); today every consumer —RMSNormBatched,LayerNormBatched,CallMatMul— is already generic over the element type, so it reduces to two type changes.Honest framing: this is cleanup, not a speedup
The redundant work is measurably gone. Profiler build, Apple M2, gemma2-2b-sfp-pt, 926-token prefill + 64 generated:
MM.DecompressAcallsThe binary also shrinks 656 KB, since the f32-A MatMul instantiations for these call sites are no longer emitted, and the buffer's footprint halves (37.7 → 18.9 MB at
prefill_tbatch 4096).But end-to-end there is no measurable speedup, matching the null result reported in #560. Interleaved A/B, 4 reps each: prefill 32.96 → 33.28 tok/s (σ 1.20), total 34.80 → 34.08 s. That's inside one standard deviation.
It can't be otherwise: the conversion is O(M·K) sitting in front of an O(M·K·N) matmul, so it is inherently ~1/N of the work it precedes (N ≈ 2560 here). Worth noting the null result is uniform across architectures — #560 saw it on both a non-bf16 x86 laptop and a Cortex-X3 with native bf16, and this is an M2 with
NEON_BF16. It isn't a bf16-hardware split.No new hardware bf16 requirement
MaybeDecompressAreturnsStridedViewBFin both branches, so the kernel receives the same type and runs identical instructions either way. Targets withoutHWY_NATIVE_DOT_BF16widen to f32 viaPromoteEvenTo/PromoteOddToexactly as they did before — that widening is a property of the kernel and the CPU, not of how this buffer is stored. What goes away is one pass over M×K, on every target.Behaviour is unchanged
gemma_testcross entropy identical to every printed digit:1.108557.attention_testgoldens pass unchanged onNEON_BF16,NEON_WITHOUT_AESandEMU128— no golden regeneration needed.FillRandominattention_test.ccis templated on the element type so it can still fill the buffer; without this the Bazel build breaks.Testing limitations
attention_testtolerance is loose (KV_t = BF16→ 3e-2), so that pass is not a tight numerical proof — the byte-identical generation is the stronger evidence.pre_att_rms_outalso feeds DeepSeek MLA, T5Gemma and ViT/Gemma4-ViT; those compile but were not run.tiled_attention_testcould not be built to verify — it needs gmock+absl and doesn't build under CMake at all (pre-existing). It uses the already-templatedFillMatPtrT, so it should be unaffected.Happy to drop this if it isn't worth the review cycle — the value is genuinely marginal and I'd rather not misrepresent it as a perf win.
🤖 Generated with Claude Code