If it is undefined behaviour for loops to be infinite, then, when
encountering an infinite loop, downstream compilers are able to make
certain optimizations that may be unsafe. For example, omitting bounds
checks. To prevent this, we must ensure that any loops emitted by our
backends are provably bounded. We already do this for both the MSL and
HLSL backends. This patch makes us do so for SPIRV as well.
The construct used is the same as for HLSL and MSL backends: use a
vec2<u32> to emulate a 64-bit counter, which is incremented every
iteration and breaks after 2^64 iterations.
While the implementation is fairly verbose for the SPIRV backend, the
logic is simple enough. The one point of note is that SPIRV requires
`OpVariable` instructions with a `Function` storage class to be
located at the start of the first block of the function. We therefore
remember the IDs generated for each loop counter variable in a
function whilst generating the function body's code. The instructions
to declare these variables are then emitted in `Function::to_words()`
prior to emitting the function's body.
As this may negatively impact shader performance, this workaround can
be disabled using the same mechanism as for other backends: eg calling
Device::create_shader_module_trusted() and setting the
ShaderRuntimeChecks::force_loop_bounding flag to false.
Introduce the `AbstractRule` enum to control the behavior of
`Lowerer::type_and_init`. This is slightly clearer than
`allow_abstract: bool`, but more wordy.
Define `TypeInner::is_abstract`, and let it take a type arena so that
it can properly handle abstract arrays. Use this in compaction and in
the WGSL front end's conversion code to recognize abstract types.
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.