For some reason, the implementation of `Display` for
`ShaderError<WithSpan<ValidationError>>>` is building its own
`Diagnostic` with _only_ the labels populated: no messages or notes.
I don't know why this is, but it seems to have a strict subset of the
information already present in the private implementation of
`WithSpan::diagnostic`.
Fix this by exposing `WithSpan::diagnostic` as `pub(crate)`, and
re-using its output in `impl Display for
ShaderError<WithSpan<ValidationError>>>`.
1. Break out `word_as_ident*` helpers to keep validation of identifiers
DRY.
2. Add `peek_*` variant of `Lexer::next_ident_with_span`.
This will be consumed immediately in the subsequent commit.
Improve handling of `Access` expressions whose base is an array or
matrix (not a pointer to such), and whose index is not known at
compile time. SPIR-V does not have instructions that can do this
directly, so spill such values to temporary variables, and perform the
accesses using `OpAccessChain` instructions applied to the
temporaries.
When performing chains of accesses like `a[i].x[j]`, do not reify
intermediate values; generate a single `OpAccessChain` for the entire
thing.
Remove special cases for arrays; the same code now handles arrays and
matrices.
Update validation to permit dynamic indexing of matrices.
For details, see the comments on the new tracking structures in
`naga:🔙:spv::Function`.
Add snapshot test `index-by-value.wgsl`.
Fixes#6358.
Fixes#4337.
Alternative to #6362.
Pull out the code to build a `naga::proc::index::GuardedIndex` from a
`Handle<Expression>` into its own function,
`GuardedIndex::from_expression`. Use that function in
`GuardedIndex::try_resolve_to_constant`.
Pull out the code for building a `naga:🔙:spv::NumericType` from a
`TypeInner` into its own function, `NumericType::from_inner`. Use that
in `LocalType::from_inner`.
Replace the `return_type_override` argument of
`BlockContext::write_expression_pointer` with an enum that says how to
derive the return type from `expr_handle`'s type.
Introduce a new type, `AccessTypeAdjustment`, that covers possible
derivation rules.
This simplifies callers and the callee, in part by making the possible
alternatives less general, and by giving them explicit names (the
variants of the `AccessTypeAdjustment` enum).
Introduce a new function,
`naga:🔙:spv::BlockContext::write_checked_load`, that does the
work of `Expression::Load`.
This change is just code motion, and should have no effect on
behavior. The new function will be used in later commits.
Delete the function `BlockContext::is_intermediate`. Instead, have
`Access` and `AccessIndex` instructions decide whether to defer code
generation based on the type of the base expression: indexing
operations on pointers are deferred; anything else is not.
Simplify the definition of `naga:🔙:spv::Writer::get_pointer_id`
by using `get_type_id`'s ability to handle `LocalType::Pointer`.
This means that `get_pointer_id` is no longer fallible, and no longer
needs a type arena. Simplify callers, as well as the
`BlockContext::get_pointer_id` convenience function.
In `back::spv`:
- Factor out the numeric variants of `LocalType` into a
new enum, `NumericType`.
- Split the `Value` variant into `Numeric` and `LocalPointer`
variants, and let `LocalPointer` point to any numeric type,
including matrices.
In subsequent commits, we'll need to spill matrices out into temporary
local variables. This means we'll need to generate SPIR-V
pointer-to-matrix types, so `LocalType` needs to be able to represent
that.
Do not neglect to apply bounds checks to indexing operations on
runtime-sized arrays, even when they are accessed via an `AccessIndex`
instruction.
Before this commit, `BlockContext::write_expression_pointer` would not
apply bounds checks to `OpAccessChain` indices provided by an
`AccessIndex` instruction, apparently with the rationale that any
out-of-bounds accesses should have been reported by constant
evaluation.
While it is true that the `index` operand of an `AccessIndex`
expression is known at compile time, and that the WGSL constant
evaluation rules require accesses that can be statically determined to
be out-of-bounds to be shader creation or pipeline creation time
errors, accesses to runtime-sized arrays don't follow this pattern:
even if the index is known, the length with which it must be compared
is not.
Fixes#4441.
Let `BoundsCheckResult::Conditional` provide both the condition to
check before carrying out the access, and the index to use for that
access. The `Conditional` variant indicates that we generated a
runtime bounds check, which implies we must have had a SPIR-V id for
the index to pass to that check, so there's no reason not to provide
that to the callers - especially if the bounds check code was able to
reduce it to a known constant.
At the moment, this is not much of a refactor, but later commits will
use `GuardedIndex` in more places, at which point this will avoid a
re-matching and assertion.
Abstract the code from `write_expression_pointer` to handle one
indexing operation out into its own function,
`BlockContext::write_access_chain_index`.
Let the SPIR-V backend use `GuardedIndex::try_resolve_to_constant`,
rather than writing out its definition in `write_restricted_index` and
`write_index_comparison`.
Call `try_resolve_to_constant` in one place, in `write_bounds_check`,
and simply pass the `GuardedIndex` into subroutines.
Reduce `write_restricted_index` and `write_index_comparison` to case
analysis and code generation.
Note that this commit does have a benign effect on SPIR-V snapshot
output for programs like this:
let one_i = 1i;
var vec0 = vec3<i32>();
vec0[one_i] = 1;
The value indexing `vec0` here is an `i32`, but after this commit, the
operand to `OpAccessChain` becomes a `u32` constant (with the same
value).
This is because `write_bounds_check` now calls
`try_resolve_to_constant` itself, rather than deferring this work to
its callees, so it may return `BoundsCheckResult::KnownInBounds` even
when the `Unchecked` policy is in force. This directs the caller,
`write_expression_pointer`, to treat the `OpAccessChain` operand as a
fresh `u32` constant, rather than simply passing through the original
`i32` expression.
