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

Add Atomic.compare_exchange #3368

Merged
merged 3 commits into from
Dec 12, 2024
Merged

Add Atomic.compare_exchange #3368

merged 3 commits into from
Dec 12, 2024

Conversation

TheNumbat
Copy link
Contributor

@TheNumbat TheNumbat commented Dec 12, 2024

The atomic interface has compare_and_set, which returns a bool indicating whether the operation was successful.

It's often the case that you want to know the previous value upon failure, so this PR adds compare_exchange, which returns the previous value. Its semantics are otherwise the same.

Only adds a simple test, since the implementation is now used by compare_and_set.

@TheNumbat TheNumbat added flambda2 Prerequisite for, or part of, flambda2 runtime lambda Lambda language changes labels Dec 12, 2024
Copy link
Member

@glittershark glittershark left a comment

Choose a reason for hiding this comment

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

is this missing assembly for the primop in backend/x86...?

stdlib/atomic.mli Show resolved Hide resolved
runtime4/misc.c Outdated Show resolved Hide resolved
runtime/memory.c Outdated Show resolved Hide resolved
@TheNumbat
Copy link
Contributor Author

This actually doesn't need a backend implementation - compare_and_set also always calls into the runtime so it can check domain_alone/write barrier/etc. The lock_cmpxchg in the x86 backend is only used for particular intrinsics.

We should consider compiling the atomic primitives directly to instructions when their argument is an immediate, though. (In another PR)

Copy link
Member

@glittershark glittershark left a comment

Choose a reason for hiding this comment

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

lgtm

@TheNumbat TheNumbat merged commit 17d57bd into main Dec 12, 2024
20 checks passed
@TheNumbat TheNumbat deleted the atomic-cmpxchg branch December 12, 2024 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flambda2 Prerequisite for, or part of, flambda2 lambda Lambda language changes runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants