Skip to content
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

Rename atomic.notify and *.atomic.wait #3353

Merged
merged 4 commits into from
Nov 13, 2020

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Nov 13, 2020

  • atomic.notify -> memory.atomic.notify
  • i32.atomic.wait -> memory.atomic.wait32
  • i64.atomic.wait -> memory.atomic.wait64

See WebAssembly/threads#149.

This renames instruction name printing but not the internal data
structure names, such as AtomicNotify, which are not always the same
as printed instruction names anyway. This also does not modify C API.
But this fixes interface functions in binaryen.js because it seems
binaryen.js's interface functions all follow the corresponding
instruction names.

- atomic.notify -> memory.atomic.notify
- i32.atomic.wait -> memory.atomic.wait32
- i64.atomic.wait -> memory.atomic.wait64

See WebAssembly/threads#149.

This renames instruction name printing but not the internal data
structure names, such as `AtomicNotify`, which are not always the same
as printed instruction names anyway. This also does not modify C API.
But this fixes interface functions in binaryen.js because it seems
binaryen.js's interface functions all follow the corresponding
instruction names.
@aheejin aheejin requested review from kripken and tlively November 13, 2020 19:15
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Please mention the JS API change in CHANGELOG.md.

@dcodeIO , any concerns about the JS API change?

@dcodeIO
Copy link
Contributor

dcodeIO commented Nov 13, 2020

LGTM, thanks for the heads up :)

@aheejin aheejin merged commit 75e6120 into WebAssembly:master Nov 13, 2020
@aheejin aheejin deleted the memory_atomic branch November 13, 2020 23:54
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.

4 participants