wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard
verified publisher icon gfx-rs

[naga wgsl] Implement local `const` declarations

Open sagudev opened this issue 1 year ago • 9 comments

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

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

sagudev avatar Aug 24 '24 12:08 sagudev

Error validating tests/out/wgsl/functions.wgsl: Could not parse WGSL: error: Not implemented as constant expression: Dot built-in function ┌─ tests/out/wgsl/functions.wgsl:11:18 │ 11 │ const c_2_ = dot(a_2_, b_2_); │ ^^^ see msg

I think the problem is that ExpressionKindTracker detects dot as const, but constevaler doesn't: https://github.com/gfx-rs/wgpu/blob/fac49ee97c5dd8e37419d425e4cdf79fbe760f8f/naga/src/proc/constant_evaluator.rs#L1243

sagudev avatar Aug 25 '24 07:08 sagudev

rebased!

sagudev avatar Aug 27 '24 10:08 sagudev

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 avatar Aug 27 '24 17:08 sagudev

webgpu:shader,validation,expression,access,vector:abstract:* failures are https://github.com/gfx-rs/wgpu/issues/4400 webgpu:shader,validation,expression,binary,bitwise_shift:shift_left_concrete:* fails because https://github.com/gfx-rs/wgpu/issues/6175

sagudev avatar Aug 27 '24 19:08 sagudev

@sagudev: I suspect that const. eval. is actually rejecting that code properly, but we're not treating the error as fatal in WGSL parsing. I can only find checked_{shl,shr} calls for bit shifting operations in naga/src/proc/constant_evaluator.rs: https://github.com/gfx-rs/wgpu/blob/5deaef3b67259008844b15e1fd1d3a8924d1cde0/naga/src/proc/constant_evaluator.rs#L1782-L1815

ErichDonGubler avatar Aug 27 '24 20:08 ErichDonGubler

I suspect that const. eval. is actually rejecting that code properly, but we're not treating the error as fatal in WGSL parsing

That would be true, if I didn't write < (less) operator instead of << (shift left) operator, my bad.

Now const c = 1073741824i << 1u; produces const c: i32 = i32(-2147483648);, instead of error.

sagudev avatar Aug 28 '24 05:08 sagudev

Per https://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.

sagudev avatar Aug 28 '24 05:08 sagudev

@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 😅: https://github.com/gfx-rs/wgpu/issues/6175

ErichDonGubler avatar Aug 28 '24 13:08 ErichDonGubler

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

Right.

sagudev avatar Aug 28 '24 13:08 sagudev