Add multi-threaded tests for acquire/release atomics - #8999
Conversation
b28920c to
b9ded3f
Compare
| ;; 1, 3 is only possible with acqrel, while others are also possible with | ||
| ;; seqcst. | ||
| (assert_return (invoke "check") | ||
| (either (i32.const 1) (i32.const 4)) |
There was a problem hiding this comment.
Is there a way to express "either (1, 2) or (3, 4) or (2, 4)" with the either construct? Or would you have to write test code to explicitly check that kind of condition?
There was a problem hiding this comment.
Right, I was having trouble understanding how to read this because I would have expected either to be more powerful. It's fine for this case, but it would be useful to have more expressive power for the seqcst case.
There was a problem hiding this comment.
I'll leave it for now, and if we have a seqcst test in the future it might make more sense to make both into a check function.
| (func $lock | ||
| (loop $spin | ||
| (if (i32.eqz (i32.atomic.rmw.cmpxchg acqrel (i32.const 0) (i32.const 0) (i32.const 1))) | ||
| (then (return)) | ||
| ) | ||
| (pause) | ||
| (br $spin) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
It's awfully inconvenient to have to repeat so much of the module contents on each thread! It might be worth going back and adding some richer .wast infrastructure either in the original threads repo or as part of the acquire-release proposal.
There was a problem hiding this comment.
I looked into this and one potential solution is the (module definition ...) and (module instance ...) syntax from the upstream spec interpreter (example). I think we would have one module that exports the shared memory, then a module definition that imports the shared memory and exports functions for the spinlock, then each threads instantiates the second module separately.
But I think we should wait on this solution, since the threads repo (and thus the acquire-release-atomics repo) is behind the upstream spec and doesn't support this syntax, so we wouldn't be able to run this on the spec interpreter. I'd suggest that we keep it this way and simplify the test once the threads + acquire-release-atomics interpreter have support for this syntax.
| (func $lock | ||
| (loop $spin | ||
| (if (i32.eqz (i32.atomic.rmw.cmpxchg acqrel (i32.const 0) (i32.const 0) (i32.const 1))) | ||
| (then (return)) | ||
| ) | ||
| (pause) | ||
| (br $spin) | ||
| ) | ||
| ) |

seqcst. It's impossible to write a test that would fail on an engine that implementsacqrelusingseqcst, since strengthening an atomic operation is always sound.(atomic.fence acqrel)currently can't be exercised in a way that's correct without being completely redundant alongside another acqrel load/store. With a relaxed memory ordering the test would make more sense.basic.wasttest which was previously unexercised(thread ...)and(wait ...)expressions are captured in the split output (the latter is currently a no-op anyway but best to include it for readability and correctness).(thread)blocks always run sequentially in a blocking manner.