Skip to content

[Kernel] Skip KV writes to reserved padding slots - #32477

Merged
ispobock merged 2 commits into
sgl-project:mainfrom
YazhiGao:agent/skip-kv-write-reserved-padding-slots
Jul 29, 2026
Merged

ispobock merged 2 commits into
sgl-project:mainfrom
YazhiGao:agent/skip-kv-write-reserved-padding-slots

Conversation

@YazhiGao

@YazhiGao YazhiGao commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Motivation

CUDA-graph padding rows can contain undefined K/V values while still targeting the reserved padding slot at index 0. In sliding-window attention, page-table padding also points at that reserved page. A partially valid tile may load the page before masking invalid lanes, allowing a NaN value to contaminate the result through 0 * NaN.

Changes

  • add a reserved_skip_index argument to the JIT KV-cache store kernel
  • skip K/V copies targeting the reserved slot while preserving the PDL trigger
  • default to skipping the CUDA-graph padding slot at index 0, with -1 as an opt-out
  • cover int32/int64 indices, split counts 1/2/4, reserved-slot preservation, and the opt-out path

Validation

  • pre-commit run --files python/sglang/kernels/jit/csrc/elementwise/kvcache.cuh python/sglang/kernels/ops/kvcache/kvcache.py python/sglang/srt/mem_cache/memory_pool.py test/registered/kernels/ops/kvcache/test_store_cache.py
  • pytest -q test/registered/kernels/ops/kvcache/test_store_cache.py -x — 340 passed

CI States

Latest PR Test (Base): ✅ Run #30371139759
Latest PR Test (Extra): ✅ Run #30371134897

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@sshleifer sshleifer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zcnrex zcnrex left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ispobock
ispobock merged commit ee67891 into sgl-project:main Jul 29, 2026
147 of 177 checks passed
adityavaid pushed a commit to adityavaid/sglang that referenced this pull request Jul 29, 2026
Co-authored-by: Andrew Gu <andrew@thinkingmachines.ai>
jinzhenfan pushed a commit to jinzhenfan/sglang that referenced this pull request Jul 29, 2026
Co-authored-by: Andrew Gu <andrew@thinkingmachines.ai>
jetd1 pushed a commit to jetd1/sglang that referenced this pull request Aug 3, 2026
Co-authored-by: Andrew Gu <andrew@thinkingmachines.ai>
hassellof added a commit to hassellof/sglang that referenced this pull request Aug 6, 2026
fused_store_flashmla_cache and fused_store_indexer_cache compute
page = index >> kPageBits and store unconditionally, even though the
kernel comment says "always load the value from input (don't store if
invalid)" — the skip was never implemented, and unlike the generic pool
kernel there is no bounds check at all.

Two ways invalid indices reach these kernels in normal operation:

* CUDA-graph padding rows carry out_cache_loc = 0, the allocator's
  reserved slot (PagedTokenToKVPoolAllocator.clear() starts free pages
  at 1, and the SWA page LUT maps 0 -> 0). Undefined padding values are
  silently stored into slot 0, poisoning the page that page-table
  padding also points at; partially-valid tiles read the garbage back
  before masking.

* The SWA page LUT deliberately keeps a live -1 ("no page") entry at
  the end. Unguarded, page goes negative and the store is an
  out-of-bounds write before the cache base.

Guard both kernels with store_valid = index > 0, wrapping only the
stores (not early-returning) so the PDL trigger still fires and warp
reductions see the full warp. The predicate is uniform per block /
per warp, so there is no added divergence.

Same class of fix as sgl-project#32477 for the generic store_cache kernel; the
regression test mirrors test_store_cache_reserved_skip_index. The -1
case is made deterministic without compute-sanitizer by placing a
sentinel-filled guard page immediately before the cache view: the
unguarded kernel's negative-page store lands in the guard page.
saturn-acc pushed a commit to saturn-acc/sglang that referenced this pull request Aug 16, 2026
Co-authored-by: Andrew Gu <andrew@thinkingmachines.ai>
jakki-amd pushed a commit to jakki-amd/sglang that referenced this pull request Sep 9, 2026
Co-authored-by: Andrew Gu <andrew@thinkingmachines.ai>
Atituiset pushed a commit to Atituiset/sglang that referenced this pull request Sep 10, 2026
Co-authored-by: Andrew Gu <andrew@thinkingmachines.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants