gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.6k stars 922 forks source link

WGSL constant values should not be concretized #6232

Open jimblandy opened 2 months ago

jimblandy commented 2 months ago

I think this ought to pass validation, but it fails:

const one = 1;
const_assert one == 1.0;

When I run this on 50b71287c, I get:

Could not parse WGSL:
error: const_assert failure
  ┌─ const.wgsl:2:14
  │
2 │ const_assert one == 1.0;
  │              ^^^^^^^^^^ evaluates to false

This seems to cause WebGPU CTS failures:

webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
jimblandy commented 2 months ago

cc @sagudev

sagudev commented 2 months ago

const_assert works for:

const one = 1.0;
const_assert one == 1.0;

and for:

const_assert 1 == 1.0;

Now if we have:

fn f() -> i32 {
    const one = 1;
    if (one == 1.0) {
        return 42;
    } else {
        return 35;
    }
}

it compiles to:

fn f() -> i32 {
    if false {
        return 42i;
    } else {
        return 35i;
    }
}

so I think something is wrong with evaler (specifically how it handles abstract types - I think it's #4400).

sagudev commented 2 months ago

The problem is that abstractint is to eagerly converted to i32, then in binary_op we return result of I32(1) == AbstractFloat(1.0)

jimblandy commented 2 months ago

WGSL says constants can have abstract types. I don't think Naga implements that.

sagudev commented 2 months ago

WGSL says constants can have abstract types. I don't think Naga implements that.

Indeed, we try to concertize value:

https://github.com/gfx-rs/wgpu/blob/9b36a3e129da04b018257564d5129caff240cb75/naga/src/front/wgsl/lower/mod.rs#L1118