Skip to content

Acquire/release memory ordering for loads #8169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
stevenfontanella merged 7 commits into from
Jan 9, 2026
Merged

Acquire/release memory ordering for loads #8169

stevenfontanella merged 7 commits into from
Jan 9, 2026

Conversation

@stevenfontanella
Copy link
Member

@stevenfontanella stevenfontanella commented Jan 2, 2026 •
edited
Loading

Part of #8165. Adds support for parsing and serializing the text and binary formats for memory orderings for loads. The C and JS apis are left unchanged, they'll be updated for all atomic operations in the same commit later. test/passes was updated using python3 scripts/auto_update_tests.py wasm-opt.

@stevenfontanella stevenfontanella linked an issue Jan 2, 2026 that may be closed by this pull request
Support for acquire/release semantics in atomics #8165
Open
@stevenfontanella stevenfontanella mentioned this pull request Jan 2, 2026
Merged
stevenfontanella added a commit that referenced this pull request Jan 2, 2026
@stevenfontanella
Replace isAtomic field with MemoryOrder enum in Load (#8170)
35e98b3 
Supplement to #8169. We're adding a MemoryOrder field in Loads (as well
as other memory accesses) to support relaxed atomics in #8165. Only
touching loads here to get feedback before handling the remaining memory
operations.
@stevenfontanella stevenfontanella force-pushed the relaxed-atomics-cp branch 15 times, most recently from 5e2de8f to b92a217 Compare January 7, 2026 07:56
@stevenfontanella stevenfontanella marked this pull request as ready for review January 7, 2026 19:46
stevenfontanella
stevenfontanella commented Jan 7, 2026
View reviewed changes
test/spec/relaxed-atomics.wast
Copy link
Member Author

@stevenfontanella stevenfontanella Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update this to match the structure that we chatted about. In the meantime, this should cover all of the logic from this PR.

tlively
tlively reviewed Jan 7, 2026
View reviewed changes
@stevenfontanella stevenfontanella mentioned this pull request Jan 8, 2026
Merged
stevenfontanella added a commit that referenced this pull request Jan 8, 2026
@stevenfontanella
Port pass tests that use safe heap + threads to lit (#8184)
f4138bb 
For #8169. Part of #8165. Remaining tests are ported in #8185.

```sh
python3 scripts/port_passes_tests_to_lit.py test/passes/safe-heap_enable-threads_enable-simd.wast test/passes/safe-heap_enable-threads_enable-simd64.wast test/passes/safe-heap_low-memory-unused_enable-threads_enable-simd.wast
```
tlively
tlively reviewed Jan 8, 2026
View reviewed changes
src/wasm/wasm-stack.cpp
o << int8_t(BinaryConsts::AtomicPrefix) << int8_t(BinaryConsts::AtomicNotify);
emitMemoryAccess(4, 4, curr->offset, curr->memory);
emitMemoryAccess(
4, 4, curr->offset, curr->memory, MemoryOrder::SeqCst, /*isRMW=*/false);
Copy link
Member

@tlively tlively Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have an emitRMWMemoryAccess helper to avoid the bool params here, too.

Copy link
Member Author

@stevenfontanella stevenfontanella Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with the bool param here? My thinking is that RMW is a memory access but if we split the method into two it would be wrong to call emitMemoryAccess on a RMW, and instead the caller would have to know to call emitRMWMemoryAccess. The bool parameter ensures that the caller gets the right behavior without needing to know about the right function.

Copy link
Member

@tlively tlively Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. I was just coming at it from a readability standpoint. It's fine the way it is.

tlively
tlively approved these changes Jan 9, 2026
View reviewed changes
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@stevenfontanella stevenfontanella enabled auto-merge (squash) January 9, 2026 01:14
@stevenfontanella stevenfontanella merged commit 57d5ae3 into main Jan 9, 2026
17 checks passed
@stevenfontanella stevenfontanella deleted the relaxed-atomics-cp branch January 9, 2026 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tlively tlively tlively approved these changes

@kripken kripken Awaiting requested review from kripken

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for acquire/release semantics in atomics

3 participants

@stevenfontanella @tlively