[naga wgsl] Implement local `const` declarations #6156

sagudev · 2024-08-24T12:47:28Z

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.

Signed-off-by: sagudev <[email protected]>

sagudev · 2024-08-27T17:38:46Z

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"

ErichDonGubler · 2024-08-28T13:29:44Z

@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 · 2024-08-28T13:32:13Z

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

Right.

teoxoy

This is great!

Make `cargo doc --document-private-items` work again in Naga. Update some documentation missed by the local const work in gfx-rs#6156.

Make `cargo doc --document-private-items` work again in Naga. Update some documentation missed by the local const work in #6156.

sagudev changed the title ~~naga/wgsl: Implement local const declerations~~ [naga wgsl] Implement local const declerations Aug 24, 2024

sagudev force-pushed the fn-local-const-declare branch 2 times, most recently from 8580b6f to 19f03dc Compare August 25, 2024 06:59

This comment was marked as resolved.

Sign in to view

sagudev force-pushed the fn-local-const-declare branch 7 times, most recently from d5205d0 to 6abd2ad Compare August 25, 2024 16:03

sagudev mentioned this pull request Aug 25, 2024

Function-local const declarations not implemented #4493

Closed

sagudev force-pushed the fn-local-const-declare branch 3 times, most recently from 3b4d18c to 6e7eddb Compare August 25, 2024 18:26

sagudev added 5 commits August 27, 2024 12:08

[wgsl-in] Add LocalConst and handle it

d5ac547

Signed-off-by: sagudev <[email protected]>

[wgsl-out] Write const named expr as const declarations

6125ab4

Signed-off-by: sagudev <[email protected]>

Workaround for some builtins not implemented as constant expression

8e967db

Signed-off-by: sagudev <[email protected]>

Update test expectations

f13bd64

Signed-off-by: sagudev <[email protected]>

Tests for local consts

fba1985

Signed-off-by: sagudev <[email protected]>

sagudev force-pushed the fn-local-const-declare branch from 6e7eddb to fba1985 Compare August 27, 2024 10:08

This comment was marked as outdated.

Sign in to view

rebase fixes

428f74e

Signed-off-by: sagudev <[email protected]>

This comment was marked as resolved.

Sign in to view

sagudev marked this pull request as ready for review August 27, 2024 19:55

sagudev requested a review from a team as a code owner August 27, 2024 19:55

This comment was marked as resolved.

Sign in to view

sagudev mentioned this pull request Aug 28, 2024

Report overflow when shl overflows into sign bit #6175

Closed

ErichDonGubler changed the title ~~[naga wgsl] Implement local const declerations~~ [naga wgsl] Implement local const declarations Aug 28, 2024

teoxoy approved these changes Aug 30, 2024

View reviewed changes

teoxoy merged commit 34bb9e4 into gfx-rs:trunk Aug 30, 2024
25 checks passed

sagudev mentioned this pull request Aug 30, 2024

chore: Update wgpu to 34bb9e4ceb45a5b1cfc5df6aa2b2e201cc55372c servo/servo#33266

Merged

3 tasks

jimblandy added a commit to jimblandy/wgpu that referenced this pull request Sep 5, 2024

[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 mentioned this pull request Sep 5, 2024

[naga] Update docs for private items. #6218

Merged

teoxoy pushed a commit that referenced this pull request Sep 5, 2024

[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.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[naga wgsl] Implement local `const` declarations #6156

[naga wgsl] Implement local `const` declarations #6156

sagudev commented Aug 24, 2024 •

edited

Loading

This comment was marked as resolved.

This comment was marked as outdated.

sagudev commented Aug 27, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ErichDonGubler commented Aug 28, 2024 •

edited

Loading

sagudev commented Aug 28, 2024

teoxoy left a comment

[naga wgsl] Implement local const declarations #6156

[naga wgsl] Implement local const declarations #6156

Conversation

sagudev commented Aug 24, 2024 • edited Loading

This comment was marked as resolved.

This comment was marked as outdated.

sagudev commented Aug 27, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ErichDonGubler commented Aug 28, 2024 • edited Loading

sagudev commented Aug 28, 2024

teoxoy left a comment

Choose a reason for hiding this comment

[naga wgsl] Implement local `const` declarations #6156

[naga wgsl] Implement local `const` declarations #6156

sagudev commented Aug 24, 2024 •

edited

Loading

ErichDonGubler commented Aug 28, 2024 •

edited

Loading