Escape non-ASCII identifier characters with `write!(…, "u{:04x}", …)`,
surrounding with `_` as appropriate. This solves (1) a debugging issue
where stripped characters would otherwise be invisible, and (2) failure
to re-validate that stripped identifiers didn't start with an ASCII
digit.
I've confirmed that this fixes [bug
1978197](https://bugzilla.mozilla.org/show_bug.cgi?id=1978197) on the
Firefox side.
The HLSL external texture implementation didn't have to do anything in
particular to handle both coordinate types, as int2 automatically gets
promoted to uint2. But in MSL this is not the case, and it is
therefore important to test that we correctly handle both coordinate
types.
This adds several fields to `ExternalTextureDescriptor`, specifying
how to handle color space conversion for an external texture. These
fields consist of transfer functions for the source and destination
color spaces, and a matrix for converting between gamuts. This allows
`ImageSample` and `ImageLoad` operations on external textures to
return values in a desired destination color space rather than the
source color space of the underlying planes.
These fields are plumbed through to the `ExternalTextureParams`
uniform buffer from which they are exposed to the shader. Following
conversion from YUV to RGB after sampling/loading from the external
texture planes, the shader uses them to gamma decode to linear RGB in
the source color space, convert from source to destination gamut, then
finally gamma encode to non-linear RGB in the destination color space.
This adds HLSL backend support for `ImageClass::External` (ie WGSL's
`external_texture` texture type).
For each external texture global variable in the IR, we declare 3
`Texture2D` globals as well as a `cbuffer` for the params. The
additional bindings required by these are found in the newly added
`external_texture_binding_map`. Unique names for each can be obtained
using `NameKey::ExternalTextureGlobalVariable`.
For functions that contain ImageQuery::Size, ImageLoad, or ImageSample
expressions for external textures, ensure we have generated wrapper
functions for those expressions. When emitting code for the
expressions themselves, simply insert a call to the wrapper function.
For size queries, we return the value provided in the params
struct. If that value is [0, 0] then we query the size of the plane 0
texture and return that.
For load and sample, we sample the textures based on the number of
planes specified in the params struct. If there is more than one plane
we additionally perform YUV to RGB conversion using the provided
matrix.
Unfortunately HLSL does not allow structs to contain textures, meaning
we are unable to wrap the 3 textures and params struct variables in a
single variable that can be passed around.
For our wrapper functions we therefore ensure they take the three
textures and the params as consecutive arguments. Likewise, when
declaring user-defined functions with external texture arguments, we
expand the single external texture argument into 4 consecutive
arguments. (Using NameKey::ExternalTextureFunctionArgument to ensure
unique names for each.)
Thankfully external textures can only be used as either global
variables or function arguments. This means we only have to handle the
`Expression::GlobalVariable` and `Expression::FunctionArgument` cases
of `write_expr()`. Since in both cases we know the external texture
can only be an argument to either a user-defined function or one of
our wrapper functions, we can simply emit the names of the variables
for each three textures and the params struct in a comma-separated
list.
Adds a new flag to the IR indicating when image sample coordinates are
to be clamped. Adds wgsl-in support for parsing and lowering to
IR. Validation ensures this flag is only used when sampling a 2D
non-arrayed sampled texture, without offset, gather, or depth
comparison. This matches the WGSL requirements, with the exception of
supporting `texture_external` textures, which will follow in a later
patch.
SPIRV, HLSL, and Metal backends are supported so far, with GLSL left
for a follow up. (In GLSL the texture will simply be sampled without
the coordinates being clamped.)
It may seem unfortunate to have to handle this separately for each
backend, and indeed it would have been possible to implement this simply
in the WGSL frontend. However, future patches will add support for using
textureSampleBaseClampToEdge() with external textures, which will
actually have to be handled by each backend. This patch is laying the
groundwork for that.
Emit optimized code for `dot4{I, U}8Packed` regardless of SPIR-V version
as long as the required capabilities are available. On SPIR-V < 1.6,
require the extension "SPV_KHR_integer_dot_product" for this. On
SPIR-V >= 1.6, don't require the extension because the corresponding
capabilities are part of SPIR-V >= 1.6 proper.
Closes#7481.
This implementation roughly follows approach 2 outlined in #7481, i.e.,
it adds a polyfill for the signed and unsigned dot product of packed
vectors for each platform. It doesn't use the specialized instructions
that are available for this operation on SPIR-V (with capability
DotProductInput4x8BitPacked).
As we know that minimum value integer literals can cause problems for
some compilers. (See #7437)
Make the code which generates these functions call
msl::Writer::put_literal() and hlsl::Writer::write_literal()
respectively to output the minimum value integer literals instead of
just writing them directly, ensuring we only have to handle this
workaround in a single location (per backend).
As we did for MSL in #7437. eg `-2147483648` becomes `-2147483647 - 1`.
Neither FXC nor DXC currently have any issues parsing the most negative
value literals. However, we have been informed this is not guaranteed to
always be the case, so are making this change as a precaution.
The literal `-2147483648` is parsed by Metal as negation of positive
2147483648. As 2147483648 is too large for a int, the expression is
silently promoted to a long. Sometimes this does not matter as it will
often be implicitly converted back to an int after the negation.
However, if the expression is used in a bitcast then we hit a compiler
error due to mismatched bitwidths.
Similarily for `-9223372036854775808`, as 9223372036854775808 is too
large for a long, metal emits a `-Wconstant-conversion` warning and
changes the value to -9223372036854775808. This would then be negated
again, possibly causing undefined behaviour.
In both cases we can avoid the issue by expressing the literals as the
second most negative value expressible by the type, minus one.
eg `-2147483647 - 1` and `-9223372036854775807L - 1L`.
We have added a test which uses the most negative i32 literal in an
addition. Because we bitcast addition operands to unsigned in metal,
this would cause a validation error without this fix. For the i64 case
existing tests already make use of the minimum literal value. Passing
the flag `-Werror=constant-conversion` to Metal during validation will
therefore catch this issue.
In #7424 we fixed a bug where the representation of the minimum int64
literal generated by naga was invalid WGSL. It changed us from
expressing it as `-9223372036854775808` which was invalid, to
`-9223372036854775807li - 1li`.
This is valid WGSL. However, as the values are concrete i64 types if the
shader is parsed again by naga the expression does not get const
evaluated away, leading to suboptimal code generated by the
backends. This patch makes us perform the subtraction using abstract
integers before casting to i64, solving this problem.
Additionally the input WGSL test is updated to use the same construct.
Currently we generate code to convert floating point values to integers
using constructor-style casts in HLSL, static_cast in MSL, and
OpConvertFToS/OpConvertFToU instructions in SPV. Unfortunately the
behaviour of these operations is undefined when the original value is
outside of the range of the target type.
This patch avoids undefined behaviour by first clamping the value to
be inside the target type's range, then performing the cast.
Additionally, we specifically clamp to the minimum and maximum values
that are exactly representable in both the original and the target
type, as per the WGSL spec[1]. Note that these may not be the same as
the minimum and maximum values of the target type.
We additionally must ensure we clamp in the same manner for
conversions during const evaluation. Lastly, although not part of the
WGSL spec, we do the same for casting from F64 and/or to I64 or U64.
[1] https://www.w3.org/TR/WGSL/#floating-point-conversion
This avoids a panic due to f16::to_u32()/f16::to_u64() returning None
when the value of the f16 is <= -1.0. The correct behaviour when
converting from a floating point to an integer when the value is out
of range is to clamp to the nearest value that is representable in
both the source and destination type. ie zero for negative numbers.
ie the second-most negative value minus 1.
The most negative value of an integer type is not directly expressible
in WGSL, as it relies on applying the unary negation operator to a
value which is one larger than the largest value representable by the
type.
To avoid this issue for i32, we negate the required value as an
AbstractInt before converting to i32. AbstractInt, being 64 bits, is
capable of representing the maximum i32 value + 1.
However, for i64 this is not the case. Instead this patch makes us
express the mimimum i64 value as the second most negative i64 value,
minus 1, ie `-9223372036854775807li - 1li`, thereby avoiding the
issue.
Add support for `naga::ir::MathFunction::Cross` to
`naga::proc::constant_evaluator`.
In the tests:
- Change `naga/tests/in/wgsl/cross.wgsl` to use more interesting
argument values. Rather than passing the same vector twice, which
yields a cross product of zero, pass in the x and y unit vectors,
whose cross product is the z unit vector. Update snapshot output.
- Replace `validation::bad_cross_builtin_args` with a new test,
`builtin_cross_product_args`, that is written more in the style of
the other tests in this module, and does not depend on the WGSL
front end. Because this PR changes the behavior of the constant
evaluator, this test stopped behaving as expected.
- In `wgsl_errors::check`, move a `panic!` out of a closure so that
the `#[track_caller]` attribute works properly.
[naga spv-out msl-out hlsl-out] Make infinite loop workaround count down instead of up
To avoid generating code containing infinite loops, and therefore
incurring the wrath of undefined behaviour, we insert a counter into
each loop that will break after 2^64 iterations. This was previously
implemented as two u32 variables counting up from zero.
We have been informed that this construct can cause certain Intel
drivers to hang. Instead, we must count down from u32::MAX. Counting
down is more fun, anyway.
Co-authored-by: Erich Gubler <erichdongubler@gmail.com>
Makes the dual source implementation in wgpu WebGPU spec compliant.
Furthermore, makes the dual source blending extension available when targeting WebGPU.
This allows abstract-typed expressions to be used for some or all of
the switch selector and case selectors. If these are all not
convertible to the same concrete scalar integer type we return an
error. If all the selector expressions are abstract then they are
concretized to i32.
The note previously provided by the relevant error message, suggesting
adding or removing the `u` suffix from case values, has been
removed. While useful for simple literal values, it was comically
incorrect for more complex case expressions. The error message should
still be useful enough to allow the user to easily identify the
problem.
Instead allow the const to be converted each time it is const
evaluated as part of another expression. This allows an abstract const
to be used as a different type depending on the context.
As a result, abstract types may now find their way in to the IR, which
we don't want. This occurs because the compact pass treats named
expressions as used, mostly so that our snapshot tests are more
useful, and therefore does not remove them. To prevent this, we avoid
adding abstract-typed local consts to the named expressions list. This
will have no functional effect on any shaders produced by the
backends, but some unused local const declarations will no longer be
present.
When an entry point's return type is anonymous, have
`naga::proc::Namer` assign it a name based on its shader stage.
Remove bespoke logic for this from the WGSL backend.
Fixes#7263.
Parsing currently fails for shaders that attempt to dynamically index
an abstract-typed array (or vector, etc), like so:
var x = array(1, 2, 3)[i];
This is caused by attempting to concretize the Expression::Access
expression, which the ConstantEvaluator fails to do so due to the
presence of a non-constant expression.
To solve this, this patch concretizes the base type *prior* to
indexing it (for non-constant indices), meaning the constant evaluator
never sees any non-constant expressions. This matches the WGSL
specification:
When an abstract array value e is indexed by an expression that is
not a const-expression, then the array is concretized before the
index is applied.
(Similar applies for both vectors and matrices, too.)
This may be somewhat non-optimal in that if there are multiple
accesses of the same abstract expression, we will produce duplicated
concretized versions of that expression. This seems unlikely to be a
major issue in practice, and we can always improve this if and when we
encounter a real issue caused by it.
Instead allow the const to be converted and each time it is const
evaluated as part of another expression. This allows an abstract const
to be used as a different type depending on the context.
A consequence of this is that abstract types may now find their way to
the validation stage, which we don't want. We therefore additionally
now ensure that the compact pass removes global constants of abstract
types. This will have no *functional* effect on shaders generated by
the backends, as the expressions belonging to the abstract consts in
the IR will not actually be used, as any usage in the input shader
will have been const-evaluated away. Certain unused const declarations
will now be removed, however, as can be seen by the effect on the
snapshot outputs.
