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

Support diagnostic filters in WGSL (i.e., diagnostic(…)) #5320

Open
12 tasks
ErichDonGubler opened this issue Feb 29, 2024 · 7 comments · May be fixed by #6148 or #6353
Open
12 tasks

Support diagnostic filters in WGSL (i.e., diagnostic(…)) #5320

ErichDonGubler opened this issue Feb 29, 2024 · 7 comments · May be fixed by #6148 or #6353
Assignees
Labels
area: correctness We're behaving incorrectly kind: diagnostics Error message should be better lang: WGSL WebGPU Shading Language naga Shader Translator

Comments

@ErichDonGubler
Copy link
Member

ErichDonGubler commented Feb 29, 2024

We currently do not support the @diagnostic(…) attribute/diagnostic(…) directive (AKA "diagnostic filters" in the WebGPU spec.). This is a rather complicated subsystem, but it's exciting to have robust management of diagnostics in WGSL a la Rust. An example of how this is used from the spec.:

EXAMPLE: Global diagnostic filter for derivative uniformity

diagnostic(off,derivative_uniformity);
var<private> d: f32;
fn helper() -> vec4<f32> {
  if (d < 0.5) {
    // The derivative_uniformity diagnostic is disabled here
    // by the global diagnostic filter.
    return textureSample(t,s,vec2(0,0));
  } else {
    // The derivative_uniformity diagnostic is set to 'warning' severity.
    @diagnostic(warning,derivative_uniformity) {
      return textureSample(t,s,vec2(0,0));
    }
  }
  return vec4(0.0);
}

Relevant part of the spec.: Diagnostics, section 2.3

An initial implementation of this should include the derivative_uniformity rule, as the only current example of filterable trigger rules that can be managed per the spec.

Parse paths that we need to support to be spec.-compliant:

  • Directive: WGSL: Parse diagnostic(…); directives #6148
  • Attributes placements (pasted from the spec.):
    • Beginning of a compound statement.
    • Beginning of a function declaration: Parse diagnostic(…) attributes on fns #6353
    • Beginning of an if statement.
    • Beginning of a switch statement.
    • Beginning of a switch_body.
    • Beginning of a loop statement.
    • Beginning of a while statement.
    • Beginning of a for statement.
    • Immediately before the opening brace ('{') of the loop body of a loop, while, or for loop.
    • Beginning of a continuing_compound_statement.
@ErichDonGubler ErichDonGubler added area: correctness We're behaving incorrectly naga Shader Translator lang: WGSL WebGPU Shading Language kind: diagnostics Error message should be better labels Feb 29, 2024
@ErichDonGubler
Copy link
Member Author

Correspondent Firefox bug: bug 1882800

@jimblandy
Copy link
Member

This is blocking some BabylonJS demos.

@Mutefish0
Copy link

This is blocking some Three.js demos.

@ErichDonGubler ErichDonGubler linked a pull request Aug 23, 2024 that will close this issue
11 tasks
@jimblandy
Copy link
Member

jimblandy commented Aug 26, 2024

I think we may want to scope this issue down to simply supporting the syntax, and including the data in the naga::Module somehow, where uniformity analysis could use it, once it's implemented.

We don't have any warnings in Naga right now. Adding the infrastructure for reporting them and filtering them will be a large project, for something we don't even do yet.

This issue's urgency comes from the fact that we can't parse valid shaders, so I think clearing that specific hurdle will be enough.

@jimblandy
Copy link
Member

After discussion this morning, we agreed that this issue would remain as filed, and we'd file separate blocker issues for the sub-issues that we want to address immediately.

@ErichDonGubler ErichDonGubler linked a pull request Oct 2, 2024 that will close this issue
14 tasks
@ErichDonGubler ErichDonGubler changed the title Support diagnostic filters in WGSL (i.e., the @diagnostic(…) attribute) Support diagnostic filters in WGSL (i.e., diagnostic(…)) Oct 4, 2024
@floooh
Copy link

floooh commented Nov 5, 2024

I'm also running into this btw (FF's dev console pointed me to this issue)

https://floooh.github.io/sokol-webgpu/

(I'm generating WGSL from SPIRV, and to increase compatibility with existing shaders I need to turn off the uniformity diagnostic)

@ErichDonGubler
Copy link
Member Author

@floooh: Looks like you're using filtering rules in directive position (i.e., diagnostic(…); at the top of the file). That should be resolved with #6148! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: correctness We're behaving incorrectly kind: diagnostics Error message should be better lang: WGSL WebGPU Shading Language naga Shader Translator
Projects
Status: In Progress
4 participants