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.
It must call ConstantEvaluator::check_and_get() to possibly retrieve
the constant expression from a separate arena, like is currently done
when evaluating `As` expressions for non-array casts.
Move the code to generate the definition of an overflow-safe
divide/modulo SPIR-V function into its own Rust function, to reduce
indentation and clarify influences. This commit isn't intended to
cause any change in behavior.
Explain we need the wrapper functions not just to avoid undefined
behaviour, but also to ensure we adhere to the WGSL spec. Additionally
link to issue #7109 in cases where our workaround needs follow-up work
for non-32-bit integer types.
Explain we need the wrapper functions not just to avoid undefined
behaviour (or unspecified in the case of division), but also to ensure
we adhere to the WGSL spec.
Replace the link to the resolved WGSL spec issue about floating-point
division by zero (gpuweb/gpuweb#2798) with links to the Direct3D 11
functional specification (which Direct3D 12 inherits) and the DXIL
specification, explaining that HLSL does what WGSL wants here.
Integer division or modulo is undefined behaviour in SPIR-V when the
divisor is zero, or when the dividend is the most negative number
representable by the result type and the divisor is negative one.
This patch makes us avoid this undefined behaviour and instead ensures
we adhere to the WGSL spec in these cases: for divisions the
expression evaluates to the value of the dividend, and for modulos the
expression evaluates to zero.
Similarily to how we handle these cases for the MSL and HLSL backends,
prior to emitting each function we emit code for any helper functions
required by that function's expressions. In this case that is helper
functions for integer division and modulo. Then, when emitting the
actual function's body, if we encounter an expression which needs
wrapped we instead emit a function call to the helper.
Emit helper functions for MathFunction::Abs and UnaryOperator::Negate
with a signed integer scalar or vector operand. And for
BinaryOperator::Divide and BinaryOperator::Modulo with signed or
unsigned integer scalar or vector operands.
Abs and Negate are written to avoid signed integer overflow when the
operand equals INT_MIN. This is achieved by bitcasting the value to
unsigned, using the negation operator, then bitcasting the result back
to signed. As HLSL's bitcast functions asint() and asuint() only work
for 32-bit types, we only use this workaround in such cases.
Division and Modulo avoid undefined behaviour for INT_MIN / -1 and
divide-by-zero by using 1 for the divisor instead. Additionally we
avoid undefined behaviour when using the modulo operator on operands
of mixed signedness by using the equation from the WGSL spec, using
division, subtraction and multiplication, rather than HLSL's modulus
operator.
Though not explicitly specified one way or the other, we have been
informed by DirectX engineers that signed integer overflow may be
undefined behaviour in some cases. To avoid this, we therefore bitcast
signed operands to unsigned prior to performing addition, subtraction,
or multiplication, then bitcast the result back to signed. As signed
types are represented as two's complement, this gives the correct
result whilst avoid any potential undefined behaviour.
Unfortunately HLSL's bitcast functions asint() and asuint() only work
for the 32-bit int and uint types. We therefore only apply this
workaround for 32-bit signed arithmetic. Support for other bit widths
could be added in the future, but extra care must be taken when
converting from unsigned to signed to avoid undefined or
implemented-defined behaviour.
There is no literal suffix in HLSL for the integer type. We can,
however, wrap integer literals in a constructor style cast. This
avoids ambiguity when passing literals to overloaded functions, which
we'll make use of in the subsequent patch in this PR.
Adds infrastructure to the MSL backend for emitting helper functions,
based upon the existing HLSL backend equivalent. Emit helper functions
for MathFunction::Abs and UnaryOperator::Negate with a signed integer
scalar or vector operand. And for BinaryOperator::Divide and
BinaryOperator::Modulo with signed or unsigned integer scalar or
vector operands.
Abs and Negate are written to avoid signed integer overflow when the
operand equals INT_MIN. This is achieved by bitcasting the value to
unsigned and using the negation operator, then bitcasting the result
back to signed.
Division and Modulo avoid undefined bevaviour for INT_MIN / -1 and
divide-by-zero by using 1 for the divisor instead. Additionally we
avoid undefined behaviour when using the modulo operator when either
or both operands are negative by using the equation from the WGSL
spec, using division, subtraction and multiplication, rather than
MSL's modulus operator.
Lastly, as the usage of the wrapper function for unary integer
negation makes the negation_avoids_prefix_decrement() testcase less
interesting, we extend it to additionally test negating floats.
Ensure that unnamed constants' types are retained, even when the
constants are used only by global expressions. Add a test case.
The old code marked all named constants as used; then processed
function bodies; and then marked the types of all used constants as
used. This ensured that, as long as a constant was used by a function,
its type would be retained, but didn't address the case where a
constant is used only by a global expression.
Attempting to convert a non-abstract type will always fail, which can
result in unrelated errors being misreported as type conversion
errors. For example, in #7035 we made it so that abstract types can be
used as return values. This changed the final testcase in the
invalid_functions() test to fail due to failing to convert a u32 to an
atomic<u32>, rather than with a NonConstructibleReturnType error.
This patch makes it so that we do not attempt to convert non-abstract
types in the first place. This avoids the AutoConversion error for
that testcase, meaning the NonConstructibleReturnType is once again
reported.
Various callsites of try_automatic_conversions() have been updated to
ensure they still return an InitializationTypeMismatch error if the
types are mismatched, even if try_automatic_conversions() succeeds (eg
when conversion was not attempted due to the type being concrete). To
reduce code duplication these callsites were all adapted to use the
existing type_and_init() helper function.
Lastly, a couple of tests that expected to result in a AutoConversion
error have been adjusted to ensure this still occurs, by ensuring the
type used is abstract.
When converting the underlying scalar type of an array during const
evaluation, we currently use the resulting base type's size as the array
stride for the resulting type. For certain types, this may not match the
required alignment and will therefore result in a validation error.
For example, `array<vec3<f32>, N>` should have a stride of 16. But if
declared with an abstract initializer, eg `array(vec3(0.0))` we will
incorrectly determine the stride to be 12.
To solve this, we use the proc::Layouter struct to determine the
required array stride during const evaluation. To avoid repeating
layouting work, we reuse the Lowerer's layouter, passing it through the
various *Contexts through to the ConstantEvaluator.
When lowering a return statement, call expression_for_abstract()
rather than expression() to avoid concretizing the return value. Then,
if the function has a return type, call try_automatic_conversions() to
attempt to convert our return value to the correct type.
This has the unfortunate side effect that some errors that would have
been caught by the validator are instead encountered as conversion
errors by the parser. This may result in a slightly less descriptive
error message in some cases. (See the change to the invalid_functions()
test, for example.)
In compaction, remove unused anonymous overrides.
Since overrides are no longer used by definition, include override
initialization expressions in the tandem traversal of types and global
expressions.
To simplify overload processing, we plan to make all override-sized
arrays refer to their lengths via actual `Override`s. Arrays with
non-identifier override expressions as their lengths would refer to
anonymous `Override`s with interesting `init` expressions. But in
order to avoid re-introducing #6788, we need compaction to remove
anonymous overrides.
Rather than reversing two iterators and then zipping them, zip them
first and then reverse the result.
However, zipped iterators are only reversible if the inputs implement
`ExactSizeIterator`, so make `UniqueArena::iter` promise that as well.
For consistency, make `Arena::iter` also promise to return an
`ExactSizeIterator`.
eval_zero_value_and_splat() is called to lower ZeroValue and Splat
expressions into Literal and Compose expressions. However, in its
current form it either calls splat() *or* eval_zero_value_impl()
depending on the expression type.
splat() will lower a Splat of a scalar ZeroValue to a vector
ZeroValue, which means eval_zero_value_and_splat() can still return a
ZeroValue. Its callers, such as binary_op(), are unable to handle this
ZeroValue, so cannot proceed with const evaluation.
This patch makes it so that eval_zero_value_and_splat() will first
call splat(), *and then* call eval_zero_value_impl(), which will lower
the vector ZeroValue returned by splat() into a Compose of Literals.
Callers such as binary_op() are perfectly able to handle this Compose,
so can now proceed with const evaluation.
It's currently trivial to write a shader that causes the wgsl parser
to recurse too deeply and overflow the stack. This patch makes the
parser return an error when recursing too deeply, before the stack
overflows.
It makes use of a new function Parser::track_recursion(). This
increments a counter returning an error if the value is too high,
before calling the user-provided function and returning its return
value after decrementing the counter again.
Any recursively-called functions can simply be modified to call
track_recursion(), providing their previous contents in a closure as
the argument. All instances of recursion during parsing call through
either Parser::statement(), Parser::unary_expression(), or
Parser::type_decl(), so only these functions have been updated as
described in order to keep the patch as unobtrusive as possible.
A value of 256 has been chosen as the recursion limit, but can be
later tweaked if required. This avoids the stack overflow in the
testcase attached to issue #5757.
