-
Notifications
You must be signed in to change notification settings - Fork 353
Infer constant types in WGSL #1529
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
Conversation
|
Myles and I think that this could be helpfully extended to vars, but we are 100% in favor of doing it for const even if the group decides not to do it for vars. |
|
I agree that every expression in a valid WGSL program must have a single type, and that can be inferred bottom-up (#723). I think we can go one step further. In #1289 (post-MVP mileston) I argued for even more abbreviated syntax. So instead of We would have: That's even fewer characters to type on your keyboard. It also makes using 'const' the path of absolutely minimal resistance, and that's good. It turns out I had seen this before, from Go. See https://golang.org/ref/spec#Short_variable_declarations
The ugliest part of my #1289 proposal was dealing with pointer vs. non-pointer. But since then we've chosen a direction where we have split (dual) types for "ref" and "ptr", and so there's no longer a confusion. So we can cleanly do both : declare a pointer and declare a copied value: A clear followup is to infer the store type for a variable as well. It uses the same principle. Two possible syntaxes suggest themselves: or |
|
@dneto0 I'm tempted to say "let's discuss := as a follow-up" since it blows out the scope of this PR and asks for much more time discussing the proper syntax. Just refining In your (future?) proposal:
|
|
Discussed at 2021-03-23 meeting |
|
Agreed on the call to land this and follow-up with type inference for variables. |
|
@dneto0 it's rebased now. Any concerns about the wording? |
|
Answering:
It's important to allow the programmer to be very explicit. I would allow both:
I would not allow that. If you want to be explicit, write the whole thing:
No. It does not complicate the parser. The parsing into cases of call_statement, assignment_statement before checking for known-ness of the identifier. Adding my proposed case adds a "short_const_decl" case. It adds one more token into the follow-set of identifier: '(' for call expression, '=' for assignment, and adds ':=' for short-const-decl. |
| : variable_decl | ||
| | variable_decl EQUAL short_circuit_or_expression | ||
| | CONST variable_ident_decl EQUAL short_circuit_or_expression | ||
| | CONST (IDENT | variable_ident_decl) EQUAL short_circuit_or_expression |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'variable_ident_decl' production also starts with IDENT.
I think there are already other shortcuts like this in the grammar, so we can revisit that if/when we get a mechanically-processable grammar.
|
Does this mean types from functions will be inferred as well? |
I don't see why not. The inference is still purely local: myFunction() will always return vec2, and it is declared to do so. |
|
It's slightly more complicated because it's inferred from multiple expressions. I think it has less value and isn't very worth it. |
|
Also now that I think about it, it could potentially make the code less readable. |
|
Yes, there was no intention to infer function types. Let's not go into this territory, please :) |
|
And what about trigonometric functions, texture samples and builtIn variables? var angle : f32 = -atan2(a_particleVel.x, a_particleVel.y);
var uv : vec2<f32> = floor(30.0 * fragUV);
var texColor : vec4<f32> = textureSample(myTexture, mySampler, fragUV * 0.8 + vec2<f32>(0.1, 0.1));
var dims : vec2<i32> = textureDimensions(inputTex, 0);
var index : u32 = GlobalInvocationID.x;Is there any clear rule when we should stop infer types? Because this thing works differently in Firefox/Chrome currently. https://github.com/austinEng/webgpu-samples/pull/121 |
|
Please do not get caught up in the status of prototype implementations just yet. We will be enforcing conformance via testing. |
Hm, I think when I said this I thought the suggestion was to infer the return type of functions. That we shouldn't do. |
This PR updates existing storage texture format tests to expect exceptions rather than validation errors in those cases.
The changes to check for exceptions being thrown in gpuweb#1529 should only apply to cases where we are testing for an required feature being enabled. The updates to createBindGroupLayout.spec.ts were unnecessary and have started causing failures. FYI @Gyuyoung Issue: gpuweb#919
Helps #736 and #560 to the point of being able to postpone the rest till after MVP.
When porting shaders to WGSL, writing down the type for every constant appears to be the most painful thing. Consider this shader for example, here is a slice of it:
Writing all of these with types is just painful:
I generally want the language to be minimal and oppose the minor ergonomic changes if they increase complexity. This problem though is a major blocker for WGSL adoption in my eyes. Types are verbose and rich in special symbols (
:,<,>), and most importantly - they are redundant in all cases where we can infer them. I believe our API is currently structured in a way that makes the right-hand-size expression always have a defined type, so inferring theconstdeclarations is trivial.Justification to limiting this change to
constonly:constshould be preferred overvar. Making the former easy to write digs a good pit of success for WGSL.Edit: this is what it will look like: