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).
Have constant evaluator use `wrapping_abs` instead of `abs` when
applying `MathFunction::Abs` to AbstractInt values. WGSL says that
applying `abs` to the most negative AbstractInt value must return it
unchanged, which is what Rust's `i64::wrapping_abs` does.
Change `resolve_type` and `resolve_type_impl` to return
`TypeResolution`s. Add a new method `resolve_type_inner` that returns a
`TypeInner` (i.e. what `resolve_type` used to do).
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.
When asked to generate WGSL for `TypeInner::Struct`, rather than
unconditionally calling `unreachable!`, defer to a new `TypeContext`
method, `write_unnamed_struct`.
Provide appropriate `write_unnamed_struct` implementations:
- In the WGSL backend, implement this as `unreachable!`, since the WGSL
backend should always know the proper name to use for a struct.
- For diagnostic messages, generate something human-readable that
indicates that some struct type was encountered.
- For logging and debugging, defer to `TypeInner`'s `Debug`
implementation.
Delete the implementation of `core::fmt::Display` for
`naga::common::DiagnosticDisplay`, as it is a footgun wherever
`Struct` types can arise.
Document that it should not be implemented for `TypeInner`.
Although it is possible to put a dummy implementation in the way of
people adding real implementations, that would also turn attempts to
use it into dynamic errors rather than compile-time errors. Make do
with a comment for now.
Simplify the implementation of `Display` for
`DiagnosticDisplay<(TypeResolution, GlobalCtx)>` by calling
`GlobalCtx`'s implementation of `write_type_resolution` directly,
rather than duplicating its code. Provide a fallback for non-wgsl
builds.
In `naga:🔙:wgsl`, prefer `TypeContext::write_type_resolution`
over `write_type_inner`, since the former is actually what we need,
and the latter can be troublesome if asked to write a `struct` type.
In `naga::common::wgsl`, delete the method
`TypeContext::type_inner_to_string`, since it's a footgun: anyone
trying to convert a `TypeInner::Struct` to a string will hit
"unreachable" code.
In `naga::front::wgsl::lower`, change logging in
`ExpressionContext::automatic_conversion_consensus` to use
`TypeResolution`s instead of `TypeInner`s, so that structs don't cause
a crash.
When a type conversion expression is ill-formed and the operand is a
struct type, don't panic trying to format the operand's type for the
error message.
Don't try to use `&TypeInner` values to generate error messages, since
`&TypeInner` cannot represent struct types in WGSL - they must be
referred to by name, and only `Type` knows the type's name. Using
`Handle<Type>` or `TypeResolution` works fine, so use that instead.
Add a new `Display` impl for `DiagnosticDisplay` for `TypeResolution`.
Fixes#7495.
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.
