Repository navigation
refactor(cuda): collapse two decode levers that were never a real choice - #1577
Merged
Merged
Conversation
Removes two `ONNX_GENAI_*` decode levers so the paths they guarded become the only paths. Each is removed for a different reason, and a third candidate is deliberately KEPT. `ONNX_GENAI_GEMV_BF16_DIRECT_OUT` was default-ON, so deleting it changes no behaviour at all — it only removes a dead branch and the possibility of running the untested configuration. The path stays guarded by `bf16_direct_store_matches_staged_cast_bit_for_bit` and `bf16_direct_store_declines_the_small_batch_row_loop`. `ONNX_GENAI_RMSNORM_WARP_REDUCE` was default-OFF, so this one is a real flip: the warp-shuffle tail is now unconditional. It is bit-identical to the shared-memory tree (covered by the CPU-side `warp_shuffle_skip_rmsnorm_matches_shared_tree_for_hidden_and_tail_sizes`) and strictly cheaper — five fewer `__syncthreads` per row. It was gated only on "should be measured per machine"; measured, and the difference is below this machine's noise floor. Verified end-to-end on Muse Glimmer 30B (bf16, 626 rmsnorm launches/token, so the path is genuinely exercised): 96 greedy tokens byte-identical before and after, throughput unchanged. The GPU test `warp_reduce_tail_is_byte_identical_to_shared_tree` goes with it: it existed only to A/B the two entries by writing the process environment, and the property it checked is covered by the CPU-side test above. The now-unreachable non-warp NVRTC entries are kept on purpose — NVRTC compiles entries lazily by name, so an unused entry costs nothing at runtime, while deleting CUDA source strings is the riskier edit. `ONNX_GENAI_INTERLEAVE_DEQUANT` is NOT removed, contrary to where this change started. It looked like an unmeasured default-off lever, but the archived decision log has it at a reproducible +8.6% on H200 (211.9 -> 230.0 tok/s). Removing it would have thrown away a real, measured win that this model simply cannot see, because the path requires symmetric weights and Muse Glimmer is asymmetric. Investigating that turned up the reason it must STAY opt-in, which no comment recorded: `ensure_interleaved` allocates a second copy of every routed weight and keys the cache off the original ORT-owned pointer, which is never freed. Routed int4 weights are therefore resident twice for the life of the process — the cache caps at 4096 entries while a 52-layer model registers a few hundred, so nothing is ever evicted. That is ~20 GB of extra VRAM on a 20 GB int4 model. Both the lever's doc comment and the research note that recommended it for RTX are corrected: RTX is exactly where doubling weight residency is fatal, so "helps RTX most" was the most dangerous place for the omission. Also fixes a mangled doc-comment line that had swallowed the lever name. Tests: 450 passed / 0 failed / 24 ignored (was 451; the removed A/B test accounts for the difference). Workspace builds clean with no new warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e
justinchuby
added a commit
that referenced
this pull request
Aug 20, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1577 +/- ##
=======================================
Coverage 82.64% 82.64%
=======================================
Files 12 12
Lines 5475 5475
Branches 5475 5475
=======================================
Hits 4525 4525
Misses 757 757
Partials 193 193
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
Removes two
ONNX_GENAI_*decode levers so the paths they guarded becomethe only paths. Each is removed for a different reason, and a third
candidate is deliberately KEPT.
ONNX_GENAI_GEMV_BF16_DIRECT_OUTwas default-ON, so deleting it changesno behaviour at all — it only removes a dead branch and the possibility of
running the untested configuration. The path stays guarded by
bf16_direct_store_matches_staged_cast_bit_for_bitandbf16_direct_store_declines_the_small_batch_row_loop.ONNX_GENAI_RMSNORM_WARP_REDUCEwas default-OFF, so this one is a realflip: the warp-shuffle tail is now unconditional. It is bit-identical to
the shared-memory tree (covered by the CPU-side
warp_shuffle_skip_rmsnorm_matches_shared_tree_for_hidden_and_tail_sizes)and strictly cheaper — five fewer
__syncthreadsper row. It was gatedonly on "should be measured per machine"; measured, and the difference is
below this machine's noise floor. Verified end-to-end on Muse Glimmer 30B
(bf16, 626 rmsnorm launches/token, so the path is genuinely exercised):
96 greedy tokens byte-identical before and after, throughput unchanged.
The GPU test
warp_reduce_tail_is_byte_identical_to_shared_treegoes withit: it existed only to A/B the two entries by writing the process
environment, and the property it checked is covered by the CPU-side test
above. The now-unreachable non-warp NVRTC entries are kept on purpose —
NVRTC compiles entries lazily by name, so an unused entry costs nothing at
runtime, while deleting CUDA source strings is the riskier edit.
ONNX_GENAI_INTERLEAVE_DEQUANTis NOT removed, contrary to where thischange started. It looked like an unmeasured default-off lever, but the
archived decision log has it at a reproducible +8.6% on H200 (211.9 ->
230.0 tok/s). Removing it would have thrown away a real, measured win that
this model simply cannot see, because the path requires symmetric weights
and Muse Glimmer is asymmetric.
Investigating that turned up the reason it must STAY opt-in, which no
comment recorded:
ensure_interleavedallocates a second copy of everyrouted weight and keys the cache off the original ORT-owned pointer, which
is never freed. Routed int4 weights are therefore resident twice for the
life of the process — the cache caps at 4096 entries while a 52-layer
model registers a few hundred, so nothing is ever evicted. That is ~20 GB
of extra VRAM on a 20 GB int4 model. Both the lever's doc comment and the
research note that recommended it for RTX are corrected: RTX is exactly
where doubling weight residency is fatal, so "helps RTX most" was the most
dangerous place for the omission. Also fixes a mangled doc-comment line
that had swallowed the lever name.
Tests: 450 passed / 0 failed / 24 ignored (was 451; the removed A/B test
accounts for the difference). Workspace builds clean with no new warnings.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e