Repository navigation
Conversation
…ound cuDNN's left_bound = L keeps the L keys ending at the diagonal, diagonal included (cudnn-frontend sliding_window_mask keeps col + L > row). hipDNN kept L + 1, because its references, provider window mappings and golden generator all used FlashAttention's window_size_left convention. Code written against the cuDNN API (set_sliding_window_length or set_diagonal_band_left_bound) therefore attended to one extra key per query. - References: the CPU dense/ragged and GPU dense/ragged SDPA references now mask skv <= sq + off - L. - Frontend: SDPA forward/backward validation rejects left_bound = 0 (and < -1), as cuDNN does; -1 stays "unbounded". The setters document the cuDNN meaning. - Providers: resolveDiagonalBand converts the graph's left_bound to the kernels' FlashAttention window (L - 1) for asm/AITER, hip_flash2 and gfx950; the gfx1151 pack converts the same way. Kernels and rocKE keep their FlashAttention convention. - Golden generator: --window-left stays FlashAttention-style, but the graph now carries left_bound = W + 1. - Shape miner: left_bound passes through as rocKE's sliding_window. - Tests: a cuDNN-rule oracle (from cudnn-frontend's fp16_ref.py) checks the generator's mask; the reference, provider, frontend and miner tests cover the new meaning and the rejection of 0. Behavior change: callers that passed FlashAttention-style values into left_bound must now pass W + 1. Co-Authored-By: Claude <noreply@anthropic.com>
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #14287 +/- ##
========================================
Coverage 75.96% 75.96%
========================================
Files 2880 2880
Lines 479017 479083 +66
Branches 71962 71976 +14
========================================
+ Hits 363846 363903 +57
- Misses 90316 90319 +3
- Partials 24855 24861 +6
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
|
Superseded by #14483, which carries this commit unchanged (rebased on current develop, authorship kept) plus follow-ups for code that landed since and a |
BradPepersAMD
added a commit
that referenced
this pull request
Oct 28, 2026
…ound (#14483) ## Summary hipDNN's SDPA `left_bound` (and `set_sliding_window_length`) now means what it means in cuDNN, whose API hipDNN mirrors: **`left_bound = L` keeps the L keys ending at the diagonal, diagonal included.** Until now every hipDNN component kept **L + 1** keys (FlashAttention's `window_size_left` convention), so a graph written against the cuDNN contract attended to one extra key per query, forward and backward. JIRA ID : ALMIOPEN-3024 This supersedes #14287 by Daming Feng: his commit is included unchanged (same patch, authorship kept), rebased on current `develop`, with follow-ups for code that landed since and a generator flag that names each convention explicitly. ## Description cuDNN's left bound is a `CMP_GT` diagonal-band mask that keeps `col + L > row` (cudnn-frontend `attn_score_modifiers.h` `sliding_window_mask`; reference `test/python/sdpa/fp16_ref.py` masks `rel <= diag_offset - left_bound`). Example, `L = 2`, query 5, TOP_LEFT: cuDNN keeps keys {4, 5}; hipDNN kept {3, 4, 5}. The right bound already matched. Nothing caught it because every check shared hipDNN's convention: the golden generator built its own mask and cross-checked against AITER with the same value, the engine tests compared against our own CPU reference, and no golden bundle uses a left bound. | Convention | Name | `L = 3`, query on the diagonal | |---|---|---| | hipDNN graph (now), cuDNN | `left_bound`, `set_sliding_window_length` | the 3 keys ending at the diagonal | | vLLM | `sliding_window = W` | W keys including the token (`left_bound = W`) | | FlashAttention, AITER, TransformerEngine, FlashInfer | `window_size_left` / `window_left` | the 3 keys before the diagonal plus the diagonal (`left_bound = W + 1`) | Changes: - **References:** `CpuFpReferenceSdpa` (forward and backward), `CpuFpReferenceSdpaRagged`, `GpuRefSdpaFwd` and `GpuRefSdpaRaggedFwd` mask `skv <= sq + off - L`. - **Frontend:** `detail::validateSdpaLeftBound` in `SdpaFwdNode` / `SdpaBwdNode` rejects `left_bound = 0` and values below -1. Doxygen on `set_diagonal_band_left_bound` and `set_sliding_window_length` states the cuDNN meaning and the FlashAttention mapping. - **Graph plans:** the CPU executor's `extractDiagonalBandParams` and the GPU `GpuSdpaRaggedFwdPlan` reject 0. - **hip-kernel-provider:** `plan_utils::resolveDiagonalBand` is the single conversion point; `DiagonalBand::leftBound` holds the kernels' FlashAttention window (`L - 1`) for asm/AITER forward and backward, hip_flash2 and the gfx950 dense pack (a window that exactly covers the sequence is now correctly classified as causal). `Gfx1151WmmaAttentionNative::maskFor` (prefill and decode) converts the same way and declines 0. Kernels and rocKE keep their own conventions. - **Golden generator:** `generate_sdpa_fwd_golden.py` gains `--left-bound L` (graph / cuDNN convention, written unchanged). `--window-left W` keeps FlashAttention's convention (written as `left_bound = W + 1`) for `--validate` against AITER. The two are mutually exclusive; the README records the conversion. No checked-in bundle uses a left bound, so no golden data changes. - **IngestorGenerator shape miner:** the graph's `left_bound` and rocKE's `sliding_window` count keys the same way, so the value passes through unchanged (was `left + 1`), and 0 is refused. - **Tests added since #14287:** the gfx1151 decode-engine window tests (#13897) use `left_bound = 1024` for their 1024-key window; a heuristics test no longer builds a `left_bound = 0` graph; the CPU and GPU plans' rejection of 0 is tested directly. An audit of every other file on `develop` that mentions `left_bound` found no other consumer of the raw value: the remaining plans and engines take the band already converted by `resolveDiagonalBand` / `extractDiagonalBandParams`, and the rest carry the field through (descriptors, packers, schema, Python bindings, corpus tools). ## Behavior change and migration - A graph with `left_bound = L` now attends to one fewer key than before. Callers that passed FlashAttention-style values straight into `left_bound` must pass `W + 1`. Out-of-tree providers that read the graph's `left_bound` must convert to their kernel's convention (FlashAttention-style kernels take `L - 1`). - `left_bound = 0` and values below -1 are rejected; -1 still means unbounded. (cuDNN's frontend rejects a forward `left_bound <= 0` only on backends before 9.10 and a backward one always; hipDNN rejects 0 in both directions, because a provider's `L - 1` would turn 0 into -1, which kernels read as unbounded.) - The serialized field is unchanged. SDPA was off by default in every hipDNN build in TheRock until less than a week ago, so no released build has users of the old meaning; whether to bump the `flatbuffers_sdk` / `frontend` versions is being discussed separately and is not part of this PR. ## Risk Assessment Low. It is an intentional behavior change to a public SDPA attribute, but SDPA was off by default in every hipDNN build in TheRock until less than a week ago, so no released build has users relying on the old meaning. The in-tree consumers all change together in this PR. | Risk | Mitigation | |---|---| | A provider left on the old convention silently attends to one extra key | References and every in-tree provider mapping change together in this PR, so the engine-vs-reference tests check them against each other (a provider on the old convention fails them). Conversion is centralized in `resolveDiagonalBand` (and gfx1151's `maskFor`), each with tests that pin `L -> L - 1`, `1 ->` diagonal only, `0 ->` declined. | | The reference and its tests share the misreading again | `LeftBoundKeepsLKeysIncludingDiagonalLikeCudnn` pins the cuDNN rule by hand; the generator's mask is compared against an independent mask built from cudnn-frontend's `fp16_ref.py` rule (top-left, bottom-right, `S_q != S_kv`). | | Out-of-tree providers or serialized graphs written to the old meaning | Migration note above; Doxygen and the generator README state the convention; versioning decision tracked separately. | | Tools and generators mixing the two conventions | Each generator flag is named after the convention it uses (`--left-bound` vs `--window-left`); the shape miner and rocKE agree on counting the diagonal. | | Code that landed on `develop` after #14287 still uses the old values | Audited every `left_bound` consumer on current `develop`; the gfx1151 decode tests (#13897) were the only ones affected and are updated. | | Work in flight assumes the old meaning | #14358 (SDPA reference implementations) assumes the cuDNN meaning and stacks on this PR; the FlyDSL attention engines (#14111) are validated against these references, so they land after it. | ## ASIC Coverage The semantic change is arch-independent. Affected engines: asm/AITER and hip_flash2 (gfx942/gfx950) and the gfx1151 WMMA pack (gfx11/gfx12). The gfx950 dense pack declines windows and is unaffected. Validated on gfx942 (below); gfx950 and gfx1151 have no device run in this PR: their changes are the shared band resolution and `maskFor`, which the host-side tests cover. ## Testing New and updated tests: - `TestCpuFpReferenceSdpaFp64.LeftBoundKeepsLKeysIncludingDiagonalLikeCudnn` (L = 2, query 5 keeps {4, 5}; L = 1 keeps only the diagonal); existing sliding-window tests keep their hand-checked masks with `leftBound` raised by one. - `TestSdpaDiagonalBandParams.RejectsZeroLeftBound` (CPU plans) and a `left_bound = 0` case in `TestGpuSdpaRaggedFwdPlanBuilder.BuildRejectsInvalidMaskAttributes` (GPU plan). - hip-kernel-provider: `TestSdpaFwdResolveDiagonalBand`, `LeftBoundZero` / `LeftBoundOneIsDiagonalOnly` mask cases, covering-window boundaries (backward 64, hip_flash2 256 vs 255), gfx1151 binding (7 binds as 6, 1 as 0, 0 declined) and the decode-engine window cases. - `test_generate_sdpa_fwd_golden.py`: the generator's mask against cudnn-frontend's rule; `--left-bound 3` and `--window-left 2` give the same graph; invalid flag combinations are rejected. Results, gfx942 (MI300X), this branch built with `-Werror`, `HIPDNN_ENABLE_SDPA=ON`, `HIPDNN_ENABLE_KERNEL_INGESTOR=ON`: | Suite | Passed / skipped | |---|---| | hipdnn_test_sdk_tests | 2423 / 1 | | hipdnn_frontend_tests | 2222 / 0 | | hipdnn_public_frontend_tests | 490 / 0 | | hipdnn_plugin_sdk_tests | 1293 / 2 | | hipdnn_flatbuffers_sdk_tests | 269 / 0 | | hipdnn_integration_tests_unit_tests | 1109 / 0 | | hipdnn_gpu_ref_tests | 13772 / 0 | | hipdnn_golden_data_tests | 66 / 0 | | hip_kernel_provider_tests (`*Sdpa*:*Flash2*:*Gfx950Attention*:*Gfx1151WmmaAttention*`) | 420 / 1 (gfx950-only) | | hip_kernel_provider_integration_tests (same filter; asm forward/backward incl. backward window cases) | 45 / 21 (gfx950-only, and causal+stats graphs the asm forward builder declines by design) | | integration-tests quick CTest | 15 / 15 | | golden generator pytest, with PyTorch | 68 passed, 23 subtests | The CI CTest for the generator tests has no PyTorch in its environment and skips them; the PyTorch run above is what exercises the generator. ## Related - Tracking: ALMIOPEN-3024. Related earlier fix of the deprecated causal setters: #13065 (ALMIOPEN-2586). - Supersedes #14287 (Daming Feng). - Must land before #14358 (SDPA CPU and GPU references), which assumes the cuDNN meaning, and before the FlyDSL attention engines (#14111), which are validated against these references. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Daming Feng <dmfeng8898@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
brockhargreaves-amd
added a commit
that referenced
this pull request
Oct 29, 2026
…undles #14483 gave the SDPA left_bound the cuDNN meaning on develop (SEM-001): left_bound = L keeps the L keys ending at the diagonal, and 0 is illegal. The frontend, both references and ASM_SDPA_ENGINE moved with it. The generator still wrote develop's old form, left_bound = W - 1, so every window case described a window one key narrower than its id says, and the W = 1 cases wrote the now-illegal 0. WINDOW_SEMANTICS is now "after_sem001": the 117 window cases are regenerated with left_bound = W. Case ids, levels, tolerance classes and the TOML block are unchanged, and the claims still describe the same windows. The plan and the generator now cite #14483, which replaced the closed #14287, including in regression_of; the window-semantics test sets both meanings explicitly instead of assuming the default. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
brockhargreaves-amd
added a commit
that referenced
this pull request
Nov 2, 2026
…undles #14483 gave the SDPA left_bound the cuDNN meaning on develop (SEM-001): left_bound = L keeps the L keys ending at the diagonal, and 0 is illegal. The frontend, both references and ASM_SDPA_ENGINE moved with it. The generator still wrote develop's old form, left_bound = W - 1, so every window case described a window one key narrower than its id says, and the W = 1 cases wrote the now-illegal 0. WINDOW_SEMANTICS is now "after_sem001": the 117 window cases are regenerated with left_bound = W. Case ids, levels, tolerance classes and the TOML block are unchanged, and the claims still describe the same windows. The plan and the generator now cite #14483, which replaced the closed #14287, including in regression_of; the window-semantics test sets both meanings explicitly instead of assuming the default. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
left_bound = Lnow keeps the L keys ending at the diagonal, diagonal included, as in cuDNN. Previously hipDNN kept L + 1 (FlashAttention'swindow_size_leftconvention).L − 1) at the boundary.left_bound = 0is now rejected, as cuDNN does.-1still means unbounded.Description
cuDNN's left bound is a
CMP_GTdiagonal-band mask that keepscol + L > row(cudnn-frontendattn_score_modifiers.h::sliding_window_mask; referencetest/python/sdpa/fp16_ref.pymasksrel <= diag_offset − left_bound). hipDNN's references, provider mappings and golden generator all used FlashAttention's convention, so code written against the cuDNN API (set_sliding_window_length(L)/set_diagonal_band_left_bound(L)) attended to one extra key per query, forward and backward. Example: L = 2, query 5, TOP_LEFT. cuDNN keeps keys {4, 5}; hipDNN kept {3, 4, 5}. The right bound already matched.Nothing caught it because every check shared hipDNN's convention: the golden generator built its own mask and cross-checked against AITER with the same value, the engine tests compared against our own CPU reference, and no golden bundle uses a left bound.
Changes:
CpuFpReferenceSdpa(forward and backward),CpuFpReferenceSdpaRagged,GpuRefSdpaFwdandGpuRefSdpaRaggedFwdnow maskskv <= sq + off − L.detail::validateSdpaLeftBoundinSdpaFwdNode/SdpaBwdNoderejectsleft_bound = 0and values below-1. Doxygen onset_diagonal_band_left_boundandset_sliding_window_lengthstates the cuDNN meaning and the FlashAttention mapping.extractDiagonalBandParamsandGpuSdpaRaggedFwdPlanalso reject0.plan_utils::resolveDiagonalBandis the single conversion point.DiagonalBand::leftBoundnow holds the kernels' FlashAttention window (L − 1) for asm/AITER forward and backward, hip_flash2 and the gfx950 dense pack. As a result, a window that exactly covers the sequence is now classified as plain causal correctly.Gfx1151WmmaAttentionNative::maskForreadsleft_bounddirectly, so it converts the same way and declines0.--window-leftstays FlashAttention-style (so--validateagainst AITER is unchanged), but the graph now carriesleft_bound = W + 1vialeft_bound_from_window_left. No checked-in bundle uses a left bound, so no golden data is regenerated.left_boundand rocKE'ssliding_windownow count keys the same way, so the value passes through unchanged (previouslyleft + 1) and0is refused.Behavior change: callers that passed FlashAttention-style values straight into
left_boundmust now passW + 1, andleft_bound = 0is rejected.Risk Assessment
Medium. This is an intentional behavior change to a public SDPA attribute. References and every provider mapping change together in this PR, so the engine-vs-reference tests keep checking them against each other: a provider left on the old convention would fail those tests.
ASIC Coverage
The semantic change is arch-independent. Affected engines: asm/AITER and hip_flash2 (gfx942/gfx950) and the gfx1151 WMMA pack. The gfx950 dense pack declines windows and is unaffected.
Test plan
New and updated tests:
TestCpuFpReferenceSdpaFp64.LeftBoundKeepsLKeysIncludingDiagonalLikeCudnn: L = 2, query 5 keeps {4, 5}; L = 1 keeps only the diagonal. The existing sliding-window tests keep their hand-checked masks withleftBoundraised by one.test_generate_sdpa_fwd_golden.py:TestGenerateSdpaFwdGoldenWindowMatchesCudnncomparesbuild_maskagainst an independent mask built from cudnn-frontend'sfp16_ref.pyrule (top-left, bottom-right, S_q ≠ S_kv).--window-left 2is written asleft_bound = 3.TestSdpaFwdResolveDiagonalBand(graph L becomes band L − 1) and newLeftBoundZero/LeftBoundOneIsDiagonalOnlymask cases.TestSdpaFwdNode/TestSdpaBwdNodePreValidateChecksLeftBound. The lowering tests now useleft_bound = 64instead of the now-invalid0.test_mine_shapes.py: graph inputs moved to the cuDNN meaning, plus a zero-left-bound refusal test.pre-commit(clang-format, black) passes on all changed files.Not yet run: the local build and the tests above, including the engine-vs-reference SDPA sliding-window tests on gfx942. CI will cover them.
Tracking
JIRA ID: ALMIOPEN-3024
🤖 Generated with Claude Code