Closed ben-clayton closed 2 years ago
Open question I don't think we figured out in the meeting:
I think @jdashg and I are trying to advocate for the following, which has been fleshed out with a bunch of more informed research:
Currently (I think), const
outside functions (global_constant_decl
) exactly corresponds to (the result of) OpConstant
or OpConstantComposite
. Function-scope const
(variable_statement
) is completely different; it corresponds to the result of a value-producing op (load, add, etc.)
We should change the outside keyword to constexpr
and keep the (non-spec-constant*) functionality exactly the same.
And I think we need Ben's addition above to allow constexpr
names to be used inside other constexpr
s (it's actually necessary, to be able to directly represent a lot of SPIR-V, I'm pretty sure, so this is somewhat necessary and entirely orthogonal).
[[constant_id]]
): spec constants with IDs must be scalar, while others are just constants that happen to be built up from other specialization constants (and may be scalar, OpSpecConstantComposite, or OpSpecConstantOp).Then, we should consider allowing constexpr
inside functions as well, for convenience (unless spir-v doesn't allow that, maybe; but we could do it even if it doesn't).
Thank you for writing this! As I stated on the call, I think we need to solve this local vs global constants difference before considering the const expression semantics.
unless spir-v doesn't allow that, maybe; but we could do it even if it doesn't
In SPIR-V the constants are declared at the beginning of the module. They aren't local to functions.
I think specialization constants need to become more distinct, with another separate keyword
I don't see how the story is leading there. If we want the module-scope constants to become OpConstant
in SPIR-V, it seems only reasonable to keep the approach we have now where some of these are annotated as specialization constants. What's the problem you are trying to solve by separating the two?
The "constexpr" name is a bit confusing if it applies to spec constants. It seems to me that we should just keep the "const" of the module scope, and rename the function-scope "const".
Not all specialization constants have IDs
True, but that's on SPIR-V side. It may very well be an implementation detail of the shader translation, this doesn't necessarily affect how we define WGSL.
I can confirm this:
Currently (I think), const outside functions (global_constant_decl) exactly corresponds to (the result of) OpConstant or OpConstantComposite. Function-scope const (variable_statement) is completely different; it corresponds to the result of a value-producing op (load, add, etc.)
As to this question:
Can texture sample offset be a specialization constant?
Yes it can (in SPIR-V Vulkan). The offset parameter in the texture builtins corresponds to the ConstOffset image operand. It says:
A following operand is added to (u, v, w) before texel lookup. It must be an <id> of an integer-based constant instruction of scalar or vector type. It is invalid for these to be outside a target-dependent allowed range
That defined term "constant instruction" is:
Constant Instruction: Either a specialization-constant instruction or a non-specialization constant instruction: Instructions that start "OpConstant" or "OpSpec".
So: it's specializable either directly (in the scalar case) or indirectly computed via OpSpecConstantComposite or OpSpecConstantOp.
NOTE: This is the baseline functionality in Vulkan. There is extra optional functionality:
I'll restate here something I said in the meeting: the various kinds of constant-ness maps to the value being finalized/fixed at different phases in the shader lifecycle:
constant_id
attribute.
I've pointed out the missing bits of expressiveness. That's where a constexpr-kind of feature can fit.
In terms of validation, it follows that when adding the ability to form expressions, the terms in an expression at a particular phase can only come from the same phase or earlier phases.
In Tint for:
* when a name comes into scope
'const' in function scope. For example, a 'const' declared inside a loop can have a different value in each iteration. But while that instance is still in scope, it retains the same value. (Like @kainino0x said, it corresponds to a singly-assigned value, an SSA ID)
If the initialize is a Scalar or Type constructor then we'll generate an OpConstant. For any other type of initializer we use OpVariable as we have to set it at runtime. So, in the case where the initializer is const_expr
we treat the const inside a function as the same as outside a function.
About naming:
We had mentioned the "go with the JS way of doing things" as good default. That works for 'const'. But JS doesn't have those 4 phases of const-ness. So it's inevitable that we'll have to go beyond JS.
So, in the case where the initializer is const_expr we treat the const inside a function as the same as outside a function.
Right, when the translator sees that values are more fixed than they could be, it has the option of bubbling up the definition of that value up to a category in an earlier phase.
Can texture sample offset be a specialization constant?
Because this is being asked again, I want to reiterate - even if it is supported, I don't think we should support it for offset
. Being able to do compile-time validation on the offsets allows us to avoid expensive runtime validation. Performing that validation at pipeline creation time is going to be really messy.
Whatever the solution (constexpr
or otherwise), I feel it should not support spec-constants. Otherwise we'd have to propagate some flag to indicate whether the variable could originate from a spec-constant, and this is entirely hidden from the grammar. Having a unique keyword for compile-time-constant-and-not-spec-constant is the ideal for offset
. I don't really mind what that thing is called.
@ben-clayton this is a great problem to consider!
even if it is supported, I don't think we should support it for offset. Being able to do compile-time validation on the offsets allows us to avoid expensive runtime validation.
I have 2 points that counter this argument:
createShaderModule
. There are reasons, one of them is, for example, that some WGSL entry points may use capabilities that aren't supported by the logical device. In WebGPU we considered this to be a valid use case, but SPIR-V would complain and fail to load in vkCreateShaderModule
. There are other reasons, too - a topic for another discussion, but the point is that this may be required, and this would be very much in line with what we do for MSL and DXIL anyway.OpSpecConstantOp
sequence to clamp it to 8, using a combination of OpUGreaterThan
and OpSelect
. This wouldn't require any run-time checks.TL;DR: specializing the texture offsets is a valid case that we can and should support
Edit: the reason for (1) is not valid. We could filter the entry points right away. There are other reasons though, like https://github.com/gpuweb/gpuweb/issues/1064#issuecomment-733067687
If we detect that a specialization constant is used for the offset, we could insert a
OpSpecConstantOp
sequence to clamp it to8
, using a combination ofOpUGreaterThan
andOpSelect
. This wouldn't require any run-time checks
I still believe that erroring is preferable to clamping, as going OOB is almost certainly not what the developer intended, and would likely be masking a bug. The difference in behavior between compile-time-error when non-spec-constant vs clamping when spec-constant a bit jarring, but I could live with it, I guess.
What are peoples thoughts on doing:
| CONST variable_ident_decl EQUAL const_expr
| FIXED variable_ident_decl EQUAL short_circuit_or_expression
Where a FIXED
is set to a specific value at creation and cannot be changed.
It would be great to have different names for "inside-const" and "outside-const", and it seems like "const" and "constexpr" respectively would fit the bill here. There are a bunch of real concerns about constexpr ballooning into something hard to implement, but I don't think we need sophisticated arbitrary-expressions-and-function-calls constexpr yet. constexpr INDENTIFIER
would be initialized a (grammar) const_expr, and then that ident could be used anywhere where we require const_expr today.
It would be great to have different names for "inside-const" and "outside-const", and it seems like "const" and "constexpr" respectively would fit the bill here.
Thanks to the discussion around offset
, we're now aware of the various definitions of const
based on lexical scope and what's on the RHS of the assignment. However, until the limitations on offset
became apparent, this was mostly an implementation detail and IIUC had no significant semantic significance to the developer aside from "this variable is immutable" (with possible exception to spec-ops).
Renaming const
based on lexical scope may help differentiate what's going on in the SPIR-V backend, but does it really help the developer writing in the language? I have concerns around renaming const
based on lexical scope, as this pushes the complexity burden on to our users, perhaps needlessly. Maybe I'm wrong, and the backend differences are significant enough to warrant different keywords for different lexical scope declarations, but I don't think we've proved that yet.
There are a bunch of real concerns about constexpr ballooning into something hard to implement, but I don't think we need sophisticated arbitrary-expressions-and-function-calls constexpr yet.
I agree this isn't necessary for MVP, but I start to have doubts about my own constexpr
proposal if we're not intending to support arithmetic (at the very least) at some point. As mentioned in the call this week, a compile-time-const that can, and will only ever contain a trivial literal-like RHS is as good as a scoped AST substitution. A preprocessor #define
is a kludgey scopeless way of giving a literal-like value a name. Even putting a comment on the texture-call argument isn't far from the limited benefits of a named literal-like value. If we really want a way to declare a variable with literal-like value and never wish to treat it as a true expression, then I agree it shouldn't be called constexpr
. Maybe alias foo = 42;
or something is better.
With that said, I'm very much for having a constexpr
variable that supports arithmetic, one day. I've lost count the number of times in my career that I've written generators to inject precomputed math into shader code (Poisson Disc offsets being a good example), and I'd have loved to have the ability to calculate this stuff without writing a bunch of bespoke and bug-prone generator tooling.
In a way, I don't care about the implementation details of constexpr (outside of "is it feasibly implementable"), and my goal is not to surface here what's going on in some conceptual spir-v equivalent. Rather, I think it's valuable to give two different concepts, that have different constraints and different valid usages, different names. Given that a very limited constexpr (or consteval), like we apparently already have for const_expr for composite (vec) type declarations, really matches well what we want here, is feasible and not onerous to implement, and is forward-compatible with expanding what we expect implementations to calculate at compile time in the future, we should move that direction.
This is in contrast to function-level const, which is just an immutable variable declaration, and is not constant for the purposes of operations that require constants, like ConstOffset for image loads, which must be a for-real compile-time constant (or spec-const), not an immutable-marked local variable.
Effectively, we already have some form of constexpr for module-const, so it's too late to avoid adding a constexpr, because we already have one, or something very much like it. Rather, it'd be great to have a more clearly distinct name for these, such as constexpr
, consteval
, or static const
. I'm not proposing any changes here to what module-const can do, just a renaming in line with the original post here, and I think we're largely getting sidetracked with discussing implications of what constexpr some day might be, rather than making our situation a little bit more clear today.
@jdashg thoughts on the const
and fixed
I suggested above? @kvark I don't know if eyes
is a good emoji response or a bad one, heh.
Effectively, we already have some form of constexpr for module-const, so it's too late to avoid adding a constexpr, because we already have one, or something very much like it. Rather, it'd be great to have a more clearly distinct name for these, such as
constexpr
,consteval
, orstatic const
.
+1.
On the topic of static, I would like to use it, because it's a very apt word for what we mean, except that its meaning has been so heavily overloaded by C, C++, Java, and so on.
I'm not proposing any changes here to what module-const can do
I wasn't originally intending to propose this either, but in investigating specialization constants I became convinced that the model we have today is flawed (see above); maybe I'm wrong.
Effectively, we already have some form of constexpr for module-const, so it's too late to avoid adding a constexpr, because we already have one, or something very much like it. Rather, it'd be great to have a more clearly distinct name for these, such as constexpr, consteval, or static const.
Okay, I agree with this. You're right that module-scope const isn't just-an-immutable var
as I had suggested (it cannot be assigned a non const_expr
), and if the module-scope const
looks like a compile-time-const-or-spec-op, swims like a compile-time-const-or-spec-op and quacks like a compile-time-const-or-spec-op, it should probably be called a compile-time-const-or-spec-op (constexpr
, consteval
, static const
, fixed
, or something else).
That said, allowing only a compile-time-const-or-spec-op at module scope, and const
only at function scope is still less than ideal, and I'd prefer to be consistent and allow both regardless of scope. This can be discussed as a followup.
I'm not proposing any changes here to what module-const can do, just a renaming in line with the original post here, and I think we're largely getting sidetracked with discussing implications of what constexpr some day might be, rather than making our situation a little bit more clear today.
The reason I brought this up is that @dj2 has expressed a strong desire to never support expressions that go much beyond what const_expr
is today, and so is not keen on the term constexpr
as this is suggestive of something more flexible than what we'll ever have. I assume this is why you're keen on fixed
, @dj2 ?
To reiterate my position, I'm very keen on supporting at least arithmetic for these at some point in the future, but if that's never going to happen, I agree constexpr
/ consteval
are bad names.
@kvark - I've given https://github.com/gpuweb/gpuweb/issues/1272#issuecomment-741866589 more thought, and I think I'm okay with offset
coming from a spec-op with the clamping as you suggested. If we're to do this, I'd recommend keeping the clamping behaviour consistent with non-spec-op, and removing the proposed compile time error from the spec, but have the compiler produce a warning (e.g. warning: offset.x evaluates to 10, which is outside the allowed limits of [-8 to 7]. Clamping offset.x to 7
). This keeps behaviour consistent regardless of value origin, and still alerts the developer that they're likely doing something stupid for the more common case of a literal-like in the shader source.
Partly, I think fixed
is a good name for the concept and I've never really liked constexpr
. Even if it was calculable (which as I mentioned I'd prefer not to do) the value will still be fixed
at compile time.
@dj2 I don't have strong feelings about "fixed", hence the eyes emoji (aka - acknowledged). I do agree that it's an interesting choice of a word that is clear, yet not confusing with regards to prior art in let
,val
,var
,const
, etc. It's definitely better than what we have today :)
Caught up on this.
fixed
is I need to know which of the 4 levels of fixed-ness is being described. See aboveI have an use case to support this constexpr
when I'm writing switch-case
statement in below:
constexpr ALPHA_MODE_DONT_CHANGE : u32 = 0u;
constexpr ALPHA_MODE_PREMULTIPLY : u32 = 1u;
constexpr ALPHA_MODE_UNPREMULTIPLY: u32 = 2u;
switch (uniform.alphaOp) {
case ALPHA_MODE_DONT_CHANGE: {
break;
}
....
}
This will improve the code readability a lot if we support constexpr
and allow it to be in 'case' selector.
Here's a proposal:
All expressions are either constexpr
or non-constexpr
.
constexpr
An expression is constexpr
iff all the leaves in its AST are constexpr
, and all the function calls in its AST are to constexpr functions.
Example: sqrt(myConstexpr[4][myOtherConstexpr].foo * 5)
is constexpr
.
Indicate a constexpr
variable by a new variable keyword: constexpr
:
var aVariable: i32 = 0;
let aConstant: i32 = 1; // Not a constexpr
constexpr aConstexpr: i32 = 1;
(Rationale: This is a natural extension from the var
vs let
divide we have today.)
For future expansion, mark constexpr
as a reserved word, to make sure users can't for example make a struct named "constexpr".
Variables which do not use this keyword in their declaration are not constexpr
.
A constexpr
variable must be initialized to a constexpr
expression. Once an author has a constexpr
variable, the rules for using it follow the rules for let
variables, with some exceptions (more on this below).
(For now,) all user-defined functions are non-constexpr
.
A function is constexpr
iff it is in one of these categories of built-in functions:
A constexpr
expression (including all its sub-expressions) must be a "constructible" type:
Exceptions to the rules for let
variables are:
constexpr
expressionconstexpr
expressionsoffset
parameters to texture sampling operations may be constexpr
expressionsvar
matrix/array may be a constexpr
expressionconstexpr
expressions do not cause observable dead code elimination.
Note: The fact that the rules for using constexpr
variables follow the rules for let
variables means constexpr
variables are lexically scoped, and their names can shadow each other.
All current non-overridable let
variables at module scope already follow these rules above, and therefore could be promoted to constexpr
variables. Let's make a rule that all let
variables at global scope must be overridable - if an author wants a non-overridable let
, they can always use the strictly-more-powerful constexpr
instead.
Thank you for putting this up! I'm a bit lacking background on this discussion so excuse if the following seems misguided.
First WGSL currently doesn't use const
so maybe that could be used as the keyword instead of constexpr
. We also might want to have user-defined constant evaluated functions at some point and that could use const fn
instead of just fn
.
The other thing is that we have a lot of different volumes of space-time at which the value of variables is fixed:
createShaderModule
time. This proposal.createPipeline
time: pipeline overridable constants.For the most part the list above seems to be in subtyping order: "uniform" variables are "workgroup uniform". In defining constexpr variables it would be great to also find a consistent way to talk about the various types above so that developers can make type assertions but also so that the specification can use the types to define the uniformity analysis. The developer could also add uniformity annotations to the arguments of function to help guide the uniformity analysis.
(thinking about it, const fn
will be great as the result will be just as good as the worst of their arguments)
We already agreed to have some form of optional annotations for uniform variables: https://github.com/gpuweb/gpuweb/issues/1791 (although I still have to propose a spec change for it).
Making it part of the same hierarchy as const/let/var is an interesting idea, although I am not sure I am in favor. One difference between these concepts is that (currently at least), we are making const/let/var a non-optional thing (we won't try to infer that some var is actually constexpr and thus can be used that way), whereas we are trying our best to infer uniformity and only need annotations to make it more precise at the global level.
I made one quick edit to the proposal above: instead of saying things like array sizes can be constexpr variables, it now says array sizes can be constexpr expressions. If we support constexpr, we ought to be able to support array<i32, 1 + 1>.
@RobinMorisset agreed that the time (constexpr, pipelineexpr,
See https://github.com/gpuweb/gpuweb/issues/732#issuecomment-623923613 for interesting issues/questions related to when expressions may be or are required to be folded. (from @johnkslang)
Google is happy with @litherum's proposal in conjunction with the discussion in #2238.
Something to watch for:
Any constexpr parts of attributes on a declaration must not depend on the declaration itself (directly or indirectly). I think this is already covered by the spec, because the attributes on a declaration form a part of the definition of the declaration. (And if that's not clear then we should make it clear).
One example is:
[[stage(vertex)]]
fn constexpr vert( // made up constexpr syntax for illustration
[[location(i32(vert(5).x))]] f: f32 // What's that location?!!
) -> [[builtin(position)]] vec4<f32> {
return vec4<f32>(f);
}
One thing we can do to keep things a bit simple is to say that an entry point function must not be constexpr.
Fixed by #2668
Discussed at the 2020-11-30 meeting.
https://github.com/gpuweb/gpuweb/pull/1238 has highlighted the desire compile-time-constant variables.
The
textureSample()
intrinsics that take anoffset
parameter require the offset value to be known at compile time. Aconst
variable might be compile-time-known, but is not guaranteed.const
at module scope might suffice foroffset
's needs, but permittingconst
based on lexical scope is confusing, and specializations may break this anyway.We may wish to consider adding
constexpr
as a new variable specifier, which can only be assigned aconst_expr
expression.variable_statement
would then look something like:Exampes:
The current grammar of
const_expr
does not allow for identifiers, soconstexpr d : vec2<i32> = c;
would not currently be valid.const_expr
could be altered to:Where the
IDENT
needs to resolve to aconstexpr
variable, allowing for: