Add a cuDNN SDPA decode tier to the ONNX standard Attention CUDA kernel (Phase 1) - #29715
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds a cuDNN SDPA-backed decode-specialized tier to the ONNX-domain Attention CUDA kernel (opset 24 external-KV-cache, single-token causal decode), inserting it ahead of the existing Flash → MEA → Unfused cascade, and introduces a Python parity test intended to exercise the new tier via sdpa_kernel provider options.
Changes:
- Add
RunCudnnSdpaAttentionand a tightly gated cuDNN SDPA eligibility block so the effective cascade becomes cuDNN → Flash → MEA → Unfused for the targeted decode case. - Add AttentionKernelDebugInfo wiring to record which tier ran (when debug info is enabled).
- Extend the TensorScatter+Attention Python test harness to accept CUDA provider options and add cuDNN-decode parity cases (with unfused fallback).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| onnxruntime/core/providers/cuda/llm/attention.cc | Implements cuDNN SDPA decode tier, adds gating in ComputeInternal, and emits tier debug-info prior to early returns. |
| onnxruntime/core/providers/cuda/llm/attention.h | Declares the cuDNN SDPA entrypoint and stores cuDNN enable/auto-enable flags on the kernel instance. |
| onnxruntime/test/python/transformers/test_onnx_attention/test_tensorscatter_attention.py | Adds provider-option plumbing to the test runner and introduces cuDNN-decode parity test cases using sdpa_kernel selection with fallback. |
Consolidated review — Phase-1 cuDNN SDPA decode tierThanks for the Phase-1 implementation. Five reviewers went over the diff against the design (#29714). The core implementation is correct: the spec-level mappings were verified faithful — the decode frontier ( The findings below are about gating, test coverage, and graph-capture safety — not wrong outputs. Major (please address before merge)1. Drop 2. Tests never assert cuDNN was actually selected (can silently pass on MATH fallback). 3. The 4-D BNSH mixed-layout branch is completely uncovered. 4. The CUDA-graph invariant is untested and unenforced. Minor / Nits
Verified — no action neededpresent-population (no double-write, correct length, GQA-identical), fully-masked guard (layout-safe, batch==row for decode), layout/stride branch, includes/layering, |
@copilot Can you fix them, if these are valid? |
…sal=0 decode cases
Addressed the valid gating/coverage findings in 0490bdc: Major 1 — Removed Major 2 — The decode test now enables Minors — Added Deferred (flagged as follow-ups): Major 3 (4-D BNSH branch) and the BF16 / asymmetric |
Re-review (round 2) — after the updateThanks for the revisions. The two headline items are in good shape; four Major items remain, all in the test layer (they don't affect the kernel logic, which was verified correct, but they block trustworthy verification of this path). ✅ Resolved & verified faithful
🔴 Major — please address1. (New) The cuDNN-capability probe reads torch's cuDNN version, not ORT's.
2. The 4-D BNSH mixed-layout path is still uncovered. 3. CUDA-graph capture is still untested. 4. BF16 is still untested. 🟡 Nits (non-blocking)
Verified — no action needed
|
|
Related to #29717? |
|
@xadupre Yes, they are related through the cuDNN SDPA integration, but they target different Attention implementations and use cases. #29717 extends the contrib |
Scope-B minimal: extends test_tensorscatter_attention.py with opset-24 external-KV-cache q_seq==1 decode fast-path coverage. Pre-existing tests and torch usage are unchanged. - M1: replace the torch.backends.cudnn.version() probe with an observe-dispatch cudnn_decode_supported() that runs a minimal decode with ORT_ENABLE_ATTENTION_KERNEL_DEBUG_INFO and asserts ORT actually routes to CUDNN_FLASH_ATTENTION (fixes the torch-vs-ORT trustworthiness bug). - M2: 4-D BNSH q/k/v coverage via use_4d in the graph builder and runner. - M3: CUDA graph capture/replay test for the decode tier. - M4: bfloat16 coverage using the torch data_ptr() IO-binding pattern (test_gqa style). - Canary: ORT_TEST_REQUIRE_CUDNN_SDPA makes every decode dispatch assertion non-skippable and fails loud on MATH fallback, closing the all-skips-green hole. - Minors: bit-exact present_k/present_v parity (pure D2D copy), explicit session/io-binding release (del + gc.collect()), bf16 tolerance comment, and stale bf16 comment fixes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fills the multi-batch validation-loop-index gap: a valid batch-0 must not mask a
bad batch-1. Feeds nonpad_kv_seqlen={0,-3} (batch=2, kv=4) on the CPU EP and asserts
the host-validation rejection (core/providers/cpu/llm/attention_helper.h). Mirrors
GQA's MultiBatchOneBadSeqlensK_OOB. TEST-ONLY; no kernel change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@titaiwangms — measured the Phase-1 tier against the design's premise (#29714: cuDNN takes external-cache decode from the Flash floor to GQA-class latency), using the #28352/#29684 benchmark plus a tier-on arm. Setup: PR head Decode latency, µs (fp16 / bf16)
ᵃ residual box jitter. Routing asserted per timed session via this PR's debug-info wiring (
Repro: |
…patch verification) Documents the hard-won GPU transformers pytest gotchas: the neutral + private mktemp-cwd rule (avoids the repo-root source-shadow ModuleNotFoundError and the shared /tmp sitecustomize injection vector), LD_PRELOAD lib-pinning when torch's bundled CUDA/cuDNN shadow ORT's, and how to prove real cuDNN SDPA dispatch via ORT_ENABLE_ATTENTION_KERNEL_DEBUG_INFO + the ORT_TEST_REQUIRE_CUDNN_SDPA non-skippable canary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nSdpaAttention
RunCudnnSdpaAttention called context->GetComputeStream() directly, which
does not exist on the plugin-EP adapter OpKernelContext
(onnxruntime::ep::adapter::OpKernelContext exposes GetGPUComputeStream),
breaking all three CUDA Plugin EP builds (Linux, Windows with/without cuDNN).
Switch to the established dual-build-safe helper GetOrtStream(context) and
pass ort_stream.get() to cudnn_sdpa::run(), matching the pattern used by the
other CUDA attention kernels (contrib_ops/cuda/bert/{attention,multihead_attention,
group_query_attention,decoder_attention,paged_attention}.cc).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f38b9775-a6b6-4422-8f42-9e48e626a358
275719e to
df87b1a
Compare
tianleiwu
left a comment
There was a problem hiding this comment.
Review: cuDNN SDPA decode tier (Phase 1)
Verdict: COMMENT (not approving yet — the author notes CUDA was not buildable in their environment, so compilation + GPU tests are unvalidated, and this is an explicitly phased draft).
Overall
This is a well-scoped and unusually well-documented change. The new RunCudnnSdpaAttention faithfully mirrors RunFlashAttention's Path-1/prompt population, the cudnn_sdpa::run(...) and is_supported(...) argument order matches the wrapper header exactly, and the whole design correctly stays CUDA-graph-capturable by reading the valid KV length device-side (LaunchConvertNonpadKvSeqlenToFlashSeqlensK) with no D2H sync — the exact failure mode that sank #29689.
Things that are done well
- Cascade placement — cuDNN is gated in front of Flash/MEA/Unfused and is intentionally not wrapped in
USE_FLASH_ATTENTION/USE_MEMORY_EFFICIENT_ATTENTION, matching GQA; hoistinghas_output_qkto an unconditional[[maybe_unused]]is the correct consequence. - Fully-masked-batch guard — zeroing
nonpad==0rows afterrun()is a genuine spec-equivalence requirement (cuDNN emits NaN over an all-infrow), applied on the BSNH buffer before the BSNH→BNSH transpose with the correct per-batch element stride. It cannot be a host-side gate without breaking capture; that reasoning is sound. is_causalnot gated — correct fors_q==1:is_supportedskips the causal branch andrun()drops causal masking, sois_causal={0,1}collapse to the identical padding-only frontier. Confirmed againstcudnn_flash_attention.cc.- Parameter consistency — present_key/value population uses
kv_sequence_length, which equalstotal_sequence_length(used for cuDNN strides) precisely becausepast_sequence_length == 0is enforced. Correct, though implicit. - Tests — the debug-info routing assertion (
SdpaKernel=CUDNN_FLASH_ATTENTION), theORT_TEST_REQUIRE_CUDNN_SDPAnon-skippable CI gate + canary, and the capture/replay-with-mutation test (proving the length is re-read from the device buffer, not baked in) close the exact holes that make a decode-tier test silently pass on MATH fallback.
Cross-cutting notes
- Scope —
.agents/skills/ort-transformers-gpu-pytest/SKILL.md(+218 lines) is developer tooling unrelated to the runtime feature. Consider splitting it into a separate PR so the functional change reviews cleanly on its own. - Merge gating — since the code path could not be compiled/run in the author's environment, please attach a green GPU CI leg that actually shows
CUDNN_FLASH_ATTENTIONdispatch (e.g. withORT_TEST_REQUIRE_CUDNN_SDPA=1) before this leaves draft. - The two concerns from the earlier automated review round (undocumented
provider_options; missing routing assertion) are already addressed on this head.
One minor test-robustness nit inline.
…l head config cudnn_decode_supported() previously probed a single hardcoded config (q_heads=1, kv_heads=1) keyed only by head_size, while the assertion it gates runs against the case's actual head counts. This was safe only because all current cases share a divisible head ratio and head_size=64. Thread (q_num_heads, kv_num_heads) through _observe_cudnn_decode_dispatch and cudnn_decode_supported (cache keyed by the full tuple) and update all call sites to pass the case's real head configuration, so the probe can't drift from what is_supported() will actually dispatch for that case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d32dce6-edfc-4d15-9d94-0bc65713f01d
tianleiwu
left a comment
There was a problem hiding this comment.
Review round 2 — head 76f8cad4
Verdict: COMMENT (still not approving — the PR states CUDA was not buildable in the author's environment, so compilation and GPU tests remain unvalidated).
My previous inline nit (cudnn_decode_supported() probing a hardcoded config) is addressed on this head and that thread is resolved. Re-checking the kernel on this head, the following are confirmed correct and need no action:
sequence_length_kv = total_sequence_length(cuDNN strides) vskv_sequence_length(present-cache population) — equal, becausepast_key == nullptrforcespast_sequence_length == 0.- Fully-masked zero-fill
elements_per_batch = q_seq * q_num_heads * v_head_sizematches the BSNH scratch allocation in both the 3-D and 4-D branches, and runs before the BSNH→BNSH transpose. is_causalintentionally ungated fors_q == 1:is_supportedskips the causal branch andrun()foldsis_causal &&= (s_q > 1), so both values share one cached plan and the same padding-only frontier.- The unguarded cuDNN block still builds under
--disable_contrib_ops(the cmakecontrib_ops/cuda/bert/.*filter coverscudnn_fmha/) and is excluded underCUDA_MINIMAL(non-recursive glob overcore/providers/cuda/). AttentionKernelOptionsis a per-provider member, so the per-sessionsdpa_kernel/ORT_ENABLE_ATTENTION_KERNEL_DEBUG_INFOmanipulation in the tests behaves as intended (no cross-teststd::once_flagleakage).
The remaining concerns are about how this lands, not the math. Details inline; two are worth calling out here:
1. The tier becomes default-ON on SM≥90, and that path has no test coverage. AllowCudnnFlashAttentionAuto() is true in the default configuration, so auto_enable_cudnn_flash_attention_ is true and every opset-24 external-cache decode on H100/H200/B200 silently moves off Flash onto this tier. But every test here passes sdpa_kernel=24, which sets has_explicit_kernel_selection_ and therefore disables the auto path — so the route production will actually take is never exercised. Given the code could not be compiled or run by the author, please either make Phase 1 opt-in (enable_ only) until a green GPU leg exists, or add a case that omits sdpa_kernel.
2. No latency measurements. The tier is justified entirely by a ~3 µs decode delta, but the PR contains no before/after numbers, and the implementation adds several per-step launches/allocations (details inline). Please attach a measurement, and note that the ~10 µs target is only reachable when present_key/present_value are not requested — otherwise the full-KV-cache copy dominates any kernel-level win.
Merge gating (repeating from the previous round): please attach a green GPU CI leg that actually shows SdpaKernel=CUDNN_FLASH_ATTENTION dispatch before this leaves draft. And .agents/skills/ort-transformers-gpu-pytest/SKILL.md (+218 lines of developer tooling) would still review more cleanly as a separate PR.
verification, auto-enable test, bare-except hardening, canary docstring
Responds to the remaining inline comments from tianleiwu's two review passes
on the cuDNN SDPA decode tier:
- attention.cc: document why the warmup-before-capture invariant holds
(thread_local cuDNN plan cache + PerThreadContext capture counter are both
per-thread, so warmup and capture Runs always land on the same thread) and
what would silently break it. Also fix stale dispatch-cascade comments to
include the new cuDNN tier.
- test_tensorscatter_attention.py:
- Generalize _CaptureStdout -> _CaptureNativeFd(fd=...) to also capture
fd 2 (stderr), where ORT's default logger sink writes.
- TestTensorScatterAttentionCudnnSdpaDecodeCudaGraph now actually verifies
CUDA-graph capture engaged (scans the CUDA EP's "Capturing the cuda
graph for this model" INFO log across warmup Runs) instead of assuming
it from the mutate/replay behavior alone. The assertion requires this
test's own session logid on the same log line as the capture message,
since fd 2 is a process-global descriptor and a bare substring match
could false-pass on an unrelated concurrent session's capture line.
- Add TestTensorScatterAttentionCudnnSdpaDecodeAuto: exercises the default
(auto-enable, no explicit sdpa_kernel) dispatch path, which every other
test in this file bypasses by pinning sdpa_kernel explicitly.
- cudnn_decode_supported(): replace bare except/pass with a warning so a
genuine harness/kernel bug surfaces instead of masquerading as "cuDNN
unavailable, skip".
- _observe_cudnn_decode_dispatch(): release the probe session eagerly
(del + gc.collect()), mirroring run_tensorscatter_attention.
- Soften docstrings that implied ORT_TEST_REQUIRE_CUDNN_SDPA is already
enforced by CI; no pipeline exports it today, so it is manual/local only.
- benchmark_onnx_attention_vs_gqa.py (new): add an attn_scatter_cudnn arm
(cuDNN SDPA enabled) alongside the existing attn_scatter Flash baseline,
with backend-assertion coverage for both, to make the decode speedup
measurable end to end.
- SKILL.md: sync the canary section's CI-enforcement wording with the above.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a1011908-d09e-411a-8979-fcf0dc18e20e
# Conflicts: # onnxruntime/test/python/transformers/benchmark_onnx_attention_vs_gqa.py
… 4D path) Addresses tianleiwu's decode-path kernel-launch-overhead feedback on CreateConstantSeqLenBuffer/small-kernel-count (comment 3654616716) for the one part of that overhead we can act on without touching the shared cudnn_flash_attention.cc wrapper or GQA/MHA. The cuDNN SDPA decode tier's 4D (BNSH) output path previously ran two separate kernel launches after cudnn_sdpa::run(): a standalone LaunchZeroOutputForFullyMaskedBatches on the BSNH scratch buffer (required because cuDNN's softmax over an all-masked row is undefined, while the ONNX spec and every other tier defines output=0 there), followed by a TransposeBSNHtoBNSH to the final output tensor. Both passes touch every output element, so they are fused into a single new kernel, TransposeBSNHtoBNSHWithZeroMask, that reads BSNH and writes BNSH while zeroing masked batches in the same pass. Scope is intentionally narrow: - New kernel lives in attention_mask_impl.cu/.h (files this PR already owns), not in the shared attention_transpose.cu (Transpose_BSNH_to_BNSH / LaunchTransQkv / TransposeQKV) used by GQA, MHA, and sparse-attention. That shared pipeline is untouched. - The 3D path (no transpose needed) and the MEA tier's own call to the original LaunchZeroOutputForFullyMaskedBatches are both unchanged. - Only RunCudnnSdpaAttention's 4D branch is rewired to call the fused kernel instead of the standalone zero-fill + transpose pair. Testing: - New TestTensorScatterAttentionCudnnSdpaDecodeFused4D class (16 cases): masked batch first/last/middle, multiple non-adjacent masked batches, GQA/MQA head configs, all-masked, no-masked. Full suite: 113/113 passing (97 pre-existing + 16 new). - nsys profile (A100): 2 launches / 4543ns -> 1 launch / 3362ns per decode step (-26% GPU time on the fused portion). - Reviewed by code-review + critical-reviewer agents: indexing math verified against BNSH/BSNH layout, out_bsnh_buffer null-guard confirmed unreachable for the 4D case, CUDA-graph-capture launch-config safety confirmed (no data-dependent grid sizing), no aliasing hazard.
Description
Adds a cuDNN SDPA tier to the ONNX
Attention(opset 23/24) CUDA kernel, making the effective dispatch cascade cuDNN → Flash → MEA → Unfused. The tier is narrowly gated to the opset-24 external-KV-cache single-token decode path (its actual value proposition) and takes the valid KV length as a deviceint*, so it stays CUDA-graph-capturable — unlike the host-readback approach that sank #29689.This is a draft landing Phase 1 only; correctness-sensitive follow-ups are gated out and flagged inline.
attention.ccComputeInternal) — new cuDNN eligibility block before the Flash block. Hard gate (all required):nonpad_kv_seqlen != nullptr,past_key == nullptr,is_causal,q_sequence_length == 1, noattn_mask/output_qk/softcap, fp16/bf16,cudnn_sdpa::is_supported. Not built-guarded (relies on the wrapper'sCUDNN_MAJORstubs +is_stable()).RunCudnnSdpaAttention— honors existing ONNX-Attention semantics: rank-branchedqkv_format(Q_K_V_BSNHfor 3-D, mixedQ_K_V_BSNH_BNSH_BNSHfor 4-D with Q transposed BNSH→BSNH), BSNH output scratch → transpose to BNSH, device int32 seqlens via the existingLaunchConvertNonpadKvSeqlenToFlashSeqlensK,sequence_length_kv = total_sequence_length(capacity) + per-batch mask,mask_sequence_lengths_q = nullptr, separatepresent_key/present_valuepopulation.LaunchZeroOutputForFullyMaskedBatchesafterrun(). cuDNN emits NaN for anonpad==0row where every other tier defines0; this is a spec-equivalence requirement, not defense-in-depth (can't be a host-side gate without a D2H sync).UseCudnnFlashAttention()/AllowCudnnFlashAttentionAuto()(no new option key); addsenable_/auto_enable_cudnn_flash_attention_members mirroring GQA.AttentionKernelDebugInfodispatch block the op previously lacked, recording which tier ran.sdpa_kernel, covering MHA/MQA/GQA, fully-masked and heterogeneousnonpad; falls back to unfused (still parity-correct) where cuDNN is unavailable.Deferred / needs validation (inline TODOs): Phase 2 (internal
past_key/past_value) and Phase 3 (prefills_q>1) are gated out. Cold cuDNN plan-cache miss under graph capture relies on the warmup-before-capture invariant (documented in the function header); wrapper-API enhancement + a C++ cold-cache capture test are follow-ups. CUDA was not buildable in this environment — compilation and GPU tests need validation on cuDNN 9.3+/SM≥90.Motivation and Context
The standard ONNX
AttentionCUDA kernel has no decode-specialized tier, capping decode latency at the Flash floor (~13 µs) versus GQA's cuDNN/XQA tiers (~10 µs). Fixing Flash split-sizing (#29686, wontfix) can't close this structural gap. cuDNN SDPA is the widest-eligibility, already-linked option and reads valid length device-side, so it reaches GQA-class decode latency without breaking CUDA-graph capture.