Repository navigation
fix(cuda): size a kernel's dynamic shared memory and declare its base once - #271
Merged
Merged
Conversation
FeathBow
marked this pull request as draft
October 26, 2026 11:36
Contributor
Author
|
Turning this into a draft while I add the |
FeathBow
marked this pull request as ready for review
October 26, 2026 12:13
zhen8838
approved these changes
Oct 26, 2026
zhen8838
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @FeathBow, approving.
Scanning the TIR views is the right place for this. compile accepts TIR directly, from a schedule finalize output file or written by hand, and neither carries the memory analysis, so the host entry has to work out the size from the views. Your reasons against my suggestion in #267 are right too: a request for the full capacity fails as soon as the kernel holds any static shared memory, and it also costs occupancy.
This PR also exposed two gaps on our side, which I tracked in #272. Neither blocks this PR:
- The allocator places shared memory against the full per-CTA limit and does not leave room for the runtime's static shared memory.
schedule finalizedrops the shared memory peak that the analysis already computed.
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.
Why
The synthesized host entry launches every TIR kernel with
dynamic_smem=0. A kernel that addresses shared memory through integerTensorViewoffsets therefore runs without it, and above 48 KiB the launch would also needcudaFuncAttributeMaxDynamicSharedMemorySize, which the launcher never sets. These are the first two steps of the plan in #267.In #267 the suggestion was to default to the target's shared memory capacity. I've sized it from the kernel instead, for two reasons. CUDA counts a kernel's static shared memory against the same limit for each block, and ours can carry some (
ProgramMetaDatainruntime.cuh, or a sharedAllocTensor), so asking for the full capacity fails to launch. And what each block asks for decides how many blocks fit on an SM. On sm_89 a 99 KiB request leaves room for one block even when the kernel uses 512 bytes.CUTLASS and TileLang size it the same way. CUTLASS's
GemmUniversalAdapterlaunches withsizeof(SharedStorage)and TileLang's CUDA wrapper with the size its shared memory planning computes, and both raise the attribute to that size.Codegen also declares
smem_basewhere a kernel first names it. Inloop_outputsthat is inside the first loop's block, so the second loop can't see it and nvcc stops withidentifier "smem_base" is undefined. In #267 the question was whether the allocation records that buffer under the first loop. It doesn't. Each loop has its own 8 bytelandingat offset 0 with a lifetime inside that loop, so the accounting is right and the declaration point is what needs fixing.What
insert_default_host_entrygives the synthesized TIR entry'sLaunchthe farthest byte reached by any shared memoryTensorViewat an integer address in the device function or its variants. Each view is sized withstorage_bytes, the span the allocator reserves for it within one CTA, the level that owns shared memory. A view split across CTAs counts one CTA's share, and a view split across threads counts the whole tile. An authored host entry keeps the size its author wrote.Above 48 KiB the CUDA launcher raises the kernel's
cudaFuncAttributeMaxDynamicSharedMemorySizeto the request before launching, and returns CUDA's error text if the device refuses.CUDA codegen now writes each kernel body first and, if the body names
smem_base, declares it once ahead of the first statement, where every scope in the kernel can reach it.Three new host entry cases cover a ring of two slots that reaches 1024 bytes, a prototype whose variants reach 512 and 2560 bytes, and a view split across two CTAs that asks for one CTA's 128 KiB. Two GPU tests run on the H200 and sm_89 targets.
test_launch.pystages an 80 KiB tile through shared memory, andtest_shared_memory_base.pyruns the two staging loops ofloop_outputson one thread, with the wait each read back needs. On a single sm_89 GPU both pass, and without this change the first hits an illegal memory access and the second fails to compile. The other sm_89 tests still pass, and the CPU suite gives the same results before and after apart from the new host entry cases.Contract
targets/nvidia/tilefoundry_nvidia/cuda/docs/spec/passes.mdrequires the synthesized TIR entry to ask for the dynamic shared memory its device function reaches, and lets an authored host entry keep its own size.targets/nvidia/tilefoundry_nvidia/cuda/docs/spec/codegen.mdrequires a launcher asked for more than 48 KiB to raise the kernel's limit first and to report a refusal as a failed launch, and each kernel to declare its dynamic shared memory base once, ahead of its first statement.Risk
A kernel whose static and dynamic shared memory together exceed the device limit now fails at launch with CUDA's error. HIR entries keep
dynamic_smem=0, since HIR codegen doesn't use dynamic shared memory. The GPU tests' H200 cases run only in CI, on an H200.