Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wgsl-in] workgroup_size parsed as a builtin variable although it is an attribute only #1220

Closed
hcsch opened this issue Aug 18, 2021 · 2 comments · Fixed by #2147
Closed
Labels
area: validation Validation of the IR help wanted Extra attention is needed

Comments

@hcsch
Copy link
Contributor

hcsch commented Aug 18, 2021

workgroup_size doesn't seem to be a builtin variable at all (it's not listed in the builtin variables section of the spec). The spec defines it as an attribute (see workgroup_size). This means it should not be allowed as a parameter to main.
For example, the following should not be allowed:

fn main([[builtin(workgroup_size)]] workgroup_size: vec3<u32>) {

This attribute is only supposed to appear like this (see workgroup_size):

[[stage(compute), workgroup_size(128)]]
fn main() {

I find this somewhat regrettable, as you'll have to save your workgroup_size in a variable as well (since you can't use a constant variable in the attribute, only non-negative integer literals; duplicating the value, allowing for a mismatch between the actual value and the variable) if you wanna do some math with it, but that is an issue to be raised with the spec.

@kvark
Copy link
Member

kvark commented Aug 18, 2021

The spec has it at some point, and then gpuweb/gpuweb#1590 happened.
We haven't updated Naga for this specifically yet. It's likely that we'll keep the builtin in IR, since it's represented by the native languages, but we'd fail WebGPU validation if it's used.

@kvark kvark added area: validation Validation of the IR help wanted Extra attention is needed labels Aug 18, 2021
@teoxoy teoxoy added this to the WGSL Specification V1 milestone Apr 30, 2022
@jimblandy
Copy link
Member

Now the spec permits constant expressions as workgroup_size parameters, so you can use a named constant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: validation Validation of the IR help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants