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] firstLeadingBit returns wrong integer type + unsanitised clamp error #1844

Closed
hanawatson opened this issue Apr 16, 2022 · 3 comments · Fixed by #1865
Closed

[wgsl-in] firstLeadingBit returns wrong integer type + unsanitised clamp error #1844

hanawatson opened this issue Apr 16, 2022 · 3 comments · Fixed by #1865
Labels
area: front-end Input formats for conversion kind: bug Something isn't working lang: WGSL WebGPU shading language

Comments

@hanawatson
Copy link

hanawatson commented Apr 16, 2022

The spec definition for firstLeadingBit seems to suggest that if it's given 0 as an argument, it will return -1 - and if it's 0u specifically, will return u32(-1) (i.e. a conversion to keep the return type as u32). naga seems to be returning an i32 instead

code:

@stage(compute) @workgroup_size(1)
fn compute_main(
) {
	var var0: u32 = firstLeadingBit(0u);
}

error:

[2022-04-16T20:01:40Z ERROR naga::front::wgsl] Given type Scalar { kind: Uint, width: 4 } doesn't match expected Scalar { kind: Sint, width: 4 }
error: the type of `var0` is expected to be `i32`
  ┌─ test.wgsl:4:6
  │
4 │     var var0: u32 = firstLeadingBit(0u);
  │         ^^^^ definition of `var0`

Could not parse WGSL

I found the above after reducing some larger code that errored on compilation - during reduction I saw a (possibly?) unsanitised error as well

code:

@stage(compute) @workgroup_size(1)
fn compute_main(
) {
	var var0: u32 = clamp(abs(u32()), firstLeadingBit(((u32()))), 538497703u);
}

error:

error: 
  ┌─ test.wgsl:4:17
  │
4 │     var var0: u32 = clamp(abs(u32()), firstLeadingBit(((u32()))), 538497703u);
  │                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ naga::Expression [6]

Entry point compute_main at Compute is invalid: 
	Expression [6] is invalid
	Argument [1] to Clamp as expression [4] has an invalid type.

My original code was a large switch statement, and the error I got was that my case was a u32 (i.e. the wrong type for an i32 selector) - since the selector expression contained the above clamp code, it seems like there might be an issue wrt naga accepting that code - i.e. if the clamp is invalid as it says above, the whole selector should surely be invalid instead of being accepted as a legitimate expression returning an i32 value (and therefore my u32 case being the actual problem)? Extracting the selector expression by itself and assigning it to a u32 var as above also just gave an incorrect-typing error instead of invalid error. Let me know if dropping that code here also would be useful :)

@teoxoy
Copy link
Member

teoxoy commented Apr 16, 2022

The issue with the switch with different types in its case selectors is tracked in #1777.

Regarding the clamp validation error, according to the spec clamp only works with floats.

@teoxoy teoxoy added kind: bug Something isn't working lang: WGSL WebGPU shading language area: front-end Input formats for conversion labels Apr 16, 2022
@hanawatson
Copy link
Author

Ah I see, sorry for the unnecessary repeat report in that case!

Regarding clamp, I believe there's currently a separate version in the spec that takes integers as arguments (but only all the same, e.g. all arguments u32 or all i32) - I think possibly the error here comes from the fact that the middle expression is returning i32 so doesn't match the others?

@teoxoy
Copy link
Member

teoxoy commented Apr 16, 2022

Ah indeed there is another overloaded clamp fn for integers. The error is most likely caused by the mismatch of argument types. The validation errors are quite cryptic in some cases, they could be improved but it takes time to get them right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: front-end Input formats for conversion kind: bug Something isn't working lang: WGSL WebGPU shading language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants