Add multi-threaded tests for acquire/release atomics by stevenfontanella · Pull Request #8999 · WebAssembly/binaryen

stevenfontanella · 2026-08-12T23:27:39Z

Add multi-threaded execution tests exercising acquire/release atomics
- Most tests demonstrate at least one possible result that would not be possible with seqcst. It's impossible to write a test that would fail on an engine that implements acqrel using seqcst, 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.
Invoke the pause instruction in the basic.wast test which was previously unexercised
In support.split_wast, ensure that (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).
Rename relaxed_atomic_execution_tests -> acquire_release_atomics_execution_tests to match the new proposal name
Passes in Binaryen's interpreter in an uninteresting way because (thread) blocks always run sequentially in a blocking manner.

tlively · 2026-08-13T00:00:23Z

+;; 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))


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?

I think we would have to write test code. We can't have either of a product, instead it has to be a product of eithers like I have here:

Are you asking for testing seqcst?

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.

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.

tlively · 2026-08-13T21:55:03Z

+      ;; Store observed flag at address 8, observed payload at address 12
+      (i32.store (i32.const 8) (i32.atomic.load acqrel (i32.const 0)))


But the flag was written at address 0, right?

It might be nice to give the addresses "names" by having immutable globals named $x, $flag, etc. that are initialized with the same values in every module.

It would also be nice to have pseudocode like x =un 1; flag =rel 1 in the comments.

Yes, I think this code is correct, can you rephrase if I missed something? We're reading the flag here (address 0), and writing the observation to address 8. Added comments on each of the loads and stores.

(For posterity, we discussed in person that adding globals to name the addresses would be more noisy since they can't be shared across threads and we'd have lots of extra global.gets)

I was confused and the new names help, thanks! This the other case below LGTM.

tlively · 2026-08-13T21:59:31Z

+      ;; Observed flag - address 8
+      (i32.store (i32.const 8) (i32.atomic.load acqrel (i32.const 0)))


Looks like we have the same address mismatch here.

Ditto here, it still looks correct to me after reading it again. Let me know if I missed something.

tlively · 2026-08-13T22:03:40Z

+    (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)
+      )
+    )


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.

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.

Sounds good to me 👍

tlively · 2026-08-14T22:50:21Z

+      ;; a =rel 1
+      ;; b =rel 2


The convention in the memory model papers is to use x,y,z as memory locations and a,b,c or r1,r2,r3 as local variables / registers. So to match those conventions, these should be x =rel 1, y =rel 2.

tlively · 2026-08-14T22:51:05Z

+  (module
+    (memory (import "mem" "shared") 1 1 shared)
+    (func (export "run")
+      ;; payload =un 42


But I also like using descriptive names as payload and flag. Let's not change those.

tlively · 2026-08-14T22:52:24Z

+    (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)
+      )
+    )


Sounds good to me 👍

Tests for acquire/release atomics

b9ded3f

stevenfontanella force-pushed the relaxed-atomics-execution branch from b28920c to b9ded3f Compare August 12, 2026 23:34

stevenfontanella marked this pull request as ready for review August 12, 2026 23:42

stevenfontanella requested a review from a team as a code owner August 12, 2026 23:42

stevenfontanella requested review from kripken, rmahdav and tlively and removed request for a team and kripken August 12, 2026 23:42

kripken approved these changes Aug 13, 2026

View reviewed changes

tlively reviewed Aug 13, 2026

View reviewed changes

Comment thread test/spec/acquire-release-atomics/threaded.wast Outdated

rmahdav reviewed Aug 13, 2026

View reviewed changes

Comment thread scripts/test/acquire_release_atomics_execution_tests.py

stevenfontanella added 2 commits August 13, 2026 17:33

Rename test script

186fde7

Write to memory instead of using shared globals, which aren't supported

0df3bff

tlively reviewed Aug 13, 2026

View reviewed changes

Add some comments

430ad10

tlively approved these changes Aug 14, 2026

View reviewed changes

		;; Store observed flag at address 8, observed payload at address 12
		(i32.store (i32.const 8) (i32.atomic.load acqrel (i32.const 0)))

		;; Observed flag - address 8
		(i32.store (i32.const 8) (i32.atomic.load acqrel (i32.const 0)))

Uh oh!

Conversation

stevenfontanella commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stevenfontanella commented Aug 12, 2026 •

edited

Loading