Conversation
cwfitzgerald
left a comment
There was a problem hiding this comment.
Looking good from the wgpu side, small comments.
There should be at least one execution test that makes sure everything at least seems to behave correctly and gets through all validation layers and such. Check calling atomicAdd N times results in a value of N, etc.
jimblandy
left a comment
There was a problem hiding this comment.
Still reviewing, this is just the stuff I've noticed so far.
jimblandy
left a comment
There was a problem hiding this comment.
Before I review this further I'd like to see the doc comments filled in enough.
|
@atlv24 Thanks very much for the new comments - these are helpful. |
|
@atlv24 I just pushed a new commit to this branch; the log message is below. Could you look it over and see what you think?
|
|
(No offense meant by removing your docs, after asking you to write them - they were helpful in getting started!) |
|
Merged trunk so CI would pay attention |
jimblandy
left a comment
There was a problem hiding this comment.
If my changes look good, then the rest of this looks good to me.
teoxoy
left a comment
There was a problem hiding this comment.
Took a brief look at the feature detection code.
|
@jimblandy I reviewed, looks great! I wish I had thought of using Option myself haha. Would have saved me a lot of copy pasting :) |
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.
Connections
Part of an incoming series of PRs needed by #5123
Others in this series: #5155 #5154
Addresses #4424
Depends on #5498 and #5497
Ultimately for bevy meshlets pipeline bevyengine/bevy#10164
Description
Shaders currently do not support 64 bit integer atomics. Add support for these gated by a feature flag when the capability is present.
Metal 64bit atomics are quite different from VK/DX style atomics, they do not have a return/output value of any kind and are only available in min/max variants. To accommodate this, a separate feature was added for min/max 64 bit atomics, and AtomicFunctionNoReturn was created in the naga IR to represent these return-less atomics.
DirectX ShaderModel version detection was also implemented, as SM 6.0 was insufficient for this feature.
Testing
A couple snapshot tests have been added to test int64 atomic operations.
Checklist
cargo fmt.cargo clippy. If applicable, add:--target wasm32-unknown-unknown--target wasm32-unknown-emscriptencargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.