[WGSL] Specify <immediate> address space - #5424
Conversation
Update index.bs to include the new <immediate> address space. Ref to proposal: immediate-data.md.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for a new immediate address space in WGSL for immediate data variables, along with minor whitespace cleanup. The immediate address space allows module-scope variables with specific type restrictions, where each entry point can statically access at most one such variable.
- Adds the
immediateaddress space with read-only access for module-scope variables - Defines constraints on immediate data variables (concrete, constructible, host-shareable types, excluding arrays and structures containing array members)
- Cleans up trailing whitespace in various sections
Comments suppressed due to low confidence (3)
wgsl/index.bs:5105
- The new
immediateaddress space should be added to the list of address spaces that must only be declared in module scope (line 5099-5101). According to the feature table at line 4841, immediate address space variables are module-scope variables, but this documentation section doesn't mention them.
Variables in the [=address spaces/private=], [=address spaces/storage=],
[=address spaces/uniform=], [=address spaces/workgroup=], and [=address
spaces/handle=] address spaces must only be declared in [=module scope=], while
variables in the [=address spaces/function=] address space must only be
declared in [=function scope=].
The address space [=shader-creation error|must=] be specified for all address
spaces except handle and function.
wgsl/index.bs:5146
- This documentation should be updated to include immediate data variables, as they are marked as 'Part of Resource Interface' with 'Yes' in the feature table at line 4845. If immediate data variables are indeed part of the resource interface, they should be listed here alongside uniform buffers, storage buffers, textures, and samplers.
As described in [[#resource-interface]], uniform buffers, storage buffers,
textures, and samplers form the [=resource interface of a shader=].
wgsl/index.bs:10122
- If immediate data variables are part of the resource interface (as indicated by 'Yes' in the feature table at line 4845), they should be added to this list of resource kinds. Otherwise, the feature table entry should be corrected to indicate 'No' for 'Part of Resource Interface'.
There are four kinds of resources:
* [=Uniform buffers=]
* [=Storage buffers=]
* [=Texture resources=]
* [=Sampler resources=]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alan-baker
left a comment
There was a problem hiding this comment.
There ought to be either a language feature or enable extension as part of this specification.
Additionally, there need to be some updates to the Resource Interface and Resource Layout Compatibility sections in the Entry Points chapter.
| <td>Invocations in the same [=shader stage=] | ||
| <td>[=access/read=] | ||
| <td>For [=immediate data=] variables.<br> | ||
| [=type/concrete|Concrete=] [=constructible=] [=host-shareable=] types, excluding arrays and structures containing array members.<br> |
There was a problem hiding this comment.
Suggest dropping this.
| [=type/concrete|Concrete=] [=constructible=] [=host-shareable=] types, excluding arrays and structures containing array members.<br> |
There was a problem hiding this comment.
Drop this because [=type/concrete=] ensures this?
There was a problem hiding this comment.
Kai mentioned
When I read this I think "immediate data variable" means one var<immediate>, of which there is of course only one (at most). But that var itself has a SizeOf the entire struct, whereas we actually want only accessible parts of the struct to need to be initialized. That is going to require the WGSL spec to "reflect" some new information for us - the map of slot -> is_accessible - for us to validate against.
I would recommend combining this PR with the WGSL PR, since there will be new integration between the specs.
In API side review. So I upload a combined PR here #5423. PTAL in this PR, thanks!
|
This PR was touched on in today's API committee meeting; minutes are here: #5423 (comment) |
Co-authored-by: alan-baker <alanbaker@google.com>
Co-authored-by: alan-baker <alanbaker@google.com>
|
Previews, as seen when this build job started (b7865fa): |
|
Copied the open comment to #5423, closing this |
Update index.bs to include the new address space. Ref to proposal: immediate-data.md.