bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Correct alignment of atomic types and (re)add Atomic{I,U}128

Browse source 
LLVM requires that atomic loads and stores be aligned to at least the size of the type.
This commit is contained in:
Oliver Middleton 2018-09-08 12:50:19 +01:00 • committed by Simonas Kazlauskas
parent 10f42cbde0
commit 01674fbe06
8 changed files with 169 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

56
src/libcore/sync/atomic.rs
View file

@ -124,6 +124,7 @@ pub fn spin_loop_hint() {
/// [`bool`]: ../../../std/primitive.bool.html
#[cfg(target_has_atomic = "8")]
#[stable(feature = "rust1", since = "1.0.0")]
#[repr(align(1))]
pub struct AtomicBool {
v: UnsafeCell<u8>,
}
@ -147,6 +148,9 @@ unsafe impl Sync for AtomicBool {}
/// This type has the same in-memory representation as a `*mut T`.
#[cfg(target_has_atomic = "ptr")]
#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(target_pointer_width = "16", repr(align(2)))]
#[cfg_attr(target_pointer_width = "32", repr(align(4)))]
#[cfg_attr(target_pointer_width = "64", repr(align(8)))]
pub struct AtomicPtr<T> {
p: UnsafeCell<*mut T>,
}
@ -1088,6 +1092,7 @@ macro_rules! atomic_int {
$s_int_type:expr, $int_ref:expr,
$extra_feature:expr,
$min_fn:ident, $max_fn:ident,
$align:expr,
$int_type:ident $atomic_type:ident $atomic_init:ident) => {
/// An integer type which can be safely shared between threads.
///
@ -1101,6 +1106,7 @@ macro_rules! atomic_int {
///
/// [module-level documentation]: index.html
#[$stable]
#[repr(align($align))]
pub struct $atomic_type {
v: UnsafeCell<$int_type>,
}
@ -1831,6 +1837,7 @@ atomic_int! {
"i8", "../../../std/primitive.i8.html",
"#![feature(integer_atomics)]\n\n",
atomic_min, atomic_max,
1,
i8 AtomicI8 ATOMIC_I8_INIT
}
#[cfg(target_has_atomic = "8")]
@ -1844,6 +1851,7 @@ atomic_int! {
"u8", "../../../std/primitive.u8.html",
"#![feature(integer_atomics)]\n\n",
atomic_umin, atomic_umax,
1,
u8 AtomicU8 ATOMIC_U8_INIT
}
#[cfg(target_has_atomic = "16")]
@ -1857,6 +1865,7 @@ atomic_int! {
"i16", "../../../std/primitive.i16.html",
"#![feature(integer_atomics)]\n\n",
atomic_min, atomic_max,
2,
i16 AtomicI16 ATOMIC_I16_INIT
}
#[cfg(target_has_atomic = "16")]
@ -1870,6 +1879,7 @@ atomic_int! {
"u16", "../../../std/primitive.u16.html",
"#![feature(integer_atomics)]\n\n",
atomic_umin, atomic_umax,
2,
u16 AtomicU16 ATOMIC_U16_INIT
}
#[cfg(target_has_atomic = "32")]
@ -1883,6 +1893,7 @@ atomic_int! {
"i32", "../../../std/primitive.i32.html",
"#![feature(integer_atomics)]\n\n",
atomic_min, atomic_max,
4,
i32 AtomicI32 ATOMIC_I32_INIT
}
#[cfg(target_has_atomic = "32")]
@ -1896,6 +1907,7 @@ atomic_int! {
"u32", "../../../std/primitive.u32.html",
"#![feature(integer_atomics)]\n\n",
atomic_umin, atomic_umax,
4,
u32 AtomicU32 ATOMIC_U32_INIT
}
#[cfg(target_has_atomic = "64")]
@ -1909,6 +1921,7 @@ atomic_int! {
"i64", "../../../std/primitive.i64.html",
"#![feature(integer_atomics)]\n\n",
atomic_min, atomic_max,
8,
i64 AtomicI64 ATOMIC_I64_INIT
}
#[cfg(target_has_atomic = "64")]
@ -1922,8 +1935,49 @@ atomic_int! {
"u64", "../../../std/primitive.u64.html",
"#![feature(integer_atomics)]\n\n",
atomic_umin, atomic_umax,
8,
u64 AtomicU64 ATOMIC_U64_INIT
}
#[cfg(target_has_atomic = "128")]
atomic_int! {
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
"i128", "../../../std/primitive.i128.html",
"#![feature(integer_atomics)]\n\n",
atomic_min, atomic_max,
16,
i128 AtomicI128 ATOMIC_I128_INIT
}
#[cfg(target_has_atomic = "128")]
atomic_int! {
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
unstable(feature = "integer_atomics", issue = "32976"),
"u128", "../../../std/primitive.u128.html",
"#![feature(integer_atomics)]\n\n",
atomic_umin, atomic_umax,
16,
u128 AtomicU128 ATOMIC_U128_INIT
}
#[cfg(target_pointer_width = "16")]
macro_rules! ptr_width {
() => { 2 }
}
#[cfg(target_pointer_width = "32")]
macro_rules! ptr_width {
() => { 4 }
}
#[cfg(target_pointer_width = "64")]
macro_rules! ptr_width {
() => { 8 }
}
#[cfg(target_has_atomic = "ptr")]
atomic_int!{
stable(feature = "rust1", since = "1.0.0"),
@ -1935,6 +1989,7 @@ atomic_int!{
"isize", "../../../std/primitive.isize.html",
"",
atomic_min, atomic_max,
ptr_width!(),
isize AtomicIsize ATOMIC_ISIZE_INIT
}
#[cfg(target_has_atomic = "ptr")]
@ -1948,6 +2003,7 @@ atomic_int!{
"usize", "../../../std/primitive.usize.html",
"",
atomic_umin, atomic_umax,
ptr_width!(),
usize AtomicUsize ATOMIC_USIZE_INIT
}

15
src/librustc_codegen_llvm/builder.rs
View file

@ -482,14 +482,12 @@ impl Builder<'a, 'll, 'tcx> {
}
}
pub fn atomic_load(&self, ptr: &'ll Value, order: AtomicOrdering, align: Align) -> &'ll Value {
pub fn atomic_load(&self, ptr: &'ll Value, order: AtomicOrdering, size: Size) -> &'ll Value {
self.count_insn("load.atomic");
unsafe {
let load = llvm::LLVMRustBuildAtomicLoad(self.llbuilder, ptr, noname(), order);
// FIXME(eddyb) Isn't it UB to use `pref` instead of `abi` here?
// However, 64-bit atomic loads on `i686-apple-darwin` appear to
// require `___atomic_load` with ABI-alignment, so it's staying.
llvm::LLVMSetAlignment(load, align.pref() as c_uint);
// LLVM requires the alignment of atomic loads to be at least the size of the type.
llvm::LLVMSetAlignment(load, size.bytes() as c_uint);
load
}
}
@ -564,15 +562,14 @@ impl Builder<'a, 'll, 'tcx> {
}
pub fn atomic_store(&self, val: &'ll Value, ptr: &'ll Value,
order: AtomicOrdering, align: Align) {
order: AtomicOrdering, size: Size) {
debug!("Store {:?} -> {:?}", val, ptr);
self.count_insn("store.atomic");
let ptr = self.check_store(val, ptr);
unsafe {
let store = llvm::LLVMRustBuildAtomicStore(self.llbuilder, val, ptr, order);
// FIXME(eddyb) Isn't it UB to use `pref` instead of `abi` here?
// Also see `atomic_load` for more context.
llvm::LLVMSetAlignment(store, align.pref() as c_uint);
// LLVM requires the alignment of atomic stores to be at least the size of the type.
llvm::LLVMSetAlignment(store, size.bytes() as c_uint);
}
}

8
src/librustc_codegen_llvm/intrinsic.rs
View file

@ -477,8 +477,8 @@ pub fn codegen_intrinsic_call(
"load" => {
let ty = substs.type_at(0);
if int_type_width_signed(ty, cx).is_some() {
let align = cx.align_of(ty);
bx.atomic_load(args[0].immediate(), order, align)
let size = cx.size_of(ty);
bx.atomic_load(args[0].immediate(), order, size)
} else {
return invalid_monomorphization(ty);
}
@ -487,8 +487,8 @@ pub fn codegen_intrinsic_call(
"store" => {
let ty = substs.type_at(0);
if int_type_width_signed(ty, cx).is_some() {
let align = cx.align_of(ty);
bx.atomic_store(args[1].immediate(), args[0].immediate(), order, align);
let size = cx.size_of(ty);
bx.atomic_store(args[1].immediate(), args[0].immediate(), order, size);
return;
} else {
return invalid_monomorphization(ty);

6
src/libstd/panic.rs
View file

@ -264,6 +264,9 @@ impl RefUnwindSafe for atomic::AtomicI32 {}
#[cfg(target_has_atomic = "64")]
#[unstable(feature = "integer_atomics", issue = "32976")]
impl RefUnwindSafe for atomic::AtomicI64 {}
#[cfg(target_has_atomic = "128")]
#[unstable(feature = "integer_atomics", issue = "32976")]
impl RefUnwindSafe for atomic::AtomicI128 {}
#[cfg(target_has_atomic = "ptr")]
#[stable(feature = "unwind_safe_atomic_refs", since = "1.14.0")]
@ -280,6 +283,9 @@ impl RefUnwindSafe for atomic::AtomicU32 {}
#[cfg(target_has_atomic = "64")]
#[unstable(feature = "integer_atomics", issue = "32976")]
impl RefUnwindSafe for atomic::AtomicU64 {}
#[cfg(target_has_atomic = "128")]
#[unstable(feature = "integer_atomics", issue = "32976")]
impl RefUnwindSafe for atomic::AtomicU128 {}
#[cfg(target_has_atomic = "8")]
#[stable(feature = "unwind_safe_atomic_refs", since = "1.14.0")]

8
src/test/run-make-fulldeps/atomic-lock-free/atomic_lock_free.rs
View file

@ -58,6 +58,14 @@ pub unsafe fn atomic_u64(x: *mut u64) {
pub unsafe fn atomic_i64(x: *mut i64) {
atomic_xadd(x, 1);
}
#[cfg(target_has_atomic = "128")]
pub unsafe fn atomic_u128(x: *mut u128) {
atomic_xadd(x, 1);
}
#[cfg(target_has_atomic = "128")]
pub unsafe fn atomic_i128(x: *mut i128) {
atomic_xadd(x, 1);
}
#[cfg(target_has_atomic = "ptr")]
pub unsafe fn atomic_usize(x: *mut usize) {
atomic_xadd(x, 1);

46
src/test/run-pass/atomic-alignment.rs Normal file
View file

@ -0,0 +1,46 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(cfg_target_has_atomic)]
#![feature(integer_atomics)]
use std::mem::{align_of, size_of};
use std::sync::atomic::*;
fn main() {
#[cfg(target_has_atomic = "8")]
assert_eq!(align_of::<AtomicBool>(), size_of::<AtomicBool>());
#[cfg(target_has_atomic = "ptr")]
assert_eq!(align_of::<AtomicPtr<u8>>(), size_of::<AtomicPtr<u8>>());
#[cfg(target_has_atomic = "8")]
assert_eq!(align_of::<AtomicU8>(), size_of::<AtomicU8>());
#[cfg(target_has_atomic = "8")]
assert_eq!(align_of::<AtomicI8>(), size_of::<AtomicI8>());
#[cfg(target_has_atomic = "16")]
assert_eq!(align_of::<AtomicU16>(), size_of::<AtomicU16>());
#[cfg(target_has_atomic = "16")]
assert_eq!(align_of::<AtomicI16>(), size_of::<AtomicI16>());
#[cfg(target_has_atomic = "32")]
assert_eq!(align_of::<AtomicU32>(), size_of::<AtomicU32>());
#[cfg(target_has_atomic = "32")]
assert_eq!(align_of::<AtomicI32>(), size_of::<AtomicI32>());
#[cfg(target_has_atomic = "64")]
assert_eq!(align_of::<AtomicU64>(), size_of::<AtomicU64>());
#[cfg(target_has_atomic = "64")]
assert_eq!(align_of::<AtomicI64>(), size_of::<AtomicI64>());
#[cfg(target_has_atomic = "128")]
assert_eq!(align_of::<AtomicU128>(), size_of::<AtomicU128>());
#[cfg(target_has_atomic = "128")]
assert_eq!(align_of::<AtomicI128>(), size_of::<AtomicI128>());
#[cfg(target_has_atomic = "ptr")]
assert_eq!(align_of::<AtomicUsize>(), size_of::<AtomicUsize>());
#[cfg(target_has_atomic = "ptr")]
assert_eq!(align_of::<AtomicIsize>(), size_of::<AtomicIsize>());
}

12
src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.rs
View file

@ -61,6 +61,16 @@ pub unsafe fn atomic_u64(x: *mut u64) {
pub unsafe fn atomic_i64(x: *mut i64) {
atomic_xadd(x, 1);
}
#[cfg(target_has_atomic = "128")]
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
pub unsafe fn atomic_u128(x: *mut u128) {
atomic_xadd(x, 1);
}
#[cfg(target_has_atomic = "128")]
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
pub unsafe fn atomic_i128(x: *mut i128) {
atomic_xadd(x, 1);
}
#[cfg(target_has_atomic = "ptr")]
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
pub unsafe fn atomic_usize(x: *mut usize) {
@ -81,6 +91,8 @@ fn main() {
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
cfg!(target_has_atomic = "64");
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
cfg!(target_has_atomic = "128");
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
cfg!(target_has_atomic = "ptr");
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
}

38
src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.stderr
View file

@ -65,7 +65,7 @@ LL | #[cfg(target_has_atomic = "64")]
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:64:7
|
LL | #[cfg(target_has_atomic = "ptr")]
LL | #[cfg(target_has_atomic = "128")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
@ -73,13 +73,29 @@ LL | #[cfg(target_has_atomic = "ptr")]
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:69:7
|
LL | #[cfg(target_has_atomic = "128")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:74:7
|
LL | #[cfg(target_has_atomic = "ptr")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:76:10
--> $DIR/feature-gate-cfg-target-has-atomic.rs:79:7
|
LL | #[cfg(target_has_atomic = "ptr")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:86:10
|
LL | cfg!(target_has_atomic = "8");
| ^^^^^^^^^^^^^^^^^^^^^^^
@ -87,7 +103,7 @@ LL | cfg!(target_has_atomic = "8");
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:78:10
--> $DIR/feature-gate-cfg-target-has-atomic.rs:88:10
|
LL | cfg!(target_has_atomic = "16");
| ^^^^^^^^^^^^^^^^^^^^^^^^
@ -95,7 +111,7 @@ LL | cfg!(target_has_atomic = "16");
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:80:10
--> $DIR/feature-gate-cfg-target-has-atomic.rs:90:10
|
LL | cfg!(target_has_atomic = "32");
| ^^^^^^^^^^^^^^^^^^^^^^^^
@ -103,7 +119,7 @@ LL | cfg!(target_has_atomic = "32");
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:82:10
--> $DIR/feature-gate-cfg-target-has-atomic.rs:92:10
|
LL | cfg!(target_has_atomic = "64");
| ^^^^^^^^^^^^^^^^^^^^^^^^
@ -111,13 +127,21 @@ LL | cfg!(target_has_atomic = "64");
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:84:10
--> $DIR/feature-gate-cfg-target-has-atomic.rs:94:10
|
LL | cfg!(target_has_atomic = "128");
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error[E0658]: `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
--> $DIR/feature-gate-cfg-target-has-atomic.rs:96:10
|
LL | cfg!(target_has_atomic = "ptr");
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add #![feature(cfg_target_has_atomic)] to the crate attributes to enable
error: aborting due to 15 previous errors
error: aborting due to 18 previous errors
For more information about this error, try `rustc --explain E0658`.