-
Notifications
You must be signed in to change notification settings - Fork 193
fix(hlsl-out): use Interlocked<op>
intrinsic for atomic integers
#2294
fix(hlsl-out): use Interlocked<op>
intrinsic for atomic integers
#2294
Conversation
326fe8f
to
6a9dc19
Compare
@jimblandy and I are planning on pair-reviewing this draft Soon™. 😊 |
264e08b
to
f4c1539
Compare
@jimblandy: I've pushed some significant updates to this during my afternoon yesterday. Most of the logic is the same, just renamed or shuffled around a bit. |
1160b52
to
2557d13
Compare
@jimblandy: Just pushed up newest changes branching on address space. TIL a bunch, thanks for the help! ❤️ |
2557d13
to
e3c994d
Compare
I noticed that I had let `dbg!(…)` sneak into commits in gfx-rs#2294 by accident. Clippy can help us with this! Set `deny(clippy::dbg_macro)`, so CI catches this for future contributions.
4a76b78
to
bf2a65e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new test file looks much more complete than what we had before!
Lines 154 to 169 in da8e911
@compute @workgroup_size(1) | |
fn atomics() { | |
var tmp: i32; | |
let value = atomicLoad(&bar.atom); | |
tmp = atomicAdd(&bar.atom, 5); | |
tmp = atomicSub(&bar.atom, 5); | |
tmp = atomicAnd(&bar.atom, 5); | |
tmp = atomicOr(&bar.atom, 5); | |
tmp = atomicXor(&bar.atom, 5); | |
tmp = atomicMin(&bar.atom, 5); | |
tmp = atomicMax(&bar.atom, 5); | |
tmp = atomicExchange(&bar.atom, 5); | |
// https://github.com/gpuweb/gpuweb/issues/2021 | |
// tmp = atomicCompareExchangeWeak(&bar.atom, 5, 5); | |
atomicStore(&bar.atom, value); | |
} |
I think we could remove this but the atomicExchange
function seems to be missing from the new test file.
😊
Agreed. Added coverage for |
We currently assume that we are using raw `RWByteAddressBuffer` methods for all atomic operations (`<pointer>.Interlocked<op>(<raw_byte_offset>, …)`), which is only true when we use `var<storage, read_write>` globals. For `var<workgroup>` globals, we need `Interlocked<op>(<pointer>, …)`, using the original expression as the first argument. Fix this by branching on the `pointer`'s address space in `Atomic` statements, and implementing the workgroup address space case with intrinsics.
57a9728
to
7132bfe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Some minor cleanups needed.
I noticed that I had let `dbg!(…)` sneak into commits in gfx-rs#2294 by accident. Clippy can help us with this! Set `deny(clippy::dbg_macro)`, so CI catches this for future contributions.
I noticed that I had let `dbg!(…)` sneak into commits in gfx-rs#2294 by accident. Clippy can help us with this! Set `deny(clippy::dbg_macro)`, so CI catches this for future contributions.
I noticed that I had let `dbg!(…)` sneak into commits in gfx-rs#2294 by accident. Clippy can help us with this! Set `deny(clippy::dbg_macro)`, so CI catches this for future contributions.
Fixes #2284.
We currently assume that we are using raw
RWByteAddressBuffer
methods for all atomic operations (<pointer>.Interlocked<op>(<raw_byte_offset>, …)
), which is only true when we usevar<storage, read_write>
globals. Forvar<workgroup>
globals, we needInterlocked<op>(<pointer>, …)
, using the original expression as the first argument.Fix this by branching on the
pointer
's address space inAtomic
statements, and implementing the workgroup address space case with intrinsics.