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

move generic parts of wasm_suspend_flags.h to bh_atomic.h #2393

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

yamt
Copy link
Collaborator

@yamt yamt commented Jul 26, 2023

No description provided.

@yamt yamt force-pushed the bh_atomic branch 2 times, most recently from cf7950b to 5dcb000 Compare July 26, 2023 08:06
@loganek
Copy link
Collaborator

loganek commented Jul 26, 2023

I'd expect bh_atomic to use stdatomics when possible; suspend_flag is an exception and should not use stdatomics (due to reasons explained in the linked PR). So my preference would be to keep those two separate, even if that means having some code duplicated (unless we do some ifdef's magic to only use builtin atomics in suspend_flag, but then I'm not sure if the complexity of code is worth the benefit we'll get).

@yamt
Copy link
Collaborator Author

yamt commented Jul 26, 2023

I'd expect bh_atomic to use stdatomics when possible; suspend_flag is an exception and should not use stdatomics (due to reasons explained in the linked PR). So my preference would be to keep those two separate, even if that means having some code duplicated (unless we do some ifdef's magic to only use builtin atomics in suspend_flag, but then I'm not sure if the complexity of code is worth the benefit we'll get).

given how our aot compilation works, many of our runtime structures are
very sensitive to size/offset changes. it isn't only suspend_flags for which the "fixed-sized"
property can be convenient.

Copy link
Contributor

@wenyongh wenyongh left a comment

Choose a reason for hiding this comment

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

LGTM

core/shared/utils/bh_atomic.h Show resolved Hide resolved
@wenyongh wenyongh merged commit 228417a into bytecodealliance:main Jul 27, 2023
368 checks passed
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
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.

3 participants