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.

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

WGSL constant values should not be concretized #6232

Open
jimblandy opened this issue Sep 6, 2024 · 5 comments
Open

WGSL constant values should not be concretized #6232

jimblandy opened this issue Sep 6, 2024 · 5 comments
Labels
area: naga front-end lang: WGSL WebGPU Shading Language naga Shader Translator type: bug Something isn't working

Comments

@jimblandy
Copy link
Member

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

const one = 1;
const_assert one == 1.0;

When I run this on 50b7128, 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 jimblandy added type: bug Something isn't working naga Shader Translator area: naga front-end lang: WGSL WebGPU Shading Language labels Sep 6, 2024
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Sep 6, 2024
…reviewers

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272
@jimblandy
Copy link
Member Author

cc @sagudev

i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Sep 7, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272
@sagudev
Copy link
Contributor

sagudev commented Sep 7, 2024

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
Copy link
Contributor

sagudev commented Sep 7, 2024

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

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 7, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272
@jimblandy
Copy link
Member Author

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

@sagudev
Copy link
Contributor

sagudev commented Sep 7, 2024

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

Indeed, we try to concertize value:

init = ectx.concretize(init)?;

jamienicol pushed a commit to jamienicol/gecko that referenced this issue Sep 9, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 13, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272
aosmond pushed a commit to aosmond/gecko that referenced this issue Sep 14, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Sep 16, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272

UltraBlame original commit: 80007d515fb2b3930c29519a6b4a53dcc904b733
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Sep 16, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272

UltraBlame original commit: 2119546af72280f5daaf86044154fb670a10db70
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Sep 16, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272

UltraBlame original commit: 80007d515fb2b3930c29519a6b4a53dcc904b733
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Sep 16, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272

UltraBlame original commit: 2119546af72280f5daaf86044154fb670a10db70
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Sep 16, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272

UltraBlame original commit: 80007d515fb2b3930c29519a6b4a53dcc904b733
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Sep 16, 2024
…hain-reviewers,webgpu-reviewers,nical,ErichDonGubler

Demote some tests to backlog (filed as gfx-rs/wgpu#6232):

- webgpu:shader,validation,const_assert,const_assert:constant_expression_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_and_assert:*
- webgpu:shader,validation,const_assert,const_assert:constant_expression_logical_or_assert:*

Differential Revision: https://phabricator.services.mozilla.com/D221272
@jimblandy jimblandy changed the title const_assert doesn't evaluate conditions properly WGSL constant values should not be concretized Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga front-end lang: WGSL WebGPU Shading Language naga Shader Translator type: bug Something isn't working
Projects
Status: Todo
Development

No branches or pull requests

2 participants