Repository navigation
[Docs] Elaboration for composite type and storage - #1004
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds new language-level documentation for composite types and storage/allocator semantics, and restructures the test suite to act as conformance checks for those specs (running in the existing frontend-only harness).
Changes:
- Added two new language spec pages:
Composite typesandStorage and allocator, and linked them into the Sphinx TOC and docs index. - Replaced the prior unit-test coverage for struct/storage/shared-allocator behavior with new
tests/language/*conformance tests aligned to the docs. - Made small API-facing message/coercion tweaks in the DSL implementation (Vector coercion; error-message wording updates from “schema” → “type”).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_struct.py | Removed legacy unit tests (superseded by language conformance tests). |
| tests/unit/test_shared_allocator.py | Removed legacy unit tests (superseded by language conformance tests). |
| tests/language/test_storage_and_allocator.py | New conformance tests covering Storage, Storable layout, and allocator semantics. |
| tests/language/test_composite_types.py | New conformance tests covering struct/union declarations, nesting, and protocol closure. |
| tests/language/lang_utils.py | Added launch_ir() helper for kernel tracing via a JIT wrapper. |
| python/flydsl/expr/typing.py | Updated Constexpr error wording; added Vector.__coerce__ to support value coercion through schema/field construction. |
| python/flydsl/expr/struct.py | Error-message wording updates (“schema” → “type”, “inline schema” → “inline type”). |
| docs/sphinx/_toc.yml.in | Added new language docs pages to Sphinx TOC. |
| docs/language/storage_and_allocator.md | New language spec page for Storage + allocators. |
| docs/language/composite_types.md | New language spec page for composite types. |
| docs/index.rst | Linked the new language docs pages from the main index. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sjfeng1999
force-pushed
the
pr/docs-composite-storage
branch
from
August 12, 2026 15:51
358cc19 to
651506e
Compare
sjfeng1999
force-pushed
the
pr/docs-composite-storage
branch
from
August 12, 2026 15:54
651506e to
50c2046
Compare
coderfeli
approved these changes
Aug 14, 2026
JohnQin-llm
added a commit
that referenced
this pull request
Aug 19, 2026
#919 made `_init_dualwave_thread_mapping` re-derive (head, q_block) from the linear workgroup id with the head as the slow axis, so a head's blocks stay on one XCD and its K/V stays resident in that XCD's L2 across the concurrent wavefronts. It applied to every qualifying dualwave dense shape, bf16 included, gated only on `not SPLITK and not CAUSAL and NUM_HEADS_Q % NUM_XCD_GFX950 == 0`. #950 put a trait in front of that condition: - if const_expr(not traits.SPLITK and not traits.CAUSAL and ...): + if const_expr(traits.XCD_SWIZZLE and not traits.SPLITK and ...): `XCD_SWIZZLE` defaults to False and only the fp8 builder passes it. That PR measured and described fp8 only, and never touched the bf16 builder -- the string `xcd` does not appear in flash_attn_gfx950.py at all -- so the remap silently stopped applying to bf16. No test covered the mapping, so nothing caught it. Bisecting upstream on MI355X, B=2 H=32 D=128 bf16 non-causal, TFLOP/s at S = 61,380 / 180,180 / 239,580, aiter measured in the same process as control: cf1db2f 7/31 #919 1273.2 1287.0 1287.4 772f070 8/6 #974 1253.7 1074.5 1053.4 494b2cb 8/9 #945 1251.9 1073.3 1052.6 f6d0a41 8/14 #991 1247.2 1076.2 1061.2 47009a3 8/14 #1004 1248.2 1076.2 1061.3 daee90e 8/17 #930 1249.0 1076.4 1060.6 96af91b 8/17 #1014 1247.2 1076.6 1064.5 4521c8c 8/18 #960 1247.8 1074.8 1061.2 aiter ~1250 ~1253 ~1252 One step, between 7/31 and 8/6, and #950 (8/3) is the only commit in that window that touches the mapping. Everything after it is flat to within noise. This plumbs `xcd_swizzle` through the bf16 builder and reuses the existing dispatch condition (non-causal, `H % NUM_XCD_GFX950 == 0`, `num_q_blocks >= MIN_Q_BLOCKS_XCD_SWIZZLE`). Split-K is excluded structurally: it takes the `_build_splitk` branch above this dispatch site. The remap is bijective, so output is bit-identical. Two tests assert that over H=8/16/32/64 and cover the `H % 8 != 0` fallback both forced and auto -- the guard #919 shipped without. One loose end: the restored figure is still ~1.4% below the 7/31 measurement (1268.8 against 1287.4 at S=239,580). The candidates are the `MIN_Q_BLOCKS_XCD_SWIZZLE` threshold #950 introduced for fp8, which #919 did not have, and the LLVM bump in #945. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 tasks done
JohnQin-llm
added a commit
that referenced
this pull request
Aug 19, 2026
#919 made `_init_dualwave_thread_mapping` re-derive (head, q_block) from the linear workgroup id with the head as the slow axis, so a head's blocks stay on one XCD and its K/V stays resident in that XCD's L2 across the concurrent wavefronts. It applied to every qualifying dualwave dense shape, bf16 included, gated only on `not SPLITK and not CAUSAL and NUM_HEADS_Q % NUM_XCD_GFX950 == 0`. #950 put a trait in front of that condition: - if const_expr(not traits.SPLITK and not traits.CAUSAL and ...): + if const_expr(traits.XCD_SWIZZLE and not traits.SPLITK and ...): `XCD_SWIZZLE` defaults to False and only the fp8 builder passes it. That PR measured and described fp8 only, and never touched the bf16 builder -- the string `xcd` does not appear in flash_attn_gfx950.py at all -- so the remap silently stopped applying to bf16. No test covered the mapping, so nothing caught it. Bisecting upstream on MI355X, B=2 H=32 D=128 bf16 non-causal, cache cold, TFLOP/s at S = 61,380 / 180,180 / 239,580, aiter in the same process as control: cf1db2f 7/31 #919 1284.6 1288.2 1286.8 772f070 8/6 #974 1256.7 1188.1 1184.3 494b2cb 8/9 #945 1255.6 1187.9 1187.2 f6d0a41 8/14 #991 1256.0 1182.8 1183.6 47009a3 8/14 #1004 1260.5 1188.6 1183.4 daee90e 8/17 #930 1254.1 1077.0 1060.9 96af91b 8/17 #1014 1251.8 1074.9 1060.6 4521c8c 8/18 #960 1249.3 1073.9 1059.0 aiter ~1252 ~1255 ~1255 Two independent steps. The first, between 7/31 and 8/6 (-8.0%), is #950 and is what this commit fixes. The second, between 47009a3 and daee90e (-10.4%), is #930 dropping the fastmath flag when it moved these kernels onto fx.maxnumf, which reaches the raw MLIR builder without passing through resolve_fastmath; that is a separate fix, filed separately. This plumbs `xcd_swizzle` through the bf16 builder and reuses the existing dispatch condition (non-causal, `H % NUM_XCD_GFX950 == 0`, `num_q_blocks >= MIN_Q_BLOCKS_XCD_SWIZZLE`). Split-K is excluded structurally: it takes the `_build_splitk` branch above this dispatch site. The remap is bijective, so output is bit-identical. Two tests assert that over H=8/16/32/64 and cover the `H % 8 != 0` fallback both forced and auto -- the guard #919 shipped without. One loose end: the restored figure is 1268.3 against cf1db2f's 1286.8, a 1.4% gap. That is #930's dropped fastmath, which costs 10.4% with the remap off but only 1.4% with it on -- the two interact. Fixing #930 separately closes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 tasks done
JohnQin-llm
added a commit
that referenced
this pull request
Aug 20, 2026
#919 made `_init_dualwave_thread_mapping` re-derive (head, q_block) from the linear workgroup id with the head as the slow axis, so a head's blocks stay on one XCD and its K/V stays resident in that XCD's L2 across the concurrent wavefronts. It applied to every qualifying dualwave dense shape, bf16 included, gated only on `not SPLITK and not CAUSAL and NUM_HEADS_Q % NUM_XCD_GFX950 == 0`. #950 put a trait in front of that condition: - if const_expr(not traits.SPLITK and not traits.CAUSAL and ...): + if const_expr(traits.XCD_SWIZZLE and not traits.SPLITK and ...): `XCD_SWIZZLE` defaults to False and only the fp8 builder passes it. That PR measured and described fp8 only, and never touched the bf16 builder -- the string `xcd` does not appear in flash_attn_gfx950.py at all -- so the remap silently stopped applying to bf16. No test covered the mapping, so nothing caught it. Bisecting upstream on MI355X, B=2 H=32 D=128 bf16 non-causal, cache cold, TFLOP/s at S = 61,380 / 180,180 / 239,580, aiter in the same process as control: cf1db2f 7/31 #919 1284.6 1288.2 1286.8 772f070 8/6 #974 1256.7 1188.1 1184.3 494b2cb 8/9 #945 1255.6 1187.9 1187.2 f6d0a41 8/14 #991 1256.0 1182.8 1183.6 47009a3 8/14 #1004 1260.5 1188.6 1183.4 daee90e 8/17 #930 1254.1 1077.0 1060.9 96af91b 8/17 #1014 1251.8 1074.9 1060.6 4521c8c 8/18 #960 1249.3 1073.9 1059.0 aiter ~1252 ~1255 ~1255 Two independent steps. The first, between 7/31 and 8/6 (-8.0%), is #950 and is what this commit fixes. The second, between 47009a3 and daee90e (-10.4%), is #930 dropping the fastmath flag when it moved these kernels onto fx.maxnumf, which reaches the raw MLIR builder without passing through resolve_fastmath; that is a separate fix, filed separately. This plumbs `xcd_swizzle` through the bf16 builder and reuses the existing dispatch condition (non-causal, `H % NUM_XCD_GFX950 == 0`, `num_q_blocks >= MIN_Q_BLOCKS_XCD_SWIZZLE`). Split-K is excluded structurally: it takes the `_build_splitk` branch above this dispatch site. The remap is bijective, so output is bit-identical. Two tests assert that over H=8/16/32/64 and cover the `H % 8 != 0` fallback both forced and auto -- the guard #919 shipped without. One loose end: the restored figure is 1268.3 against cf1db2f's 1286.8, a 1.4% gap. That is #930's dropped fastmath, which costs 10.4% with the remap off but only 1.4% with it on -- the two interact. Fixing #930 separately closes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
coderfeli
pushed a commit
that referenced
this pull request
Aug 20, 2026
#919 made `_init_dualwave_thread_mapping` re-derive (head, q_block) from the linear workgroup id with the head as the slow axis, so a head's blocks stay on one XCD and its K/V stays resident in that XCD's L2 across the concurrent wavefronts. It applied to every qualifying dualwave dense shape, bf16 included, gated only on `not SPLITK and not CAUSAL and NUM_HEADS_Q % NUM_XCD_GFX950 == 0`. #950 put a trait in front of that condition: - if const_expr(not traits.SPLITK and not traits.CAUSAL and ...): + if const_expr(traits.XCD_SWIZZLE and not traits.SPLITK and ...): `XCD_SWIZZLE` defaults to False and only the fp8 builder passes it. That PR measured and described fp8 only, and never touched the bf16 builder -- the string `xcd` does not appear in flash_attn_gfx950.py at all -- so the remap silently stopped applying to bf16. No test covered the mapping, so nothing caught it. Bisecting upstream on MI355X, B=2 H=32 D=128 bf16 non-causal, cache cold, TFLOP/s at S = 61,380 / 180,180 / 239,580, aiter in the same process as control: cf1db2f 7/31 #919 1284.6 1288.2 1286.8 772f070 8/6 #974 1256.7 1188.1 1184.3 494b2cb 8/9 #945 1255.6 1187.9 1187.2 f6d0a41 8/14 #991 1256.0 1182.8 1183.6 47009a3 8/14 #1004 1260.5 1188.6 1183.4 daee90e 8/17 #930 1254.1 1077.0 1060.9 96af91b 8/17 #1014 1251.8 1074.9 1060.6 4521c8c 8/18 #960 1249.3 1073.9 1059.0 aiter ~1252 ~1255 ~1255 Two independent steps. The first, between 7/31 and 8/6 (-8.0%), is #950 and is what this commit fixes. The second, between 47009a3 and daee90e (-10.4%), is #930 dropping the fastmath flag when it moved these kernels onto fx.maxnumf, which reaches the raw MLIR builder without passing through resolve_fastmath; that is a separate fix, filed separately. This plumbs `xcd_swizzle` through the bf16 builder and reuses the existing dispatch condition (non-causal, `H % NUM_XCD_GFX950 == 0`, `num_q_blocks >= MIN_Q_BLOCKS_XCD_SWIZZLE`). Split-K is excluded structurally: it takes the `_build_splitk` branch above this dispatch site. The remap is bijective, so output is bit-identical. Two tests assert that over H=8/16/32/64 and cover the `H % 8 != 0` fallback both forced and auto -- the guard #919 shipped without. One loose end: the restored figure is 1268.3 against cf1db2f's 1286.8, a 1.4% gap. That is #930's dropped fastmath, which costs 10.4% with the remap off but only 1.4% with it on -- the two interact. Fixing #930 separately closes it. Co-authored-by: Claude Opus 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.
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist