Repository navigation
fix(hip-kernel-provider): ASM SDPA forward declines unaddressable layouts - #14686
Open
brockhargreaves-amd wants to merge 2 commits into
Open
brockhargreaves-amd wants to merge 2 commits into
brockhargreaves-amd wants to merge 2 commits into
Conversation
…outs SdpaFwdPlanBuilder::isApplicable accepted Q/K/V/O and stats (LSE) layouts that the forward kernels address wrongly. Its only stride check was that byte strides fit in 32 bits. On gfx942 (MI300X) each kind of layout below gave a silently wrong O or wrong stats instead of a decline, in the cases that were run (the exceptions are noted). The evidence comes from two sources. One is GPU runs of the develop CI plugin, which accepts all of these layouts, compared with the CPU reference. The other is disassembly of the 14 forward .co files. Q/K/V/O strides. The forward argument block carries only batch, head and sequence byte strides, and the kernels read or write each row as D contiguous elements. isApplicable now declines: - A head-dim (innermost) stride other than 1. The kernels read it as 1. Stride-2 Q, K or V gave a wrong O that changed between runs, because the kernels read the gaps between elements. A stride-2 O left half of O unwritten. Q stored as [B, H, D, S] and hd192 Q with stride 2 also failed. - A sequence stride below the tensor's own head dim, also 0 and also on a length-1 sequence axis. Each buffer ends S * seq stride bytes into its (batch, head) slice, so the end of the last row reads as 0 and writes there are dropped. A seq stride of D/2 gave a wrong O for Q and V, and for K at S_kv = 2. At S_kv = 256 the K case stayed within the bf16 tolerance, but its error was about 6x the packed run's. With S_q = 1 and an O seq stride of 1, O was left NaN. With S_kv = 1 and a V seq stride of 1, O was 0. K and V with seq stride 0 gave O and stats off by up to 1.0 and 1.3, Q with seq stride 0 gave a wrong O and stats, and O with seq stride 0 was left unwritten. With S_kv = 1 a K seq stride of 1 still gave the right O, since the one key takes all the softmax weight; by the address formula its score, and so the stats, are wrong. Each tensor is compared with its own head dim. That relies on K having Q's head dim and O being [B, H_q, S_q, D_v], because the kernels size K rows by Q's head dim and O's buffer by S_q. The forward isApplicable does not check those dims yet; a forward counterpart of the backward's mismatched-dims decline is a separate change. - A zero O batch or head stride on an axis longer than 1. Each (batch, head) slice of O is written by its own workgroups, so such slices overwrite each other: with an O batch or head stride of 0, half or two thirds of O differed from the packed run, and O changed between some runs with identical inputs. Nonzero strides that make O overlap itself are not checked. On gfx942, O with equal batch and head strides (B = H = 2), or with a head stride of half a slice, differed from the packed run in a quarter of its elements, with both plugins. As in the backward, only a zero stride is checked: a zero stride is how a broadcast is written, so O gets one when it is given a broadcast input's strides, while overlapping nonzero strides have to be built by hand. PyTorch's check for tensors that an operation writes to also rejects only a zero stride on an axis longer than 1. Stats do get an overlap check (below), because their address formula leaves a single comparison to make; for O it would take a footprint check over three axes in any order, which would fit better in hipDNN's graph validation, where every engine gets it. The zero O batch stride check assumes batch mode, the only mode with forward kernels. In group (ragged) mode the kernels locate each batch by its sequence offsets and get no O batch stride, so when group-mode forward kernels are added, a ragged O must be exempt from the batch-axis part of this check. Stats. Every forward kernel writes the LSE of (b, h, s) at ((b * H_q + h) * lse_Hs + s) * 4 bytes, with lse_Hs the stats head stride, and takes no stats batch or sequence stride. isApplicable now declines stats whose seq stride is not 1 when S_q > 1, or whose batch stride is not H_q times the head stride when B > 1. It also declines a head stride below S_q when B * H_q > 1, with which (batch, head) slices overlap. On gfx942 each of these gave wrong or NaN stats while O was correct: - a batch stride padded to H_q * S_q + 128, with hd128, hd192 and GQA; - a seq stride of 2; - [B, S_q, H] storage, and H_q = 1 with strides [S_q, 1, 1, 1]; - [H, B, S_q] storage, which by the kernels' address formula also writes 2 KiB past the end of the stats buffer. This is looser than the backward's packed-only stats rule on purpose. The backward kernels ignore the stats strides, but the forward kernels use lse_Hs: a head stride padded to S_q + 64, with a batch stride of H_q times that, gave correct stats on gfx942 and stays accepted. The stride of a size-1 stats batch or sequence axis is not checked. A size-1 head axis is, on purpose: buildPlan passes the head stride as lse_Hs, so with H_q = 1 the kernel steps between batches by the head stride. Passing the batch stride instead when H_q = 1 would also accept packed [B, 1, S_q] stats with any head stride, such as the H_q = 1 case above; they are rare and decline. The stats decline reasons name the stats dims and strides and Q's dims. These stay accepted, all GPU-verified with bf16 on gfx942: - BSHD on all tensors, and O BSHD with BHSD inputs; - seq strides D+2 and D+8; - odd strides: a seq stride of D+1 and odd head and batch strides on each of Q, K, V and O, with hd128 and hd192, gave O bitwise equal to the packed runs (the backward declines some of these); - hd192, GQA, bottom-right causal, tails, and S_q = 1 or S_kv = 1 packed; - a zero batch or head stride on Q, K or V (a broadcast): K and V with a zero batch or head stride, and Q with a zero batch stride, gave O bitwise equal to the packed runs and stats that matched the CPU reference. A zero stride on a length-1 O axis is accepted too, since it only ever multiplies index 0. The gfx950 kernels have the same argument block and the same LSE address formula (from their disassembly), but load rows with 16-byte LDS-DMA where the gfx942 kernels use 4-byte. On gfx950 (MI355X), this commit's plugin gave the BSHD, D+2, D+8 and odd-stride layouts above, with hd128 and hd192, the same mismatch counts and max and mean error against the CPU reference at atol=1e-9 as the packed layouts. The fp8 kernels also have the same argument block and LSE formula, so the same rules apply to them. They were not run; in particular odd strides, which put fp8 rows 1 or 3 bytes past a 4-byte boundary, are unverified with fp8. New tests in TestSdpaFwdPlanBuilder.cpp (gfx942 and gfx950, like the other forward isApplicable tests). Each decline arm has a case that only it declines, and each exemption has an accept case: - IsApplicableAcceptsAddressableLayouts - IsApplicableRejectsNonUnitHeadDimStride - IsApplicableRejectsShortRows, including hd192 Q and K rows long enough for V's and O's head dim but not their own - IsApplicableRejectsZeroStrideOutput - IsApplicableRejectsUnaddressableStats The zero-stride and overlapping layouts were run with a standalone hipDNN probe that drives ASM_SDPA_ENGINE on its own buffers, because the bundle harness's test tensors reject zero strides. Checked on gfx942 (MI300X). This commit's plugin replaced the one in a develop CI build whose ASM SDPA engine source matches this commit's parent, and results were compared with the CPU reference: - 84 forward layout probes. The 25 that this commit declines now skip, each with a reason that names the tensor, its dims and its strides. With the develop plugin, 21 of them gave a wrong O or wrong stats. Two could not be checked there: the harness rejects a zero stride (the standalone probe covers it), and the CPU reference rejects rank-3 stats. The last two passed but are wrong in ways the check missed: the K seq stride D/2 case stays within the bf16 tolerance, and with S_kv = 1 a K seq stride of 1 only gives the right O because one key takes all the softmax weight. - The other 59 give the same results as with the develop plugin, down to the mismatch counts and the max and mean error at atol=1e-9. 47 pass; they cover every layout named above as staying accepted except the broadcasts, which the next item covers. 3 were already declined. 9 still fail: non-positive or denormal scales, which are left to a separate change, and rank-3 stats, which the CPU reference cannot check. - On the standalone probe, K/V and Q broadcasts still match the CPU reference and are bitwise equal to their packed twins in each of 3 runs. An O batch or head stride of 0 and a Q, K/V or O seq stride of 0 now decline; with the develop plugin each gave a wrong O. The two overlapping O layouts are accepted by both plugins, with the same mismatch counts. - 19 probes with tensors of up to 4 GiB, which exercise the kernels' 32-bit address math, give the same results as before (10 of them still fail with both plugins; the 32-bit address math is a separate change). - The develop SdpaFwd and SdpaFwdRuntimeScale bundles (102 tests, quick to full) give the same results as before: 46 pass and 56 skip, with the same decline reasons. - The provider's own forward GPU integration tests (hip_kernel_provider_integration_tests, *IntegrationGpuSdpaFwd*: Smoke, SmokeFp8, SmokeStats, ShapeSweep, Gqa and AsymmetricSeq) pass against this commit's plugin: 29 pass, and 21 skip as gfx950-only or causal with stats. - The unit tests pass. Each of 33 mutants that removes, narrows or widens one arm of these checks, or compares a row stride with another tensor's head dim or a fixed 128, makes one of the new tests fail. On gfx950 the 84 forward layout probes ran with this commit's plugin only, since the develop CI build has no gfx950 kernels. The same 28 skip, and every probe that passes on gfx942 and has a gfx950 kernel passes. The forward unit tests pass there too. Rebased onto develop 98b504d, after #14075, whose range gate now runs first in isApplicable. The GPU results above are from develop 61c2675. On the new base, the four ALMIOPEN-3035 ASM SDPA fix branches (backward layouts, attention scale, a16 accumulator, forward layouts) were built and run together: - hip_kernel_provider_tests: 912 pass and 7 skip on gfx942 (MI325X), 886 pass and 33 skip on gfx950 (MI355X); none fail. - hip_kernel_provider_integration_tests: none fail on either. - develop's SDPA bundles on ASM_SDPA_ENGINE, run like the CI lane (claims enforced): 62 pass and 56 skip on gfx942, 15 pass and 103 skip on gfx950, and no support claim fails. A host-side run of the real isApplicable over these bundles finds no outcome change from develop. - No develop or #14622 bundle case changes outcome (host-checked for this branch alone). The new forward tests pass on gfx942 and gfx950. - clang-tidy 20 and clang-format 18 are clean on the changed files. Co-Authored-By: Claude Opus 5.5 (1M context) <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. |
brockhargreaves-amd
changed the title
6 of 7 tasks
brockhargreaves-amd
marked this pull request as ready for review
October 30, 2026 19:26
CMiservaAMD
approved these changes
Nov 2, 2026
CMiservaAMD
left a comment
CMiservaAMD
left a comment
Contributor
There was a problem hiding this comment.
Found by AI. Severity: none blocking.
Thanks. The new rules match the kernels' address model as I read it.
- For Q, K, V and O: the head-dim stride must be 1, and the sequence stride must be at least the head dim. A zero batch or head stride on O is refused when that axis is longer than 1.
- For Stats: the sequence stride must be 1, the batch stride must equal H_q times the head stride (the multiply is overflow-checked), and the head stride must be at least S_q.
The bshd, bhsd and sbhd orders, padded and odd strides, GQA, and zero batch or head strides on Q, K and V all stay accepted.
Built and ran it on gfx942 (MI300A):
- the 5 new tests pass, and the forward plan-builder suites pass (78/78);
- two deliberate breaks were both caught:
- loosening the row-stride rule to
< 1failsIsApplicableRejectsShortRows, in 14 cases; - tightening the stats head-stride rule to
!= S_qfails only the padded-stats case inIsApplicableAcceptsAddressableLayouts, so the accepted-layout test also catches a rule that is too strict.
- loosening the row-stride rule to
It merges cleanly onto the current develop. Statically applying the rules to every SDPA forward bundle in the tree:
- none of the 12 bundles that claim ASM_SDPA_ENGINE would be declined, so there is no CLAIM_BROKEN;
- only two ragged+paged bundles would be declined, and ASM already declines those.
This also matches our forward test-plan runs on gfx942 and gfx950. The cases this now declines are the ones where we saw a wrong O or unwritten Stats: zero-stride Q and V on the sequence axis, non-packed Stats, self-overlapping rows, a non-unit head-dim stride, and a negative stride on an all-size-1 tensor.
A few suggestions follow inline.
Found by AI. Severity: suggestion.
The PR body could mention that some layouts which gave a correct O are now declined too:
- a zero K sequence stride with S_kv > 1, where O is right only by coincidence and Stats are wrong;
- some self-overlapping rows.
That makes the trade-off clear to anyone whose graph starts being declined.
| // (stride-0) inputs, and packed stats or stats with a padded head stride. The | ||
| // padded and odd strides were also verified on gfx950. The size-1-axis cases and | ||
| // rank-3 stats differ from them only in strides or dims no kernel reads. | ||
| TEST_F(TestSdpaFwdPlanBuilder, IsApplicableAcceptsAddressableLayouts) |
Contributor
There was a problem hiding this comment.
Found by AI. Severity: important.
This test now pins that BSHD, padded and odd strides, K/V broadcast, BSHD O with BHSD inputs, and padded stats head strides stay accepted. But nothing in the tree checks the numbers for those layouts:
- the forward GPU integration test builds only packed layouts;
- every claimed bundle is packed BHSD.
So a kernel update that breaks one of these accepted layouts would still pass CI. Suggest adding 2–3 numeric cases to the forward integration test, or bundles with ASM claims. For example:
- O BSHD with BHSD inputs;
- K/V batch stride 0 (allocatable since feat(hipdnn): allow zero (broadcast) strides in data SDK Tensor #14709);
- stats head stride S_q + 64 with batch stride H_q × (S_q + 64).
Contributor
Author
There was a problem hiding this comment.
Agreed—the applicability tests protect acceptance, but cannot catch wrong O/LSE from a kernel update. #14202 adds ragged BSHD coverage, but the dense cases you list remain a gap. I checked #14622: its ordinary backward bundles initialize O/LSE through the CPU reference, so they do not exercise ASM forward. We should add explicitly selected ASM forward bundles for BHSD inputs with BSHD O, broadcast K/V, and padded LSE, plus odd-stride coverage, with full output comparisons and claims for validated devices. The merge fix adds a ragged applicability regression; it does not close this numeric coverage gap.
| // check over three axes in any order, which would fit better in hipDNN's graph | ||
| // validation, where every engine gets it. The batch-axis part assumes batch | ||
| // mode, the only mode with forward kernels: in group mode the kernels locate | ||
| // each batch by its sequence offsets, not by O's batch stride. |
Contributor
There was a problem hiding this comment.
Found by AI. Severity: important.
O with nonzero strides that overlap itself (for example equal batch and head strides, or a head stride below S × seq stride) is still accepted and gives a wrong O. The comment and ALMIOPEN-3042 say so.
The check for O may be cheaper than "a footprint check over three axes in any order":
- sort the axes whose extent is greater than 1 by stride;
- require each stride to be at least the previous axis' extent × stride, with the row's D as the innermost extent.
Would you consider adding that here (and for dQ/dK/dV in #14684)? Or, if the gap stays, sould it be tracked by a ticket?
Contributor
Author
There was a problem hiding this comment.
Confirmed: equal nonzero batch/head strides can alias O and still pass these checks. A small conservative guard is feasible; the main choice is which layouts it may decline. The proposed previous-extent rule also rejects some disjoint layouts—for example dims [2,2,2,128], strides [640,384,128,1]. Accumulating the occupied span is less restrictive, though still conservative. I propose tracking general overlap validation separately with an explicit ticket and rejection tests, including the backward outputs after checking their addressing/accumulation contracts. Ragged batches need separate treatment because offsets replace the batch stride. dc932ac handles that distinction for the zero-batch-stride check only; general overlap remains open.
| // head), so such slices share memory and overwrite each other: an O batch or | ||
| // head stride of 0 left half or two thirds of O wrong, and O changed between | ||
| // some runs with identical inputs. | ||
| // Only a zero stride is checked, as in the backward: it is how a broadcast is |
Contributor
There was a problem hiding this comment.
Found by AI. Severity: suggestion.
The new comments (about FB:265-304 and 346-367, and SdpaFwdParams.hpp:80-82) are long and mix the address facts with history and alternatives:
- "On gfx942 each kind of layout declined here gave a wrong O in the cases run";
- "as in the backward", "which the backward declines on some axes", "Unlike in the backward". That code is only in fix(hip-kernel-provider): ASM SDPA backward declines layouts and dims its kernels address wrongly #14684, so if that PR does not land first, these comments are wrong;
- "would fit better in hipDNN's graph validation";
- "Passing the batch stride as lse_Hs when H_q = 1 would also accept …".
Suggest keeping the address formulas and the buffer-range constraint (8–10 lines per helper), and moving the evidence and the comparisons with the backward to the PR description.
Contributor
Author
There was a problem hiding this comment.
Agreed. The helper comments should keep the address formulas, buffer-range constraint, singleton-axis rules, and ragged exception. GPU probe history, backward comparisons, and alternative implementations belong in the PR description. #14684 is still open, so the unqualified backward comparisons are premature. This cleanup remains to be applied; dc932ac only updated the batch-mode explanation as part of the merge resolution.
| const hipdnn_flatbuffers_sdk::data_objects::TensorAttributes& o) | ||
| { | ||
| // NOLINTNEXTLINE(readability-identifier-naming) | ||
| static const char* HIP_KERNEL_LOG_PREFIX = "[SdpaFwdPlanBuilder::isApplicable] "; |
Contributor
There was a problem hiding this comment.
Found by AI. Severity: nit.
HIP_KERNEL_LOG_PREFIXis redefined in each new helper (FB:312, FB:373). A file-scopeconstexprwould serve all three users.- The tests hand-compute strides (TF:1790-1803, 1956, 2006).
generateStrides(dims, order)from the data SDK'sShapeUtilities.hppwould make the BSHD/BHSD intent explicit. - The decline tests assert only
false, not the reason. Each case is one change from an accepted graph, so they are safe today, but a comment or a log-capture assert would keep that from eroding. - The
SdpaFwdParams.hpp:80-82comment describesisApplicable's behaviour in another file. Suggest stating only the kernel fact: the kernel steps between batches by numHeadsQ × s_lse_Hs.
Contributor
Author
There was a problem hiding this comment.
Agreed on sharing the logging prefix and using generateStrides for packed BHSD/BSHD layouts. Explicit strides are still useful for the deliberately padded, odd, broadcast, and invalid cases. For the decline tests, I favor documenting the accepted baseline and single changed condition rather than coupling them to log wording. The SdpaFwdParams comment should state only the kernel fact: batches advance by numHeadsQ * s_lse_Hs. These cleanup items remain open after the merge fix.
Preserve both the ragged addressing validation and the forward IO layout guards. Ignore the output batch stride in group mode, where sequence offsets address batches, while keeping the other output layout checks. Add regression coverage for dense versus ragged zero output batch strides and for invalid ragged head, innermost, and sequence strides. Validation: pre-commit passed for all three PR files; affected C++ sources and tests compiled with -Werror. Host-only applicability runner passed 35 tests with gfx942 device properties and 31 with gfx950 properties (4 existing fp8 skips). GPU execution was not available.
10 of 13 tasks
15 of 16 tasks
brockhargreaves-amd
added a commit
that referenced
this pull request
Nov 5, 2026
…rated bundles (#14622) ## Summary This PR adds the SDPA backward test plan required by RFC 0022 (Per-Op Test Plans, merged in #13937). It also adds a generator that writes the SdpaBackward bundles from the plan, and the bundles it writes. The 16 inherited SdpaBackward cases (all B = H = 1, packed BHSD, fixed scale) are replaced by 503 generated cases in four levels. Each case carries a purpose and traces to the plan items it covers. > Written with AI assistance (Claude Code). The plan follows the merged RFC 0022; it records one remaining deviation (DV-4) for reviewers to approve. JIRA ID : ALMIOPEN-2894 ## Risk Assessment Risk 2 of 5. The PR changes test data, test tooling, test configuration and documentation only; no library or provider code changes. The changes that reach beyond the SdpaBackward bundles: - `support_matrix_units.collect_sweep_units()` leaves out cases whose `metadata.purpose` is `probe`, as RFC 0022 §9 requires. Today only SdpaBackward cases carry a `purpose`, so other ops' matrices are unchanged. The committed `SUPPORT_MATRIX_OVERVIEW.md` is re-rendered. - `ASM_SDPA_ENGINE.toml`: two dtype-only SdpaBackward tolerance rows are replaced by 12 generated rows (seven tolerance classes), plus temporary skips for the open engine defects, each naming its ticket. - `ASM_SDPA_ENGINE_test_categories_integration.yaml`: six rejection probes that ASM_SDPA_ENGINE wrongly accepts are quarantined with their tickets (RFC 0022 §6). Both files affect the ASM lane only. - #14358's eight PyTorch golden cases move, unchanged, from `quick/SdpaBackward/{Default,AdditiveMask}/` to `quick/SdpaBackward/Golden/{Default,AdditiveMask}/`, so the generator owns the `Default` sweeps. They keep their fp16 tolerance row. `bhsd/` and the `*_fwd_pair` bundles are untouched. The main risk is CI noise from new cases. Every level has been run on gfx942 and gfx950 with no failure (see Testing Checklist). ## ASIC Coverage The bundles run in every provider lane whose tier YAML matches `*Sdpa*`. Only `ASM_SDPA_ENGINE` accepts SdpaBackward graphs today, and only on gfx942: its backward plan builder declines every other device ("Device string does not match gfx942"). Claims are harvested for gfx942 only. PR CI tests gfx942. On gfx950 every generated backward case is declined, so a gfx950 run only shows that nothing fails; one is recorded below. PR CI on gfx942 is sufficient. ## Testing Summary - **Generator tests** (`hipdnn_sdpa_bwd_bundle_generator_python_tests`, label `quick`, 19 tests). They fail when the checked-in sweeps or the TOML block drift from the generator. They also check legality, all-pairs coverage up to nightly, the per-case ceilings and cumulative budgets of each level, two-way traceability between plan items and cases, that every TOML skip still matches a written case, and that the tier-YAML quarantine holds only rejection probes. - **`generate_sdpa_bwd_bundles.py --dry-run --report`.** Reports counts, budgets, the all-pairs proof, legality, and each declared-but-gated case with its gate. - **Claims run** (2026-10-26, MI300X, gfx942). All generated bundles ran on `ASM_SDPA_ENGINE`, configured like CI. `harvest_support_claims.py` turned the passing cases into the 272 claims. Every claimed case passes its tolerance class with a margin of at least 2.4 (median 4.2). - **Measured run, every level on GPU** (2026-11-04, plan §4.5). `hipdnn_integration_tests` ran on `ASM_SDPA_ENGINE` configured like the CI lane: `--test-engine` (claims enforced), the PR's `ASM_SDPA_ENGINE.toml`, the tier YAML's quarantine as a GTest filter, device input fill and the GPU reference as oracle. It ran all four levels, on a fresh MI300X (gfx942) and an MI355X (gfx950) node, built from this branch. The bundles, TOML and YAML are identical to `6e014ad50e0`. - **PR CI** runs the quick and standard levels on gfx942. ## Testing Checklist - [x] Generator tests - CTest `hipdnn_sdpa_bwd_bundle_generator_python_tests` - 19 of 19 - Status: Passed - [x] Plan report - `python3 reference-data-scripts/generate_sdpa_bwd_bundles.py --dry-run --report` - 503 written, 77 gated, 0 legality violations, all-pairs 669 of 669 - Status: Passed - [x] Bundle and support-matrix checks - `verify_support_claims.py`, `render_support_matrix.py --output -`, `feature_matrix.py --check`; the bundle, claim, render and support-feature pytest suites - Status: Passed (`verify_golden_bundles.py` reports no SdpaBackward error; its 322 errors in other ops' bundles are on develop too) - [x] pre-commit on the changed files - Status: Passed - [x] Claims run - `hipdnn_integration_tests`, `ASM_SDPA_ENGINE`, MI300X (gfx942), 2026-10-26 - Status: Passed - [x] Measured run, gfx942 - `hipdnn_integration_tests --test-engine ASM_SDPA_ENGINE --test-config ASM_SDPA_ENGINE.toml '--gtest_filter=*SdpaBackward*-<the tier YAML quarantine>'` - MI300X, Slurm job 68275259 - Status: Passed. 553 tests in 43.3 s, 0 failed, no claim broken: - 289 executed and passed against the GPU reference, and 15 expected rejections rejected; - 226 declined by ASM, and 23 hit the DEF skips; - per level: quick 2.4 s, PR 2.8 s, nightly 16.5 s, full 21.4 s. - [x] Quarantine check, gfx942 - the six quarantined cases run alone - Status: all six fail as expected ("Engine ASM_SDPA_ENGINE accepted expected-rejection bundle"), which shows the quarantine is needed. - [x] Every level, gfx950 - same command - MI355X, Slurm job 68275260 - Status: Passed. 0 failed: ASM declines every backward graph on gfx950, and the rejection probes, quarantined ones included, are correctly rejected. - [x] The other SDPA bundles, same lane - `--gtest_filter=*Sdpa*:-*SdpaBackward*` - Status: Passed. gfx942: 47 passed, 84 skipped. gfx950: 16 passed, 115 skipped. 0 failed on either. - [ ] PR CI on gfx942 - TheRock multi-arch CI - Status: Pending on `6e014ad50e0`. The 2026-11-02 run failed one case, `prbr01`, which is now gated. Later runs failed only on the hipCCL configure break from #14976, which develop reverted in #15089 (merged into this branch). ## Technical Changes ### What this adds - **The plan:** `dnn-providers/integration-tests/docs/test-plans/sdpa-bwd.md`. It follows the RFC 0022 template: the operation, the feature inventory, the shape catalog, building the sweep (levels, budgets, probes, regressions, trade-off register), the golden reference, pass criteria, and exhaustive mode. - **The generator:** `reference-data-scripts/sdpa_bwd_plan.py` holds the declarations: axes, legality, the shape catalog, level rules and the cost model, one record per case. `generate_sdpa_bwd_bundles.py` writes the sweeps, the support-claim files and the engine-TOML tolerance block from them. Nobody edits the bundles by hand. RFC 0022 open question 1 cites this module as the per-op Python option. - **The bundles:** 503 written cases, spread across the levels: | Level | Directory | Cases | |---|---|---| | quick | `quick/` | 56 | | PR | `standard/` | 152 | | nightly | `comprehensive/` | 220 | | full | `full/` | 75 | - Each case carries `purpose` (`claim`, `probe` or `regression`) and its plan items. - 77 more cases are declared but not written yet, each waiting on a named gate: an oracle extension, a harness feature or golden data. - One of the 77 is PRB-R01 (`dropout_probability = 0`). Since #14358 the references decline any graph that sets `dropout_probability`, so it waits on ALMIOPEN-3149 or ALMIOPEN-3150. - **Levels:** the cost model assigns each case to the lowest level whose per-case limits it fits: | Level | Per-case time | Per-case bytes | Cumulative budget | |---|---|---|---| | quick | 1.5 s | 4 MiB | 20 s | | PR | 5 s | 16 MiB | 120 s | | nightly | 75 s | 64 MiB | 480 s | | full | 300 s | no limit | 1,800 s | Each cumulative budget covers that level and every level below it, and keeps 5 % in reserve. The budgets come from a CPU-reference cost model; with the GPU reference, the measured run above is far below them. The ASM lane YAML picks the cases up by GTest prefix (`quick_*Sdpa*`, `standard_*Sdpa*` and so on), and each level also runs every level below it. - **Rejection probes** (21) carry `metadata.expected_rejection` (#14346): the harness never executes them, and an engine that accepts one fails the lane. PRB-M02 and PRB-M02f (`right_bound = -2`) are not written, because the frontend rejects them since #14358; `TestSdpaBwdNode.PreValidateChecksDiagonalBandBounds` covers that rule. - **dBias:** the five dBias probes and the live GL-14 case are written, now that #14358's references model dBias. To keep PR and nightly inside their reserve, the plan's trim rule moved M07.pr to nightly and M26.n to full. - **Tolerance classes:** seven classes, written as 12 `[[tolerance_overrides]]` rows between generated BEGIN and END markers in `ASM_SDPA_ENGINE.toml`. - **Support claims:** 272 claims for `ASM_SDPA_ENGINE` on gfx942. - **Probes left out of the support matrices** (RFC 0022 §9). This is the `support_matrix_units` change above. - **Generator file I/O is UTF-8**, so its tests pass on Windows. ### Temporary ASM_SDPA_ENGINE exclusions These stay until each fix lands; the PR with the fix deletes its entries. **Open engine defects** (`[[test_skips]]` in `ASM_SDPA_ENGINE.toml`). These are valid graphs that ASM accepts and computes wrong. Other engines still run them. | Cases | What goes wrong | Ticket | Fix | |---|---|---|---| | DEF-01 | head-dim stride other than 1 | ALMIOPEN-3036 | #14684 | | DEF-02 | non-packed Stats | ALMIOPEN-3037 | #14684 | | DEF-04 | Hk = Hq > Hv | ALMIOPEN-3039 | #14684 | | DEF-05 and H61 | odd element strides | ALMIOPEN-3038 | #14684 | | DEF-06 | scale of 0 or below | ALMIOPEN-3043 | #14688 | | DEF-03 | fp16 dV precision in the AITER kernel | ALMIOPEN-3046 | none in hipDNN (kernel) | **Quarantined rejection probes** (`_common_excludes` in `ASM_SDPA_ENGINE_test_categories_integration.yaml`, as RFC 0022 §6 asks). These are undefined graphs ASM should decline but accepts. | Cases | What ASM accepts | Ticket | Fix | |---|---|---|---| | prbu01, prbu03 | O or dQ dims that differ from Q's | ALMIOPEN-3039 | #14684 | | prbu13 | a dQ sequence stride below D (overlapping rows) | ALMIOPEN-3038 | #14684 | | prbt04, prbt04n | an O dtype that differs from Q's | ALMIOPEN-3050 | none yet | | prbr05 | `dropout_scale_inv` without a dropout mask | ALMIOPEN-3051 | none yet | ### Dependencies (all merged) - #13937, RFC 0022 (2026-10-30). Its first-phase documents, the methodology guide and the `docs/test-plans/` index, are not on develop yet, so the plan links the RFC's sections directly. - #14312: O and Stats derived from the forward of the same inputs. - #14483: `left_bound` uses cuDNN's meaning. The bundles write `left_bound = W`. - #14358 (2026-11-02): SDPA references cover the full graph API, and the GPU reference is the first oracle in `auto` mode. It also adds frontend rule R14 (`right_bound >= -1`). - #14346 (2026-11-03): the `expected_rejection` and `no_*_reference` bundle flags. - #14709 (2026-11-02): zero strides in the data SDK `Tensor`. ### Interaction with other open PRs - **#14684, #14686, #14687, #14688** (ASM declines, ALMIOPEN-3035). Each deletes its own entries: TOML skips for the DEF cases, and tier-YAML lines for the quarantined probes it fixes (#14684: prbu01, prbu03, prbu13). Whichever of these and this PR lands second removes the entries. - **#14999** (SDPA forward plan bundles, ALMIOPEN-2893). Both PRs edit two files the same way: - `ASM_SDPA_ENGINE.toml`: keep this PR's generated block and put #14999's `SdpaFwdPlan` skips after the `# END SdpaBackward tolerance classes` line. The generator test only checks the BEGIN to END span. The filters do not overlap (`SdpaBackward` vs `SdpaFwdPlan`). - `SUPPORT_MATRIX_OVERVIEW.md`: re-render it; do not hand-merge. The two PRs also add lines near each other in `integration-tests/CMakeLists.txt` and `test_categories_external.yaml`; these auto-merge. ### Before review - [x] `prbr01` (`dropout_probability = 0`) gated; ALMIOPEN-3149 and ALMIOPEN-3150 track the decision. - [x] Rejection probes carry `expected_rejection`; the six ASM accepts are quarantined in the ASM tier YAML with their tickets. - [x] `prbm02` and `prbm02f` removed; the frontend unit test of R14 covers them. - [x] The five dBias probes and GL-14 written; REG-08 (rank-3 Stats) still waits for the derivation extension. - [x] The plan refreshed against the merged RFC: DV-1 and DV-2 now agree with it, DV-3 is an RFC §11 gap, and only DV-4 remains a deviation. - [x] The measured run (plan §4.5) recorded above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
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.
Motivation
ASM_SDPA_ENGINEforward accepts Q/K/V/O and stats layouts that its kernels address wrongly. It returns a wrong O or wrong stats instead of declining, and some of the results change from run to run. Tickets ALMIOPEN-3041 and ALMIOPEN-3042, epic ALMIOPEN-3035. Both reproduce on gfx942 and gfx950.Technical Details
isApplicablenow declines:H_q * head stride, head stride at leastS_q.Further notes:
SdpaFwdPlanBuilder.cpp; the second to land takes the union.ASIC Coverage
Forward
isApplicableaccepts gfx942 and gfx950, the two architectures the ASM forward kernels are built for, and the new checks apply on both. TheRock PR CI tests gfx942 only: on gfx94X-dcgpu it runs the unit tests, the provider integration tests and the ASM SDPA bundle lane with claims enforced. It does not test gfx950, so gfx950 is covered by the run of this branch alone under Test Result.Test Plan
Unit tests and provider integration tests on gfx942 and gfx950; develop's SDPA bundles on
ASM_SDPA_ENGINEon both.Test Result
This branch alone at
eff2d333346, built with thehip-kernel-providerpreset (-DGPU_TARGETS=gfx950 -DHIPDNN_ENABLE_SDPA=ON) and run on one MI355X (gfx950):hip_kernel_provider_tests --gtest_filter='*-*DISABLED*:Full*'hip_kernel_provider_integration_tests --gtest_filter='*Sdpa*-*DISABLED*'ctest -R '^hip-kernel-provider-asm-sdpa-external-integration_standard_suite$'(quick and standard)ctest -R '^hip-kernel-provider-asm-sdpa-external-integration_full_suite$'(all tiers)No support claim names
ASM_SDPA_ENGINEon gfx950, so on gfx950 the bundle lane enforces no claim, and its result is the counts above.Combined build on develop
98b504d92c5(after #14075; supplementary). The four ALMIOPEN-3035 ASM fix PRs (#14684, #14688, #14687, this PR) were built together for gfx942 and gfx950 and run on one MI325X (gfx942) and one MI355X (gfx950). Each branch was also checked on its own with a host-side runner of the realisApplicableand unit tests.hip_kernel_provider_testship_kernel_provider_integration_testsASM_SDPA_ENGINElane config, claims enforced138218c0755), its temporary skips removedclang-tidy 20 (
--warnings-as-errors='*') and clang-format 18 are clean on the changed files.IsApplicableAcceptsAddressableLayouts,IsApplicableRejectsNonUnitHeadDimStride,IsApplicableRejectsShortRows,IsApplicableRejectsZeroStrideOutputandIsApplicableRejectsUnaddressableStats.61c26758398.Submission Checklist
🤖 Generated with Claude Code