Repository navigation
[BugFix][CUDA] Keep warp-specialized stages until moved non-TMA copies are consumed - #3288
ghostrider0470 wants to merge 1 commit into
Conversation
…s are consumed ProducerConsumerWSRewriter::BuildWSBlock places each TMA group's consumer release (arrive_insert_pos) after the last consumer access of that group's own TMA destination. Consumer statements in front of a group's first wait that are movable (e.g. synchronous global->shared copies whose rows are too narrow for TMA) are then moved into that group's producer prefix and run under the group's back-pressure wait, but the release point was not extended to cover the shared buffers they write. The producer could then overwrite that stage while the consumer still read it, which gives silently wrong results, e.g. an fp16 GEMM with A in a fragment plus an 8-byte-row bias tile, or the SM120 block-scaled GEMM with block_K=128 row-major scales. Extend each group's release point to the last non-moved consumer access of every shared buffer written by its moved prefix statements, and take the maximum when the TMA barriers are merged. Kernels without moved prefix writes generate the same code as before.
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: tile-ai/tilelang/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe warp-specialization rewrite now extends consumer release positions to cover the last consumer access to shared buffers written by moved producer-prefix statements. New CUDA regression tests cover row-bias copies and NVFP4 scale copies. ChangesConsumer release position
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue was established for the release-ordering change; it is ready for normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change appears to correct a race that could silently corrupt results in affected CUDA kernels. It does not appear to expand who can invoke the compiler pass or grant new authority. Verification is limited to the inspected source and supplied regression cases. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
tile-ai/tilelang#3430 is ready for review after its NVF4 tests passed on GB10. The MXFP8/MXFP4 and example PRs passed their GB10 suites ahead of opening. tile-ai/tilelang#3288 leaves our decode-tile race unchanged (identical generated CUDA), so the race gets its own planned upstream issue.
Summary
Fixes #3287.
A
T.Pipelinedloop compiled with warp specialization can release a pipeline stage to the producer while the consumeris still reading part of it. This produces silently wrong results. It happens when a consumer-side synchronous
global->shared copy (for example, a tile whose rows are too narrow for TMA) is moved into a TMA group's producer prefix,
and the group's release point comes before the last read of that copy's buffer.
This PR moves the group's release point after that last read. Kernels without such a moved copy generate byte-identical
CUDA (checked on 27 upstream example and test kernels for sm_90a and sm_120a, see below).
Root cause
In
src/cuda/transform/producer_consumer_ws.cc,ProducerConsumerWSRewriter::BuildWSBlock:arrive_insert_pos[g]) is placed after the last consumeraccess of its own TMA destination. With A copied into a register fragment (
A_frag[i, k] = A_shared[i, k]),the A group is released right after that copy.
IsProducerMovableLoopPrefixStmtare moved into
producer_loop_prefix_stmts[g]. This includes synchronous global->shared copies. In the producerthey are emitted after that group's back-pressure wait (lines 1575-1599), so they write their multi-versioned stage
under the A group's barrier.
arrive_insert_posis never updated for the buffers written by the moved statements. The consumer arrives on theA group's back-pressure barrier before it reads the moved buffer (in the GEMM or the epilogue). The producer then
overwrites that stage for iteration
ko + num_stageswhile the current one is still being read: a write-after-readrace.
Generated CUDA before the fix (fp16 GEMM with an 8-byte-row bias tile, 2 stages, abridged; sm_90a looks the same with
wgmma_rs):Fix
After the prefix-move loop, for each group, collect the shared buffers written by its moved prefix statements (the copy
destination from
ExtractProducerWriteBufferData, plus any sharedBufferStore). Then raisearrive_insert_pos[g]to just after the last non-moved consumer statement that accesses any of them. When the TMA barriers are merged, the
single merged group takes the maximum over all groups. If a group's prefix writes nothing, or its release already comes
after the last access, nothing changes.
With the fix, the consumer arrive moves below the last read:
A more aggressive alternative would be to put such copies in the prefix of a group whose release already covers
their buffer, which would avoid moving any release. That is a larger change to the prefix placement, so this PR keeps
the minimal, conservative fix.
Tests
New tests in
testing/python/transform/test_tilelang_transform_producer_consumer_ws.py:test_tiled_ws_release_covers_moved_non_tma_copy(requires_cuda_compute_version(9, 0)): an fp16 GEMM with Athrough a fragment and a per-row bias tile of two float32 per row (8-byte rows, so no TMA), 2 stages. It checks the
generated source (the last back-pressure
arrivein the consumer follows the lastS_sharedread, which holds onsm_90a and sm_120a codegen with the fix and fails without it), and checks the result bit-exactly (small integers).
test_tiled_ws_release_covers_moved_scale_copies_sm120_blockscaled(requires_cuda_compute_version_eq(12, 0)): theSM120 NVFP4 block-scaled GEMM with A in a fragment and row-major scales at
block_K=128(8-byte scale rows),checked bit-exactly with power-of-two scales.
Repro matrix (RTX PRO 6000 Blackwell, SM120; M = N = 256, 128x128 tiles)
FAIL= tens of thousands of the 65,536 outputs wrong (max abs error 200-2,300), not rounding.mainblock_K64 / 128 / 192block_K=128, 2 stagesblock_K=256(TMA scales)Totals over the 35 configurations: 15 fail on
mainand 0 with this PR; 19 pass on both. The remaining one(
block_K=256with 3 stages) exceeds SM120 shared memory on both builds.Upstream transform tests
test_tilelang_transform_producer_consumer_ws.py,..._producer_consumer_ws_persistent.py,..._pipeline_barrier_ownership.pyand..._materialize_ws_schedule.pyfrom this branch, on SM120:maintest_tiled_ws_multi_versioned_wide_linear_smem_keeps_tmatest_tiled_ws_wide_linear_smem_three_stagestest_tiled_ws_release_covers_moved_non_tma_copy(new)test_tiled_ws_release_covers_moved_scale_copies_sm120_blockscaled(new)The two wide-linear failures are
Failed to set the allowed dynamic shared memory sizeon this GPU (SM120 allowsabout 99 KiB per block), and they fail the same way without this PR. I could not run the sm_90-only tests.
Unaffected kernels: generated CUDA is unchanged
I lowered 27 kernels from upstream examples and tests with and without the patch (
tilelang.lower, no devicecompile), for sm_90a and for sm_120a (57 sources; the three SM120-only kernels only for sm_120a), and diffed the CUDA
source:
examples/gemm(example_gemm, non-persistent 3-stage GEMM), the four WS test GEMMs(2 and 3 stages, dual GEMM, windowed), the wide-linear reduction,
warp_specialize/..._softpipe_stage2,gemm_splitk,blocksparse_gemm,gemm_fp8(e4m3 and 2xAcc),deepseek_deepgemmfp8 2xAcc (block_N128 and64), flash attention (MHA fwd, causal fwd, GQA fwd with 2 stages, MHA bwd and its fwd),
deepseek_mladecode, thedequant GEMMs (bf16/fp4, bf16/mxfp4 with and without bias, fp16/fp4, w4a8), the SM120 block-scaled example at
block_K256 and 128 (its scales go through cp.async, not a moved synchronous copy), and the new test's kernel with16-byte bias rows.
These are the only kernels in the set whose producer branch contains a synchronous shared store, i.e. a moved
non-TMA copy. In each, the diff is one line: the A group's
mbarrier[(ko_1 & 1) + 4].arrive()moves from after thefragment copy to after the last read of the moved buffer. Nothing else changes.
Performance of affected kernels
The later release means less producer run-ahead for those kernels. RTX PRO 6000 Blackwell (SM120),
M = N = K = 4096, base and fix alternated in separate processes, 3 rounds x median of 5
do_benchruns:main(wrong results)block_K=128, 2 stagesblock_K=128, 3 stagesblock_K=64, 3 stagesThe baseline numbers come from the racy code, so they are not a correct result to compare against. With
block_K=256the scales use TMA and the bug does not apply.Relation to other PRs and issues
tvm_storage_syncbefore TMApublication when prefix statements write shared memory. That is the forward (read-after-write) edge. It does not move
the consumer release, so it does not fix this write-after-read race. The two changes are in different hunks and
[Compiler] Unify async data staging and transfer-aware lowering #3279's
producer_consumer_ws.cchunk applies cleanly on top of this commit.not change the prelude path, so it is a separate issue.
Checklist
.pre-commit-config.yamlpin): no changes. ruff 0.16.6 check and format: clean.codespell: clean.
maint/scripts/audit_cpp_api_style.py --include-sources src/cuda/transform/producer_consumer_ws.cc: the same 5pre-existing findings before and after (3 x TLCPP004, 2 x TLCPP001); none are in the new code.
Summary
Tests
The PR objectives report that the added fp16 and SM120 block-scaled tests pass. They also report byte-identical generated CUDA for kernels without moved writes.
C++ style / lint notes
docs/developer_guide/cpp_style.md.