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

Implement compare_exchange and compare_exchange_weak #30969

Merged
merged 3 commits into from
Feb 22, 2016

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Jan 17, 2016

This is an implementation of rust-lang/rfcs#1443.

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@Amanieu Amanieu force-pushed the extended_atomic_cmpxchg branch 4 times, most recently from 841b5ac to 93b5e82 Compare January 17, 2016 11:49
@brson
Copy link
Contributor

brson commented Jan 20, 2016

Just noting that this is not an accepted RFC, so not reviewing yet.

@Amanieu Amanieu changed the title Extend atomic compare_and_swap Implement compare_exchange and compare_exchange_weak Feb 18, 2016
@Amanieu
Copy link
Member Author

Amanieu commented Feb 18, 2016

I've updated the implementation to match the accepted RFC.

/// `compare_exchange` takes two `Ordering` arguments to describe the memory ordering
/// of this operation. The first describes the required ordering if the operation succeeds while
/// the second describes the required ordering when the operation fails. The failure ordering
/// can't be `Acquire` or `AcqRel` and must be equivalent or weaker than the success ordering.
Copy link
Member

Choose a reason for hiding this comment

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

Could you wrap these comments to 80 chars? (the surrounding style)

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually the surrounding style seems to wrap comments at 100 chars, which is what I've been using.

@alexcrichton
Copy link
Member

Thanks @Amanieu!

@Amanieu
Copy link
Member Author

Amanieu commented Feb 18, 2016

Updated with comments addressed

@alexcrichton
Copy link
Member

Thanks! Just to ensure that we've got all code paths covered, could you make sure that there's at least some call to each cmpxchg instrinsic in the tests? Just to make sure that trans doesn't choke on accident from any of these.

@Amanieu
Copy link
Member Author

Amanieu commented Feb 22, 2016

Done

@alexcrichton
Copy link
Member

@bors: r+ 4fdbc2f

Thanks!

@bors
Copy link
Contributor

bors commented Feb 22, 2016

⌛ Testing commit 4fdbc2f with merge d3929b2...

bors added a commit that referenced this pull request Feb 22, 2016
@bors bors merged commit 4fdbc2f into rust-lang:master Feb 22, 2016
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.

None yet

5 participants