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.
In `naga::front::wgsl`, add a new helper method,
`Lowerer::math_function_helper`, that constructs `Expression::Math`
expressions for calls to `MathFunction` builtins. Use this helper in
`Lowerer::call`.
Since we have moved some code out into separate functions, we have
less context to compete with, so we can use simpler names for the
arguments.
This commit is just renaming.
In `naga::front::wgsl::lower::Lowerer`, add a new helper method,
`resolve_overloads`, that selects the appropriate rule for a given
list of of arguments from an overload set.
Use this helper in the `call` method when building `Math`
expressions.
This is just code motion, and should have no effect on behavior.
In `naga::front::wgsl::lower::Lowerer`, add a new helper method,
`apply_automatic_conversions_for_call`, that applies the conversions
necessary to pass a given set of arguments to a given overload.
Use this helper in the `call` method when building `Math`
expressions.
This is just code motion, and should have no effect on behavior.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, use `lowered_arguments` for both the original Naga IR
argument expressions and their counterparts after applying automatic
conversions.
This is a minor refactor that prepares for later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, rename `unconverted_arguments` to `lowered_arguments`.
No change to behavior. This is a minor refactor that prepares for
later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, use `unconverted_arguments` to check the number of
arguments to the call. This means that `arguments` is never used in
this branch once we have built `unconverted_arguments`.
This is a minor refactor that prepares for later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, replace some uses of the `arguments` vector with
uses of `unconverted_arguments`.
This is a minor refactor that prepares for later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, build the vector of lowered but not yet converted
argument expressions completely before starting overload resolution.
Then iterate over those lowered arguments, not the ast arguments.
This is a minor refactor that prepares for later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, use only the
`unconverted_arguments` vector, rather that using both `arguments` and
`unconverted_arguments`.
