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

clarify when const-exprs subexpressions are evaluated #4558

Open
dneto0 opened this issue Apr 4, 2024 · 3 comments
Open

clarify when const-exprs subexpressions are evaluated #4558

dneto0 opened this issue Apr 4, 2024 · 3 comments
Labels
bug wgsl resolved Resolved - waiting for a change to the WGSL specification wgsl WebGPU Shading Language Issues
Milestone

Comments

@dneto0
Copy link
Contributor

dneto0 commented Apr 4, 2024

Example:

fn foo() {
   let x = 1;  // uses of x are  runtime-expressions, not const-expression
   let y =  x / 0;   // This should be an error. 
                          //  x/0 is a runtime expression.
}

The relevant part of the spec is at https://gpuweb.github.io/gpuweb/wgsl/#const-expr :

A const-expression E will be evaluated if and only if:

  • E is a subexpression of an expression OuterE, and OuterE will be evaluated, and evaluation of OuterE requires E to be evaluated.

There is confusion is for OuterE being x / 0. OuterE will be evaluated at runtime, not at shader-creation time. So OuterE will be evaluated, and so the RHS subexpression 0 will be evaluated to zero at shader-creation time, and then the division by zero should trigger a shader-creation error.

However, it's very easy to read the rule as OuterE having to be evaluated at shader-creation time in order for the evaluation of subexpression E to kick in. And if that's the reading, then the division rule can't be checked, and so won't trigger an error.

So we need to clarify that the OuterE doesn't have to be a const-expr for the rule to kick in.

We still want to check a bunch of shader-creation errors even in code that is in statements that will not execute.

fn foo() {
   let x = 1;
   if false {
       let y =  x / 0;   // This should be an error. 
   }
}

So we can't require OuterE to be reachable at runtime.

This is somewhat related to #4551

@dneto0 dneto0 added bug wgsl WebGPU Shading Language Issues labels Apr 4, 2024
@dneto0 dneto0 added this to the Milestone 1 milestone Apr 4, 2024
@dneto0
Copy link
Contributor Author

dneto0 commented Apr 4, 2024

I forgot to restate that we have a complicated rule like this in order to express the short-circuiting behaviour of && and ||
(But yeah, it's confusing as to what short-circuiting is protecting you from in the const-expr case.)

@dneto0
Copy link
Contributor Author

dneto0 commented Apr 4, 2024

FYI this was found via development of gpuweb/cts#3595 Thanks @Kangz !

@kdashg
Copy link
Contributor

kdashg commented Apr 17, 2024

WGSL 2024-04-16 Minutes
  • DN: Paired with 4551. Latest comment is on 4551 that shows examples to illustrate.
  • JB: This seems simply an oversight. Intention is once you know denominator is zero, regardless of division itself being const, we call it an error. Making it not care if the division is run-time or const was the consensus.
  • DN: That goes further then issue we've been discussing.
  • JB: I'm sorry.
  • DN: No, interesting and useful.
  • DN: c1, false && 1. Should not type check and it's a const expression and it's short circuting. Do we typecheck short-circuted should resolve if that's checked. My intuition is type checking happens all over the place.
  • BC: Jim is talking runtime / const expression.
  • JB: Sounds like 4551 not 4558
  • DN: Trying to conjoin the two.
  • BC: Can be separated. One is eval through short-circuit, other is div by 0
  • KG: These kinds of issues and complexities like if (false) { …. }. In that case the thing we're doing that we mandate we make a syntax(compile time) error not incredibly useful to devs. And is occasionally frustrating in that you get spurious compile errors because the code doesn't run. But because the code is bad in the branch we aren't running we fail to compile. Consider what we want is a signal to devs that we present the compile or warn? Same as with run-time expression it will compile and give you something
  • AB: If we go down that road it adds a compilation to frontends to know what they can't emit in their backends because underlying backends have type checking rules that would fail to validate the program. There is a cascade of behaviours where const is most restrive, then pipeline, then runtime. Typechecking exists in the first one and always happens. You always know the type. Don't see the benefit where you write bad code and change a value and it starts/stops working. An array of -1 size just doesn't make sense. Could have a rule that types must always be known.
  • KG: I think types should always be checked. Values I don't think should.
  • BC: Debate is types and value expressions are intertwined so separating them isn't easy. Array types are different if sizes are different and sizes can be derived from const variables so a type check isnt' easy unless you draw line on what gets value computation. So need a ruleset of RHS on short-circut for things like array counts. While array counts is one of few things we have now, should consider user function templates and all other things that come with it.
  • JB: Haven't finished reading issue, but has it been proposed that const-expressions as array lengs always be evaluated wherever they appear.
  • DN: That is one option, yes. It's completely consistent. It's consistent with my own reading of the spec but extra words could be used to clarify. Short circut section makes it sound liek you don't need to check at all. In my mind you always do type checking so always have to eval. And top-level const expressions are always evaluated. Could invent qualities of const-expr if in future versions. It isn't cutting us off from a future land of c++isms if we choose to do so.
  • JB: Sounds like types are always check regardless of where they appear has consensus. And if so, that implies array length is always evaluated.
  • KG: Yeah punt SFINAE in 2025?? :P
  • JP: Interested in other implementers. For Naga, are you happy with having to walk down RHS for type checking and not eval?
  • JB: Yes. we will always fully type check all of our expressions.
  • KG: because of 4551 resolution no longer an issue.

Later discussion:

  • JP: Looking at c0, should 1/0 be static rejected?

  • JB: First example should compile without eror because we did not eval so no exception

  • KG: If that's true, happy for that. Little weird you get better behaviour then in the if false case. but, eyes wide open that's weird, sure.

  • JP: The reason we allow c0. Div by 0 is a validation rule. But we don't apply the val rule because we don't do the division.

  • JB: I think it’s okay that if false { let _ = 1/0; } is an error, but false && 1/0 is not. The way we’ve set up WGSL, constant evaluation is a thing that applies to expressions. && is an expression, and if is not. If WGSL were an expression language, then it would make sense for if to also prevent the error, but WGSL is not an expression language.

  • DN: Thumbs up.

  • BC: Can extend argument to type

  • JB: No. There’s no precedent for doing that. In dynamic languages like JavaScript or Python, things are checked if/when you get there, but WGSL is not a dynamic language, we’re statically typed. Standard ML provides the model here: there’s an “elaboration” phase where you do stuff that doesn’t require values, that comes before the “evaluation” phase. WGSL is the same. Type checking is part of the elaboration phase. WGSL’s constant evaluation and override evaluation are new phases we’re introducing between elaboration and evaluation. But except in the wildly dynamic languages like JS and Python, there is no language that uses values to determine which subexpression to type check.

    @compute
    fn main() {
      let foo = false && dpdx(1.0) == 0.0;
    }
    
  • AB: Don't want to revisit type checking, want clarification on function call ones. Function calls are static call trees. So a derivative after false && is a validation error. I dont' think if this example is there. (See above example). dpdx is not a const function so would never be const-eval'd. But I think we should reject the dpdx there if it's in a compute shader tree. I think that was JPs point, we hadn't settled on that. Isn't the same as a value eval, it's a separate level of eval then what JB said was values.

  • JB: Think this is what folks talk about for effects systems. derivatives are other level. They have types that we type check but an additional requirement they can only happen in certain contexts regardless of values.

  • AB: Agree with that.
    JB: In deference to BC, C++ has that const if thing where they check based on stuff. Not entirely unprecedented but … awful.

  • KG: Think situations where that is useful is subfields we aren't targeting with wgsl. Useful for template meta programming

  • DN: Seen relevant cases where you have a fast pass for one architecture and one for a different one. Other level of consideration. Future we may want it, but design it when we get there.

  • JB: No existing consensus for anything like const-expr-if c++ thing. Committee has not discussed it.

@kdashg kdashg added the wgsl resolved Resolved - waiting for a change to the WGSL specification label Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug wgsl resolved Resolved - waiting for a change to the WGSL specification wgsl WebGPU Shading Language Issues
Projects
None yet
Development

No branches or pull requests

2 participants