Skip to content

Add Atomics support to Multi-Memory Lowering Pass #5339

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.

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

Merged
merged 3 commits into from
Dec 12, 2022
Merged

Conversation

ashleynh
Copy link
Collaborator

@ashleynh ashleynh commented Dec 12, 2022

This PR adds support for Atomic instructions in the multi-memory lowering pass. Also includes optional bounds checks per the wasm spec guidelines, (visitAtomicRMW, visitAtomicCmpxchg, visitAtomicWait, visitAtomicNotify).

Note: The latter two instructions, memory.atomic.wait and memory.atomic.notify, have browser engine implementations that predate the still-in-progress threads spec. And whether or not atomic.notify should trap for out-of-bounds addresses remains an open issue. For now, this PR is using the same semantics as v8, which is to bounds check all Atomic instructions the same way and trap for out-of-bounds.

@ashleynh ashleynh changed the title adding atomics Add Atomics support to Multi-Memory Lowering Pass Dec 12, 2022
@ashleynh ashleynh requested review from kripken and tlively December 12, 2022 22:48
@ashleynh ashleynh marked this pull request as ready for review December 12, 2022 22:48
@@ -1,6 +1,6 @@
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
;; RUN: wasm-opt %s --enable-multi-memories --multi-memory-lowering --enable-bulk-memory --enable-extended-const --enable-simd -S -o - | filecheck %s
;; RUN: wasm-opt %s --enable-multi-memories --multi-memory-lowering-with-bounds-checks --enable-bulk-memory --enable-extended-const --enable-simd -S -o - | filecheck %s --check-prefix BOUNDS
;; RUN: wasm-opt %s --enable-multi-memories --multi-memory-lowering --enable-bulk-memory --enable-extended-const --enable-simd --enable-threads -S -o - | filecheck %s
Copy link
Member

Choose a reason for hiding this comment

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

You can also do -all to enable all the features at once and simplify the command if you want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Absolutely! Definitely treating the command more as a learning opportunity, to know which features are needed.

@ashleynh ashleynh merged commit 6c0d8f7 into main Dec 12, 2022
@ashleynh ashleynh deleted the lowering_atomic branch December 12, 2022 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants