-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Added missing atomic operations to atomic types. #10154
Conversation
AtomicPtr now has fetch_add and fetch_sub for pointer arithmetic. AtomicInt and AtomicUint now have fetch_min, fetch_max and fetch_and, fetch_nand, fetch_or, fetch_xor for bitwise logical operations. Closes #7421
I am worried that this will break ARM build again. See #8151 for details. |
@sanxiyn would cfg-ing it out for ARM be acceptable? |
I would like {max, min, umax, umin} remain unimplemented until either 1. ARM problem is solved, or 2. we have actual uses for them. |
I vote for "cfg-ing" them out. In any case, when a consensus will be
|
It's unfortunate if we advertise ourselves as being a portable library if we have intrinsics that are cfg-ed off. It's true that this is all inside the unstable portions of the library, but I would personally rather see the API the same across all platforms, and just implemented differently on some. |
As long as we expose the LLVM intrinsics, they can be in another crate. On Fri, Nov 1, 2013 at 12:25 PM, Alex Crichton notifications@github.comwrote:
|
I also do not want to have atomics that can't be supported on some architectures. Let's start building for more recent ARM architectures, decide if we can not support those that don't have these features, and otherwise figure out how to provide the missing polyfills. |
My suggestion is to leave the intrinsics as-is. Anybody using them directly accepts the risk that they may not work. I agree though that higher-level API should stay as portable as is reasonable (i.e. if you use atomic types on a platform with literally no atomic support, you deserve what you get) |
This document from ARM says that modern atomic instructions where introduced with the ARMv6 architecture, which was announced in October 2001. |
Closing due to inactivity and because this should be able to work on all platforms. Feel free to reopen if you have updates, though! |
I agree that we should be careful about venturing outside of the atomics defined by C11/C++11. These are sure to be widely supported in any future architectures, but I wouldn't be so sure about any extra additions. |
Should this be re-opened following #12949? |
In terms of portability, I assume if we have However, @jonhoo talking about provenance honestly makes little sense when XOR is involved. |
Pointers stored as `usize` tend to cause miri to lose pointer provenance tracking, which means we can't take advantage of its checking! See also the discussion at rust-lang/miri#940 (comment). This does not yet compile since `AtomicPtr` does not have `fetch_*` methods. They were added and then removed from the standard library back in the day (rust-lang/rust#10154), but I think the reason they were removed has now been remedied, so they will hopefully come back again!
Yeah, I may have been overly eager when suggesting this would give us provenance. Though it is the case that this is currently something that's forcing libraries to use |
This adds various `fetch_` methods to `AtomicPtr` that are present on other `Atomic*` types. It does so such that libraries that depend on atomic operations on pointers do not have to cast those pointers to `usize` and fiddle around with `AtomicUsize` instead. Note that this patch currently implements `fetch_add` and `fetch_sub` without considering the size of the pointer target. This is unlike regular pointer additions and subtractions. The rationale for this is that for atomic operations, the user may indeed wish to truly increment by 1, which is difficult if all deltas are interpreted in increments of the type's size. This patch effectively resurrects the change from rust-lang#10154. Based on rust-lang#12949 (comment), the rationale for not making the changes at the time no longer hold.
See rust-lang/miri#1318 for the Miri-side issue. |
AtomicPtr now has fetch_add and fetch_sub for pointer arithmetic.
AtomicInt and AtomicUint now have fetch_min, fetch_max and
fetch_and, fetch_nand, fetch_or, fetch_xor for bitwise logical
operations.
Closes #7421