Skip to content

Commit

Permalink
Rollup merge of rust-lang#54280 - japaric:no-cas-for-thumbv6, r=alexc…
Browse files Browse the repository at this point in the history 
…richton

remove (more) CAS API from Atomic* types where not natively supported

closes rust-lang#54276

In PR rust-lang#51953 I made the Atomic* types available on targets like thumbv6m and
msp430 with the intention of *only* exposing the load and store API on those
types -- the rest of the API doesn't work on those targets because the are no
native instructions to implement CAS loops.

Unfortunately, it seems I didn't properly cfg away all the CAS API on those
targets, as evidenced in rust-lang#54276. This PR amends the issue by removing the rest
of the CAS API.

This is technically a breaking change because *libraries* that were using this
API and were being compiled for e.g. thumbv6m-none-eabi will stop compiling.
However, using those libraries (before this change) in programs (binaries) would
lead to linking errors when compiled for e.g. thumbv6m so this change
effectively shifts a linker error in binaries to a compiler error in libraries.

On a side note: extending the Atomic API is a bit error prone because of these
non-cas targets. Unless the author of the change is aware of these targets and
properly uses `#[cfg(atomic = "cas")]` they could end up exposing new CAS API on
these targets. I can't think of a test to check that an API is not present on
some target, but we could extend the `tidy` tool to check that *all* newly added
atomic API has the `#[cfg(atomic = "cas")]` attribute unless it's whitelisted in
`tidy` then the author of the change would have to verify if the API can be used
on non-cas targets.

In any case, I'd like to plug this hole ASAP. We can revisit testing in a
follow-up issue / PR.

r? @alexcrichton
cc @mvirkkunen
  • Loading branch information
@pietroalbini
pietroalbini committed Sep 22, 2018
2 parents 822c511 + 8282896 commit e3cc48a
Showing 1 changed file with 23 additions and 0 deletions.
23 changes: 23 additions & 0 deletions src/libcore/sync/atomic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ impl AtomicBool {
/// ```
#[inline]
#[stable(feature = "extended_compare_and_swap", since = "1.10.0")]
#[cfg(target_has_atomic = "cas")]
pub fn compare_exchange_weak(&self,
current: bool,
new: bool,
Expand Down Expand Up @@ -1041,6 +1042,7 @@ impl<T> AtomicPtr<T> {
/// ```
#[inline]
#[stable(feature = "extended_compare_and_swap", since = "1.10.0")]
#[cfg(target_has_atomic = "cas")]
pub fn compare_exchange_weak(&self,
current: *mut T,
new: *mut T,
Expand Down Expand Up @@ -1434,6 +1436,7 @@ loop {
```"),
#[inline]
#[$stable_cxchg]
#[cfg(target_has_atomic = "cas")]
pub fn compare_exchange_weak(&self,
current: $int_type,
new: $int_type,
Expand Down Expand Up @@ -1471,6 +1474,7 @@ assert_eq!(foo.load(Ordering::SeqCst), 10);
```"),
#[inline]
#[$stable]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_add(&self, val: $int_type, order: Ordering) -> $int_type {
unsafe { atomic_add(self.v.get(), val, order) }
}
Expand Down Expand Up @@ -1502,6 +1506,7 @@ assert_eq!(foo.load(Ordering::SeqCst), 10);
```"),
#[inline]
#[$stable]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_sub(&self, val: $int_type, order: Ordering) -> $int_type {
unsafe { atomic_sub(self.v.get(), val, order) }
}
Expand Down Expand Up @@ -1536,6 +1541,7 @@ assert_eq!(foo.load(Ordering::SeqCst), 0b100001);
```"),
#[inline]
#[$stable]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_and(&self, val: $int_type, order: Ordering) -> $int_type {
unsafe { atomic_and(self.v.get(), val, order) }
}
Expand Down Expand Up @@ -1571,6 +1577,7 @@ assert_eq!(foo.load(Ordering::SeqCst), !(0x13 & 0x31));
```"),
#[inline]
#[$stable_nand]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_nand(&self, val: $int_type, order: Ordering) -> $int_type {
unsafe { atomic_nand(self.v.get(), val, order) }
}
Expand Down Expand Up @@ -1605,6 +1612,7 @@ assert_eq!(foo.load(Ordering::SeqCst), 0b111111);
```"),
#[inline]
#[$stable]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_or(&self, val: $int_type, order: Ordering) -> $int_type {
unsafe { atomic_or(self.v.get(), val, order) }
}
Expand Down Expand Up @@ -1639,6 +1647,7 @@ assert_eq!(foo.load(Ordering::SeqCst), 0b011110);
```"),
#[inline]
#[$stable]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_xor(&self, val: $int_type, order: Ordering) -> $int_type {
unsafe { atomic_xor(self.v.get(), val, order) }
}
Expand Down Expand Up @@ -1688,6 +1697,7 @@ assert_eq!(x.load(Ordering::SeqCst), 9);
#[unstable(feature = "no_more_cas",
reason = "no more CAS loops in user code",
issue = "48655")]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_update<F>(&self,
mut f: F,
fetch_order: Ordering,
Expand Down Expand Up @@ -1748,6 +1758,7 @@ assert!(max_foo == 42);
#[unstable(feature = "atomic_min_max",
reason = "easier and faster min/max than writing manual CAS loop",
issue = "48655")]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_max(&self, val: $int_type, order: Ordering) -> $int_type {
unsafe { $max_fn(self.v.get(), val, order) }
}
Expand Down Expand Up @@ -1799,6 +1810,7 @@ assert_eq!(min_foo, 12);
#[unstable(feature = "atomic_min_max",
reason = "easier and faster min/max than writing manual CAS loop",
issue = "48655")]
#[cfg(target_has_atomic = "cas")]
pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type {
unsafe { $min_fn(self.v.get(), val, order) }
}
Expand Down Expand Up @@ -1987,6 +1999,7 @@ unsafe fn atomic_swap<T>(dst: *mut T, val: T, order: Ordering) -> T {

/// Returns the previous value (like __sync_fetch_and_add).
#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_add<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_xadd_acq(dst, val),
Expand All @@ -1999,6 +2012,7 @@ unsafe fn atomic_add<T>(dst: *mut T, val: T, order: Ordering) -> T {

/// Returns the previous value (like __sync_fetch_and_sub).
#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_sub<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_xsub_acq(dst, val),
Expand Down Expand Up @@ -2035,6 +2049,7 @@ unsafe fn atomic_compare_exchange<T>(dst: *mut T,
}

#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_compare_exchange_weak<T>(dst: *mut T,
old: T,
new: T,
Expand All @@ -2059,6 +2074,7 @@ unsafe fn atomic_compare_exchange_weak<T>(dst: *mut T,
}

#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_and<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_and_acq(dst, val),
Expand All @@ -2070,6 +2086,7 @@ unsafe fn atomic_and<T>(dst: *mut T, val: T, order: Ordering) -> T {
}

#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_nand<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_nand_acq(dst, val),
Expand All @@ -2081,6 +2098,7 @@ unsafe fn atomic_nand<T>(dst: *mut T, val: T, order: Ordering) -> T {
}

#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_or<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_or_acq(dst, val),
Expand All @@ -2092,6 +2110,7 @@ unsafe fn atomic_or<T>(dst: *mut T, val: T, order: Ordering) -> T {
}

#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_xor<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_xor_acq(dst, val),
Expand All @@ -2104,6 +2123,7 @@ unsafe fn atomic_xor<T>(dst: *mut T, val: T, order: Ordering) -> T {

/// returns the max value (signed comparison)
#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_max<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_max_acq(dst, val),
Expand All @@ -2116,6 +2136,7 @@ unsafe fn atomic_max<T>(dst: *mut T, val: T, order: Ordering) -> T {

/// returns the min value (signed comparison)
#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_min<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_min_acq(dst, val),
Expand All @@ -2128,6 +2149,7 @@ unsafe fn atomic_min<T>(dst: *mut T, val: T, order: Ordering) -> T {

/// returns the max value (signed comparison)
#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_umax<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_umax_acq(dst, val),
Expand All @@ -2140,6 +2162,7 @@ unsafe fn atomic_umax<T>(dst: *mut T, val: T, order: Ordering) -> T {

/// returns the min value (signed comparison)
#[inline]
#[cfg(target_has_atomic = "cas")]
unsafe fn atomic_umin<T>(dst: *mut T, val: T, order: Ordering) -> T {
match order {
Acquire => intrinsics::atomic_umin_acq(dst, val),
Expand Down

0 comments on commit e3cc48a

Please sign in to comment.