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.
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 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.
When generating SPIR-V, avoid generating unreachable blocks following
statements like `break`, `return`, and so on that cause non-local
exits. These unreachable blocks can cause SPIR-V validation to fail.
Fixes#6220.
Bring https://github.com/gfx-rs/naga/pull/723 back from the dead.
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Co-authored-by: Jim Blandy <jimb@red-bean.com>
Require `T` to be a struct in `binding_array<T, ...>`; do not permit
arrays.
In #5428, the validator was changed to accept binding array types that
the SPIR-V backend couldn't properly emit. Specifically, the validator
was changed to accept `binding_array<array<T>>`, but the SPIR-V
backend wasn't changed to wrap the binding array elements in a SPIR-V
struct type, as Vulkan requires. So the type would be accepted by the
validator, and then rejected by the backend.
See the comments in the code for details.
This patch emits the definition of the macro only when the first loop
is encountered. This does make that first loop's code look a bit odd:
it would be more natural to define the macro at the top of the
file. (See the modified files in `naga/tests/out/msl`.)
Rejected alternatives:
- We could emit the macro definition unconditionally at the top of the
file. But this changes every MSL snapshot output file, whereas only
eight of them actually contain loops.
- We could have the validator flag modules that contain loops. But the
changes end up being not small, and spread across the validator, so
this seems disproportionate. If we had other consumers of this
information, it might make sense.
- We could change the MSL backend to allow text to be generated out of
order, so that we can decide whether to define the macro after we've
generated all the function bodies. But at the moment this seems like
unnecessary complexity, although it might be worth doing in the
future if we had additional uses for it - say, to conditionally emit
helper function definitions.
Fixes#4972.
* add parsing for spirv::Op::AtomicLoad and spirv::Op::AtomicStore
* spv-in parse AtomicExchange and AtomicCompareExchange
* add atomic i decrement
* bookend atomic store statement with emmitter.finish/emitter.start to suppress a double load expression
bookend atomic result expressions with emitter.finish/start to prevent double defs
* add atomic iadd, isub, smin, umin, smax, umax, and, or, xor
* parse atomic flag test and set, parse atomic flag clear
* remove atomic compare exchange work
* changelog
* moved spirv tests into front/spv/mod.rs
* feature gate atomic spv tests because they require wgsl-[in,out]
* BlockContext::get_contained_global_variable returns Result
* Generate spans covering the entire instruction.
Granted, there is pre-existing code in the SPIR-V front end that gets
this wrong, but:
It doesn't make sense to read `self.data_offset`, and then immediately
pass that to `self.span_from_with_op`. The point of that function is
to make the span cover the entire instruction, operands included.
* Move `From` implementation into spv front end
* doc comments, minor cleanups
* remove parsing of OpAtomicFlagClear and OpAtomicFlagTestAndSet
* sync atomic spvasm files
---------
Co-authored-by: Jim Blandy <jimb@red-bean.com>
Introduce a new module, `naga:🔙:continue_forward`, containing
shared code for rendering Naga `Continue` statements as backend
`break` statements and assignments to introduced `bool` locals.
See the module's documentation for details.
- [hlsl-out] Transform degenerate single body switches into `do-while`
loops. Properly render `Continue` statements enclosed by
`Switch` statements enclosed by `Loop` statements.
- [glsl-out] Transform degenerate single body switches into `do-while`
loops.
Improve `naga xtask validate spv` error message.
Fixes#4485.
Fixes#4514.
In `naga:🔙hlsl`:
- Generate calls to `Interlocked{op}64` when necessary. not
`Interlocked{op}`.
- Make atomic operations that do not produce a value emit their
operands properly.
In the Naga snapshot tests:
- Adapt `atomicOps-int64-min-max.wgsl` to include cases that
cover non-trivial atomic operation operand emitting.
In `wgpu_hal::vulkan::adapter`:
- When retrieving physical device features, be sure to include
the `PhysicalDeviceShaderAtomicInt64Features` extending struct
in the chain whenever the `VK_KHR_shader_atomic_int64` extension
is available.
- Request both `shader_{buffer,shared}_int64_atomics` in the
`PhysicalDeviceShaderAtomicInt64Features` extending struct when either of
`wgpu_types::Features::SHADER_INT64_ATOMIC_{ALL_OPS,MIN_MAX}` is requested.
---------
Co-authored-by: Jim Blandy <jimb@red-bean.com>
* Allow unconsumed inputs in fragment shaders by removing them from vertex
outputs when generating HLSL.
Fixes https://github.com/gfx-rs/wgpu/issues/3748
* Add naga:🔙:hlsl::FragmentEntryPoint for providing information
about the fragment entry point when generating vertex entry points via
naga:🔙:hlsl::Writer::write. Vertex outputs not consumed by the
fragment entry point are omitted in the final output struct.
* Add naga snapshot test for this new feature,
* Remove Features::SHADER_UNUSED_VERTEX_OUTPUT,
StageError::InputNotConsumed, and associated validation logic.
* Make wgpu dx12 backend pass fragment shader info when generating
vertex HLSL.
* Add wgpu regression test for allowing unconsumed inputs.
* Address review
* Add note that nesting structs for the inter-stage interface can't
happen.
* Remove new TODO notes (some addressed and some transferred to an issue
https://github.com/gfx-rs/wgpu/issues/5577)
* Changed issue that regression test refers to 3748 -> 5553
* Add debug_assert that binding.is_some() in hlsl writer
* Fix typos caught in CI
Also, fix compiling snapshot test when hlsl-out feature is not enabled.
To support atomic instructions in the SPIR-V front end, observe which
global variables the input accesses using atomic instructions, and
adjust their types from ordinary scalars to atomic values.
See comments in `naga::front::atomic_upgrade`.
When the appropriate features are enabled, manually implement
`serde::Serialize` and `serde::Deserialize`, such that the serialized
form of `NonMaxU32::new(n).unwrap()` is the same as that of `n`.
This eliminates the last trace of 1-based indices from Naga's snapshot
tests, and aligns `std::fmt::Debug` with the serialized form.
Define a new type, `NonMaxU32`, that can represent any `u32` value
except `u32::MAX`, defined such that `Option<NonMaxU32>` is still a
32-bit value.
Change `Handle` to use `NonMaxU32`. Adjust all code that works
directly with handle indices to assume zero-based indices, not
one-based indices.
Make the HLSL backend more like other backends by using `back::Baked`
to generate names for baked expression identifiers. This removes the
final uses of `Handle::index` from the HLSL backend.
This is separated out from the previous commit because it changes lots
of snapshot tests, whereas the previous commit has no effect on Naga's
output.
Add the following flags to `wgpu_types::Features`:
- `SHADER_INT64_ATOMIC_ALL_OPS` enables all atomic operations on `atomic<i64>` and
`atomic<u64>` values.
- `SHADER_INT64_ATOMIC_MIN_MAX` is a subset of the above, enabling only
`AtomicFunction::Min` and `AtomicFunction::Max` operations on `atomic<i64>` and
`atomic<u64>` values in the `Storage` address space. These are the only 64-bit
atomic operations available on Metal as of 3.1.
Add corresponding flags to `naga::valid::Capabilities`. These are supported by the
WGSL front end, and all Naga backends.
Platform support:
- On Direct3d 12, in `D3D12_FEATURE_DATA_D3D12_OPTIONS9`, if
`AtomicInt64OnTypedResourceSupported` and `AtomicInt64OnGroupSharedSupported` are
both available, then both wgpu features described above are available.
- On Metal, `SHADER_INT64_ATOMIC_MIN_MAX` is available on Apple9 hardware, and on
hardware that advertises both Apple8 and Mac2 support. This also requires Metal
Shading Language 2.4 or later. Metal does not yet support the more general
`SHADER_INT64_ATOMIC_ALL_OPS`.
- On Vulkan, if the `VK_KHR_shader_atomic_int64` extension is available with both the
`shader_buffer_int64_atomics` and `shader_shared_int64_atomics` features, then both
wgpu features described above are available.
This proves a flag in msl::PipelineOptions that attempts to write all
Metal vertex entry points to use a vertex pulling technique. It does
this by:
1) Forcing the _buffer_sizes structure to be generated for all vertex
entry points. The structure has additional buffer_size members that
contain the byte sizes of the vertex buffers.
2) Adding new args to vertex entry points for the vertex id and/or
the instance id and for the bound buffers. If there is an existing
@builtin(vertex_index) or @builtin(instance_index) param, then no
duplicate arg is created.
3) Adding code at the beginning of the function for vertex entry points
to compare the vertex id or instance id against the lengths of all the
bound buffers, and force an early-exit if the bounds are violated.
4) Extracting the raw bytes from the vertex buffer(s) and unpacking
those bytes into the bound attributes with the expected types.
5) Replacing the varyings input and instead using the unpacked
attributes to fill any structs-as-args that are rebuilt in the entry
point.
A new naga test is added which exercises this flag and demonstrates the
effect of the transform. The msl generated by this test passes
validation.
Eventually this transformation will be the default, always-on behavior
for Metal pipelines, though the flag may remain so that naga
translation tests can be run with and without the tranformation.
Add tests to ensure that validation checks that `CallResult` and
`AtomicResult` expressions actually have their values provided by
`Call` and `Atomic` statements, and not `Emit` statements.
This fixes 2 issues:
- we used to index `adjusted_global_expressions` with the handle index of the constant instead of its initializer
- we used to adjust the initializer multiple times if the arena contained multiple `Expression::Constant`s pointing to the same constant
