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.

Sign up for GitHub

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

Jump to bottom

[naga wgsl] Implement local const declarations #6156

Merged
merged 6 commits into from
Aug 30, 2024
Merged

[naga wgsl] Implement local const declarations #6156

merged 6 commits into from
Aug 30, 2024

Conversation

sagudev
Copy link
Contributor

@sagudev sagudev commented Aug 24, 2024 •
edited
Loading

Connections
Fixes #4493

Description
For wgsl-in I handled const declarations similarly as let by adding expressions to directly to named_expressions (first commit). For wgsl-out I simply check if expr in start_named_expr call is constant expr or not and based on that write const/let (see second commit), this exposed some problems as naga report some expr as constant although they are not actually impl as const, workaround is in third commit by introducing ExpressionKind::ImplConst for expressions that are also impl as const in naga, alternatively we could always print them as let statements and provide WriterFlags::CONST that will constify all wgsl const expr.

Testing
Some new manually written tests and I plan to also test this against CTS via servo.

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@sagudev sagudev changed the title naga/wgsl: Implement local const declerations [naga wgsl] Implement local const declerations Aug 24, 2024
@sagudev sagudev force-pushed the fn-local-const-declare branch 2 times, most recently from 8580b6f to 19f03dc Compare August 25, 2024 06:59
@sagudev

This comment was marked as resolved.

Sign in to view
@sagudev sagudev force-pushed the fn-local-const-declare branch 7 times, most recently from d5205d0 to 6abd2ad Compare August 25, 2024 16:03
@sagudev sagudev mentioned this pull request Aug 25, 2024
Closed
@sagudev sagudev force-pushed the fn-local-const-declare branch 3 times, most recently from 3b4d18c to 6e7eddb Compare August 25, 2024 18:26
@sagudev
[wgsl-in] Add LocalConst and handle it
d5ac547 
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev
[wgsl-out] Write const named expr as const declarations
6125ab4 
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev
Workaround for some builtins not implemented as constant expression
8e967db 
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev
Update test expectations
f13bd64 
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev
Tests for local consts
fba1985 
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev

This comment was marked as outdated.

Sign in to view
@sagudev
rebase fixes
428f74e 
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev
Copy link
Contributor Author

sagudev commented Aug 27, 2024

CTS results from servo:

OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,decl,const:function_scope:*
  PASS [expected FAIL] subtest: :
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,expression,access,array:early_eval_errors:*
  PASS [expected FAIL] subtest: :case="const_func_in_bounds"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,expression,access,matrix:early_eval_errors:*
  PASS [expected FAIL] subtest: :case="const_func_in_bounds"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,expression,access,vector:abstract:*
  FAIL [expected PASS] subtest: :vector_width=2;abstract_type="float";concrete_type="u32"
  FAIL [expected PASS] subtest: :vector_width=2;abstract_type="float";concrete_type="i32"
  FAIL [expected PASS] subtest: :vector_width=3;abstract_type="float";concrete_type="u32"
  FAIL [expected PASS] subtest: :vector_width=3;abstract_type="float";concrete_type="i32"
  FAIL [expected PASS] subtest: :vector_width=4;abstract_type="float";concrete_type="u32"
  FAIL [expected PASS] subtest: :vector_width=4;abstract_type="float";concrete_type="i32"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,expression,binary,bitwise_shift:shift_left_concrete:*
  PASS [expected FAIL] subtest: :case={"lhs":"0u","rhs":"31u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"0u","rhs":"31u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"0u","rhs":"31u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"0u","rhs":"31u","pass":true};vectorize=4
  PASS [expected FAIL] subtest: :case={"lhs":"0i","rhs":"31u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"0i","rhs":"31u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"0i","rhs":"31u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"0i","rhs":"31u","pass":true};vectorize=4
  FAIL [expected PASS] subtest: :case={"lhs":"1073741824i","rhs":"1u","pass":false};vectorize="_undef_"
  FAIL [expected PASS] subtest: :case={"lhs":"1073741824i","rhs":"1u","pass":false};vectorize=2
  FAIL [expected PASS] subtest: :case={"lhs":"1073741824i","rhs":"1u","pass":false};vectorize=3
  FAIL [expected PASS] subtest: :case={"lhs":"1073741824i","rhs":"1u","pass":false};vectorize=4
  FAIL [expected PASS] subtest: :case={"lhs":"2147483647i","rhs":"1u","pass":false};vectorize="_undef_"
  FAIL [expected PASS] subtest: :case={"lhs":"2147483647i","rhs":"1u","pass":false};vectorize=2
  FAIL [expected PASS] subtest: :case={"lhs":"2147483647i","rhs":"1u","pass":false};vectorize=3
  FAIL [expected PASS] subtest: :case={"lhs":"2147483647i","rhs":"1u","pass":false};vectorize=4
  FAIL [expected PASS] subtest: :case={"lhs":"1i","rhs":"31u","pass":false};vectorize="_undef_"
  FAIL [expected PASS] subtest: :case={"lhs":"1i","rhs":"31u","pass":false};vectorize=2
  FAIL [expected PASS] subtest: :case={"lhs":"1i","rhs":"31u","pass":false};vectorize=3
  FAIL [expected PASS] subtest: :case={"lhs":"1i","rhs":"31u","pass":false};vectorize=4
  PASS [expected FAIL] subtest: :case={"lhs":"1073741824u","rhs":"1u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"1073741824u","rhs":"1u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"1073741824u","rhs":"1u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"1073741824u","rhs":"1u","pass":true};vectorize=4
  PASS [expected FAIL] subtest: :case={"lhs":"2147483647u","rhs":"1u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"2147483647u","rhs":"1u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"2147483647u","rhs":"1u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"2147483647u","rhs":"1u","pass":true};vectorize=4
  PASS [expected FAIL] subtest: :case={"lhs":"1u","rhs":"31u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"1u","rhs":"31u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"1u","rhs":"31u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"1u","rhs":"31u","pass":true};vectorize=4
  FAIL [expected PASS] subtest: :case={"lhs":"3221225472u","rhs":"1u","pass":false};vectorize="_undef_"
  FAIL [expected PASS] subtest: :case={"lhs":"3221225472u","rhs":"1u","pass":false};vectorize=2
  FAIL [expected PASS] subtest: :case={"lhs":"3221225472u","rhs":"1u","pass":false};vectorize=3
  FAIL [expected PASS] subtest: :case={"lhs":"3221225472u","rhs":"1u","pass":false};vectorize=4
  FAIL [expected PASS] subtest: :case={"lhs":"4294967295u","rhs":"1u","pass":false};vectorize="_undef_"
  FAIL [expected PASS] subtest: :case={"lhs":"4294967295u","rhs":"1u","pass":false};vectorize=2
  FAIL [expected PASS] subtest: :case={"lhs":"4294967295u","rhs":"1u","pass":false};vectorize=3
  FAIL [expected PASS] subtest: :case={"lhs":"4294967295u","rhs":"1u","pass":false};vectorize=4
  FAIL [expected PASS] subtest: :case={"lhs":"4294967295u","rhs":"31u","pass":false};vectorize="_undef_"
  FAIL [expected PASS] subtest: :case={"lhs":"4294967295u","rhs":"31u","pass":false};vectorize=2
  FAIL [expected PASS] subtest: :case={"lhs":"4294967295u","rhs":"31u","pass":false};vectorize=3
  FAIL [expected PASS] subtest: :case={"lhs":"4294967295u","rhs":"31u","pass":false};vectorize=4
  PASS [expected FAIL] subtest: :case={"lhs":"-1073741824i","rhs":"1u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"-1073741824i","rhs":"1u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"-1073741824i","rhs":"1u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"-1073741824i","rhs":"1u","pass":true};vectorize=4
  PASS [expected FAIL] subtest: :case={"lhs":"-1i","rhs":"1u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"-1i","rhs":"1u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"-1i","rhs":"1u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"-1i","rhs":"1u","pass":true};vectorize=4
  PASS [expected FAIL] subtest: :case={"lhs":"-1i","rhs":"31u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"-1i","rhs":"31u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"-1i","rhs":"31u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"-1i","rhs":"31u","pass":true};vectorize=4
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,expression,binary,bitwise_shift:shift_right_concrete:*
  PASS [expected FAIL] subtest: :case={"lhs":"0u","rhs":"31u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"0u","rhs":"31u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"0u","rhs":"31u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"0u","rhs":"31u","pass":true};vectorize=4
  PASS [expected FAIL] subtest: :case={"lhs":"0i","rhs":"31u","pass":true};vectorize="_undef_"
  PASS [expected FAIL] subtest: :case={"lhs":"0i","rhs":"31u","pass":true};vectorize=2
  PASS [expected FAIL] subtest: :case={"lhs":"0i","rhs":"31u","pass":true};vectorize=3
  PASS [expected FAIL] subtest: :case={"lhs":"0i","rhs":"31u","pass":true};vectorize=4
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,expression,call,builtin,textureGather:component_argument,non_const:*
  PASS [expected FAIL] subtest: :textureType="texture_2d";sampleType="vec4%3Cf32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_2d";sampleType="vec4%3Ci32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_2d";sampleType="vec4%3Cu32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_2d_array";sampleType="vec4%3Cf32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_2d_array";sampleType="vec4%3Ci32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_2d_array";sampleType="vec4%3Cu32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_cube";sampleType="vec4%3Cf32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_cube";sampleType="vec4%3Ci32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_cube";sampleType="vec4%3Cu32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_cube_array";sampleType="vec4%3Cf32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_cube_array";sampleType="vec4%3Ci32%3E";varType="c"
  PASS [expected FAIL] subtest: :textureType="texture_cube_array";sampleType="vec4%3Cu32%3E";varType="c"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,expression,matrix,mul:overflow_scalar_f32:*
  PASS [expected FAIL] subtest: :rhs=1;c=2;r=2
  PASS [expected FAIL] subtest: :rhs=1;c=2;r=3
  PASS [expected FAIL] subtest: :rhs=1;c=2;r=4
  PASS [expected FAIL] subtest: :rhs=1;c=3;r=2
  PASS [expected FAIL] subtest: :rhs=1;c=3;r=3
  PASS [expected FAIL] subtest: :rhs=1;c=3;r=4
  PASS [expected FAIL] subtest: :rhs=1;c=4;r=2
  PASS [expected FAIL] subtest: :rhs=1;c=4;r=3
  PASS [expected FAIL] subtest: :rhs=1;c=4;r=4
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,parse,diagnostic:invalid_locations:*
  PASS [expected FAIL] subtest: :type="attribute";location="function_const"
  PASS [expected FAIL] subtest: :type="directive";location="function_const"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,parse,identifiers:function_const_name:*
  PASS [expected FAIL] subtest: :ident="foo"
  PASS [expected FAIL] subtest: :ident="Foo"
  PASS [expected FAIL] subtest: :ident="FOO"
  PASS [expected FAIL] subtest: :ident="_0"
  PASS [expected FAIL] subtest: :ident="_foo0"
  PASS [expected FAIL] subtest: :ident="_0foo"
  PASS [expected FAIL] subtest: :ident="foo__0"
  PASS [expected FAIL] subtest: :ident="%CE%94%CE%AD%CE%BB%CF%84%CE%B1"
  PASS [expected FAIL] subtest: :ident="r%C3%A9flexion"
  PASS [expected FAIL] subtest: :ident="%D0%9A%D1%8B%D0%B7%D1%8B%D0%BB"
  PASS [expected FAIL] subtest: :ident="%F0%90%B0%93%F0%90%B0%8F%F0%90%B0%87"
  PASS [expected FAIL] subtest: :ident="%E6%9C%9D%E7%84%BC%E3%81%91"
  PASS [expected FAIL] subtest: :ident="%D8%B3%D9%84%D8%A7%D9%85"
  PASS [expected FAIL] subtest: :ident="%EA%B2%80%EC%A0%95"
  PASS [expected FAIL] subtest: :ident="%D7%A9%D6%B8%D7%81%D7%9C%D7%95%D6%B9%D7%9D"
  PASS [expected FAIL] subtest: :ident="%E0%A4%97%E0%A5%81%E0%A4%B2%E0%A4%BE%E0%A4%AC%E0%A5%80"
  PASS [expected FAIL] subtest: :ident="%D6%83%D5%AB%D6%80%D5%B8%D6%82%D5%A6"
  PASS [expected FAIL] subtest: :ident="bf16"
  PASS [expected FAIL] subtest: :ident="f64"
  PASS [expected FAIL] subtest: :ident="i16"
  PASS [expected FAIL] subtest: :ident="i8"
  PASS [expected FAIL] subtest: :ident="quat"
  PASS [expected FAIL] subtest: :ident="u16"
  PASS [expected FAIL] subtest: :ident="u8"
  PASS [expected FAIL] subtest: :ident="unsigned"
  FAIL [expected PASS] subtest: :ident="const_assert"
  FAIL [expected PASS] subtest: :ident="diagnostic"
  FAIL [expected PASS] subtest: :ident="require"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,parse,semicolon:after_fn_const_decl:*
  PASS [expected FAIL] subtest: :
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,statement,compound:parse:*
  PASS [expected FAIL] subtest: :stmt="decl"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,statement,continuing:placement:*
  PASS [expected FAIL] subtest: :stmt="continuing_const"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,statement,for:parse:*
  PASS [expected FAIL] subtest: :test="init_const"
  PASS [expected FAIL] subtest: :test="init_const_type"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,statement,phony:rhs_with_decl:*
  PASS [expected FAIL] subtest: :test="function_const"
OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,statement,switch:parse:*
  PASS [expected FAIL] subtest: :test="L_default"
  PASS [expected FAIL] subtest: :test="L_paren_default"
  PASS [expected FAIL] subtest: :test="L_case_1_2_default"
  PASS [expected FAIL] subtest: :test="L_case_1_case_2_default"
  PASS [expected FAIL] subtest: :test="L_case_1_colon_case_2_colon_default_colon"
  PASS [expected FAIL] subtest: :test="L_case_1_colon_default_colon"
  PASS [expected FAIL] subtest: :test="L_case_1_colon_default"
  PASS [expected FAIL] subtest: :test="L_case_1_default_2"
  PASS [expected FAIL] subtest: :test="L_case_1_default_case_2"
  PASS [expected FAIL] subtest: :test="L_case_1_default_colon"
  PASS [expected FAIL] subtest: :test="L_case_1_default"
  PASS [expected FAIL] subtest: :test="L_case_2_1_default"
  PASS [expected FAIL] subtest: :test="L_case_2_case_1_default"
  PASS [expected FAIL] subtest: :test="L_case_2_default_case_1"
  PASS [expected FAIL] subtest: :test="L_case_builtin_default"
  PASS [expected FAIL] subtest: :test="L_case_default_1"
  PASS [expected FAIL] subtest: :test="L_case_default_2_1"
  PASS [expected FAIL] subtest: :test="L_case_default_2_case_1"
  PASS [expected FAIL] subtest: :test="L_case_default"
  PASS [expected FAIL] subtest: :test="L_case_expr_default"
  PASS [expected FAIL] subtest: :test="L_default_break"
  PASS [expected FAIL] subtest: :test="L_default_case_1_2"
  PASS [expected FAIL] subtest: :test="L_default_case_1_break"
  PASS [expected FAIL] subtest: :test="L_default_case_1_case_2"
  PASS [expected FAIL] subtest: :test="L_default_case_1_colon_break"
  PASS [expected FAIL] subtest: :test="L_default_case_2_case_1"
  PASS [expected FAIL] subtest: :test="L_default_colon_break"
  PASS [expected FAIL] subtest: :test="L_default_colon"

@sagudev

This comment was marked as resolved.

Sign in to view
@sagudev sagudev marked this pull request as ready for review August 27, 2024 19:55
@sagudev sagudev requested a review from a team as a code owner August 27, 2024 19:55
@ErichDonGubler

This comment was marked as resolved.

Sign in to view
@sagudev

This comment was marked as resolved.

Sign in to view
@sagudev

This comment was marked as resolved.

Sign in to view
@sagudev sagudev mentioned this pull request Aug 28, 2024
Closed
@ErichDonGubler ErichDonGubler changed the title [naga wgsl] Implement local const declerations [naga wgsl] Implement local const declarations Aug 28, 2024
@ErichDonGubler
Copy link
Member

ErichDonGubler commented Aug 28, 2024 •
edited
Loading

@sagudev:

Per gpuweb.github.io/gpuweb/wgsl#bit-expr we need to report overflow when value goes into sign bits, while rust's checked_shl only reports overflow when its out of bits.

…but this doesn't need to block this PR from getting merged, right? Should we file a separate issue to track this? EDIT: D'oh, of course we have one already, heh 😅: #6175

@sagudev
Copy link
Contributor Author

sagudev commented Aug 28, 2024

…but this doesn't need to block this PR from getting merged, right?

Right.

teoxoy
teoxoy approved these changes Aug 30, 2024
View reviewed changes
Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great!

@teoxoy teoxoy merged commit 34bb9e4 into gfx-rs:trunk Aug 30, 2024
25 checks passed
@sagudev sagudev mentioned this pull request Aug 30, 2024
Merged
3 tasks
jimblandy added a commit to jimblandy/wgpu that referenced this pull request Sep 5, 2024
@jimblandy
[naga] Update docs for private items.
d112b2d 
Make `cargo doc --document-private-items` work again in Naga.

Update some documentation missed by the local const work in gfx-rs#6156.
@jimblandy jimblandy mentioned this pull request Sep 5, 2024
Merged
teoxoy pushed a commit that referenced this pull request Sep 5, 2024
@jimblandy
[naga] Update docs for private items. (#6218)
64ea7ef 
Make `cargo doc --document-private-items` work again in Naga.

Update some documentation missed by the local const work in #6156.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teoxoy teoxoy teoxoy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Function-local const declarations not implemented
3 participants
@sagudev @ErichDonGubler @teoxoy