fix: prevent NaN outputs for short/non-multiple sequence lengths via CUDA-level zero-fill - #377
Open
Onkitova wants to merge 1 commit into
Open
fix: prevent NaN outputs for short/non-multiple sequence lengths via CUDA-level zero-fill#377Onkitova wants to merge 1 commit into
Onkitova wants to merge 1 commit into
Conversation
…CUDA-level zero-fill
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
This PR fixes a bug where SageAttention produces
NaNoutputs when the sequence length is not a multiple of the block size (e.g., short sequences likeseq_len = 9in Krea 2 Turbo models, or non-multiple shapes likeQ_len = 1001, KV_len = 503).Root Cause
In
csrc/qattn/attn_utils.cuh, the predicated asynchronous global-to-shared memory copy helperload_global_to_sharewas configured withcp_async::SharedMemFillMode::kNoFill:smem.load_128b_async<cp_async::SharedMemFillMode::kNoFill>(smem_offset, *lane_ptr, base_idx < max_len);When
base_idx >= max_len(i.e. out-of-bounds elements in the boundary/tail block),kNoFillskips loading, leaving the shared memory tile with uninitialized garbage from previous GPU operations.When the Tensor Cores perform the QK matrix multiplication, they read this garbage (which can contain NaNs or subnormal values). Although the out-of-bounds attention scores are later correctly masked to
-5000000.0finapply_out_of_bound_mask,NaNvalues produced during the intermediate matrix multiplication contaminate the calculations and propagate through arithmetic operations (since0 * NaN = NaN).Fix
We changed the fill mode to
cp_async::SharedMemFillMode::kFillZero:smem.load_128b_async<cp_async::SharedMemFillMode::kFillZero>(smem_offset, *lane_ptr, base_idx < max_len);This instructs the hardware (
cp.asyncinstruction) to automatically zero-fill the out-of-bound bytes. Since the out-of-bounds scores are masked later anyway, loading clean zeros instead of uninitialized garbage prevents NaN generation and propagation entirely.Note
Note on FP16 vs FP8 paths:
This fix resolves the NaN issue for Q, K, and V loading in the FP16 paths, and Q and K loading in the FP8 paths. The FP8 V loading path (
load_fp8_V_global_to_share) remains unpredicated because the FP8 V tensor is already pre-padded to a multiple of 64 during Python-level quantization preprocessing, making it inherently memory-safe.Performance Impact
The performance overhead is negligible (~0.0%):
kFillZeroperforms acp.asyncwith 0 source bytes to clear the shared memory tile. This is handled entirely within the fast SM shared memory pipeline and does not consume any global memory bandwidth.Verification & Tests
tests/test_sageattn.pythat:1to65.Q=1001,KV=503) across bothHNDandNHDtensor layouts.