-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 more integer atomic types #1543
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
- Feature Name: integer_atomics | ||
- Start Date: 2016-03-14 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
This RFC basically changes `core::sync::atomic` to look like this: | ||
|
||
```rust | ||
#[cfg(target_has_atomic = "8")] | ||
struct AtomicBool {} | ||
#[cfg(target_has_atomic = "8")] | ||
struct AtomicI8 {} | ||
#[cfg(target_has_atomic = "8")] | ||
struct AtomicU8 {} | ||
#[cfg(target_has_atomic = "16")] | ||
struct AtomicI16 {} | ||
#[cfg(target_has_atomic = "16")] | ||
struct AtomicU16 {} | ||
#[cfg(target_has_atomic = "32")] | ||
struct AtomicI32 {} | ||
#[cfg(target_has_atomic = "32")] | ||
struct AtomicU32 {} | ||
#[cfg(target_has_atomic = "64")] | ||
struct AtomicI64 {} | ||
#[cfg(target_has_atomic = "64")] | ||
struct AtomicU64 {} | ||
#[cfg(target_has_atomic = "128")] | ||
struct AtomicI128 {} | ||
#[cfg(target_has_atomic = "128")] | ||
struct AtomicU128 {} | ||
#[cfg(target_has_atomic = "ptr")] | ||
struct AtomicIsize {} | ||
#[cfg(target_has_atomic = "ptr")] | ||
struct AtomicUsize {} | ||
#[cfg(target_has_atomic = "ptr")] | ||
struct AtomicPtr<T> {} | ||
``` | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
Many lock-free algorithms require a two-value `compare_exchange`, which is effectively twice the size of a `usize`. This would be implemented by atomically swapping a struct containing two members. | ||
|
||
Another use case is to support Linux's futex API. This API is based on atomic `i32` variables, which currently aren't available on x86_64 because `AtomicIsize` is 64-bit. | ||
|
||
# Detailed design | ||
[design]: #detailed-design | ||
|
||
## New atomic types | ||
|
||
The `AtomicI8`, `AtomicI16`, `AtomicI32`, `AtomicI64` and `AtomicI128` types are added along with their matching `AtomicU*` type. These have the same API as the existing `AtomicIsize` and `AtomicUsize` types. Note that support for 128-bit atomics is dependent on the [i128/u128 RFC](https://github.com/rust-lang/rfcs/pull/1504) being accepted. | ||
|
||
## Target support | ||
|
||
One problem is that it is hard for a user to determine if a certain type `T` can be placed inside an `Atomic<T>`. After a quick survey of the LLVM and Clang code, architectures can be classified into 3 categories: | ||
|
||
- The architecture does not support any form of atomics (mainly microcontroller architectures). | ||
- The architecture supports all atomic operations for integers from i8 to iN (where N is the architecture word/pointer size). | ||
- The architecture supports all atomic operations for integers from i8 to i(N*2). | ||
|
||
A new target cfg is added: `target_has_atomic`. It will have multiple values, one for each atomic size supported by the target. For example: | ||
|
||
```rust | ||
#[cfg(target_has_atomic = "128")] | ||
static ATOMIC: AtomicU128 = AtomicU128::new(mem::transmute((0u64, 0u64))); | ||
#[cfg(not(target_has_atomic = "128"))] | ||
static ATOMIC: Mutex<(u64, u64)> = Mutex::new((0, 0)); | ||
|
||
#[cfg(target_has_atomic = "64")] | ||
static COUNTER: AtomicU64 = AtomicU64::new(0); | ||
#[cfg(not(target_has_atomic = "64"))] | ||
static COUTNER: AtomicU32 = AtomicU32::new(0); | ||
``` | ||
|
||
Note that it is not necessary for an architecture to natively support atomic operations for all sizes (`i8`, `i16`, etc) as long as it is able to perform a `compare_exchange` operation with a larger size. All smaller operations can be emulated using that. For example a byte atomic can be emulated by using a `compare_exchange` loop that only modifies a single byte of the value. This is actually how LLVM implements byte-level atomics on MIPS, which only supports word-sized atomics native. Note that the out-of-bounds read is fine here because atomics are aligned and will never cross a page boundary. Since this transformation is performed transparently by LLVM, we do not need to do any extra work to support this. | ||
|
||
## Changes to `AtomicPtr`, `AtomicIsize` and `AtomicUsize` | ||
|
||
These types will have a `#[cfg(target_has_atomic = "ptr")]` bound added to them. Although these types are stable, this isn't a breaking change because all targets currently supported by Rust will have this type available. This would only affect custom targets, which currently fail to link due to missing compiler-rt symbols anyways. | ||
|
||
## Changes to `AtomicBool` | ||
|
||
This type will be changes to use an `AtomicU8` internally instead of an `AtomicUsize`, which will allow it to be safely transmuted to a `bool`. This will make it more consistent with the other atomic types that have the same layout as their underlying type. (For example futex code will assume that a `&AtomicI32` can be passed as a `&i32` to the system call) | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
Having certain atomic types get enabled/disable based on the target isn't very nice, but it's unavoidable because support for atomic operations is very architecture-specific. | ||
|
||
This approach doesn't directly support for atomic operations on user-defined structs, but this can be emulated using transmutes. | ||
|
||
# Alternatives | ||
[alternatives]: #alternatives | ||
|
||
One alternative that was discussed in a [previous RFC](https://github.com/rust-lang/rfcs/pull/1505) was to add a generic `Atomic<T>` type. However the consensus was that having unsupported atomic types either fail at monomorphization time or fall back to lock-based implementations was undesirable. | ||
|
||
Several other designs have been suggested [here](https://internals.rust-lang.org/t/pre-rfc-extended-atomic-types/3068). | ||
|
||
# Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
None |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AtomicPtr, Isize and Usize can't be defined
cfg(target_has_atomic)
because it's not backwards compatible. Please change this before landing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brson are you stating that because we've previously promised support for atomic ptr-size values, we're stuck supporting them for all platforms rust targets for the foreseeable future? In effect: if we ever want to support a platform without pointer-size atomics, it will require compiler-rt provide some AtomicPtr work-alike?
As the RFC notes later, all architectures we currently support would be unaffected. Only new, not yet existent archs would be broken. I'm presuming you don't think this is sufficient?
Is there some other process you're implicitly proposing here that I'm missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmesmon I think the ship has sailed on supporting ptr-sized atomics conditionally. We already guarantee support on platforms that don't have atomics.
This is a pretty bad situation though because our atomics are not guaranteed to be lock free.
Saving grace may be that no architectures without atomics are tier-1 so maybe we can roll back these prior decisions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be precise all of the targets in tier 1, 2 and 3 support atomic operations. The only way to have a target that doesn't support atomics is through a custom target spec. And even in these cases I think attempts to use atomic operations will result in a linker error due to missing functions in compiler-rt (compiler-rt uses spinlocks to implement atomic operations, which themselves require support for atomic operations).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brson
Isn't this exactly what the RFC says (which I referred to previously)? Is there something I'm missing here? Or is this just an agreement? Maybe you're just noting that this is something that needs discussion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I ran a few tests, and here's the current situation for platforms that don't support atomic types (I tested with ARMv6-M):
fetch_add
into a call to__sync_fetch_and_add_4
, which is normally provided by compiler-rt.__sync_fetch_and_add_4
, so it fails to compile on such platforms anyways.So basically, atomic operations on those platforms has never worked in the first place, so we really aren't breaking anything.