spv-in parse more atomic ops

[schell]

Connections
Part of

• [spv-in] Atomics support #4489

Depends on

• spv-in upgrading AtomicIIncrement type #5775

Description

This adds parsing for the following atomic operations to naga::spv::Frontend::next_block:

• OpAtomicLoad
• OpAtomicStore
• OpAtomicExchange
• OpAtomicCompareExchange
• OpAtomicCompareExchangeWeak (deprecated)
• OpAtomicIIncrement was included in spv-in parsing Op::AtomicIIncrement #5702
• OpAtomicIDecrement
• OpAtomicIAdd
• OpAtomicISub
• OpAtomicSMin
• OpAtomicUMin
• OpAtomicSMax
• OpAtomicUMax
• OpAtomicAnd
• OpAtomicOr
• OpAtomicXor
• OpAtomicFlagTestAndSet
• OpAtomicFlagClear

Checklist

• Run cargo fmt.
• Run cargo clippy. If applicable, add:
  • --target wasm32-unknown-unknown
  • --target wasm32-unknown-emscripten
• Run cargo xtask test to run tests.
• Add change to CHANGELOG.md. See simple instructions inside file.

[jimblandy]

The tests in src/front/atomic_upgrade.rs really belong in src/front/spv. We can't have SPIR-V specific code outside src/front/spv and src/back/spv.

[schell]

@teoxoy do you think you would feel comfortable giving this a look over?

[schell]

@jimblandy do you have any more comments? I think we're getting pretty close with this 🤞 .

[jimblandy]

@schell Does this commit look okay? 87b0109

[schell]

@jimblandy that commit seems just fine 👍

[jimblandy]

@schell So, right now this branch has a bunch of merges from trunk. We are generally trying to merge PRs by rebasing, because we don't want the work-in-progress commits in our history forever. Could you change this PR's branch to be merge-free?

Edit: I believe the standard git rebase behavior will just drop merges. Then you can fixup/squash as appropriate. cc our git authority: @ErichDonGubler

[schell]

@jimblandy I rebased earlier today, so I think we're good to go - though now it's out of date again, so I'll do that again.

[schell]

Ok - my rebase earlier clobbered your "span" commit, so I cherry-picked it back in. The change of get_contained_global_variable to return a Result I made at the same time you did, and your change got clobbered as well, but my change covers that issue. So now I think we're g2g 👍

[schell]

Tests still pass and everything :)

[jimblandy]

I think the flag ops are not going to work, because WGSL doesn't support atomic<bool>, so those Literal::Bool values are going to make something unhappy.

[schell]

Ok, I'll add tests for those and debug it. As an alternative do you think we can make it use u32?

[jimblandy]

Yes, treating the argument as atomic<u32> or atomic<i32> seems consistent with the SPIR-V ops, too, which say that the pointer should be a pointer "to a 32-bit integer type".

[schell]

Yeah, you're right. I just missed that. Ok, I'll just stick to that then.

[jimblandy]

If the tests pass with the code as it is, then if I'm reading correctly (and I'm often not), that would mean that the validator is not properly type-checking Statement::Atomic, because the pointer would be to an atomic<i32 | u32>, but the value would be a bool.

[schell]

You know what - I don't even have the ability to create code that uses the flag ops (not from rust-gpu, at least). I say we skip these until a later date.

[jimblandy]

Okay, with the untested ops removed, this is good to go.

[jimblandy]

Sorry - I just noticed that some of the .spvasm files in naga/tests/in/spv do not match their .spv files. That needs to be squared away before we can merge.

[jimblandy]

I've filed #6292 as a follow-up for converting the tests to snapshot tests.