From: Oliver Middleton Date: Sat, 8 Sep 2018 11:50:19 +0000 (+0100) Subject: Correct alignment of atomic types and (re)add Atomic{I,U}128 X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=01674fbe06aa2d05f8dafbb8a27ba6bd23fa09e1;p=rust.git Correct alignment of atomic types and (re)add Atomic{I,U}128 LLVM requires that atomic loads and stores be aligned to at least the size of the type. --- diff --git a/src/libcore/sync/atomic.rs b/src/libcore/sync/atomic.rs index f130dbfb0e3..b9c3123936d 100644 --- a/src/libcore/sync/atomic.rs +++ b/src/libcore/sync/atomic.rs @@ -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, } @@ -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 { 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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "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 @@ pub fn fetch_min(&self, val: $int_type, order: Ordering) -> $int_type { "usize", "../../../std/primitive.usize.html", "", atomic_umin, atomic_umax, + ptr_width!(), usize AtomicUsize ATOMIC_USIZE_INIT } diff --git a/src/librustc_codegen_llvm/builder.rs b/src/librustc_codegen_llvm/builder.rs index 2fe6a0377f8..f70a68c7248 100644 --- a/src/librustc_codegen_llvm/builder.rs +++ b/src/librustc_codegen_llvm/builder.rs @@ -482,14 +482,12 @@ pub fn volatile_load(&self, ptr: &'ll Value) -> &'ll Value { } } - 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 @@ pub fn store_with_flags( } 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); } } diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 272196afa6f..596a1d5e8a5 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -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); diff --git a/src/libstd/panic.rs b/src/libstd/panic.rs index 5c87035d8e9..137ba7d449a 100644 --- a/src/libstd/panic.rs +++ b/src/libstd/panic.rs @@ -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")] diff --git a/src/test/run-make-fulldeps/atomic-lock-free/atomic_lock_free.rs b/src/test/run-make-fulldeps/atomic-lock-free/atomic_lock_free.rs index 54f888b3796..2c8128b1907 100644 --- a/src/test/run-make-fulldeps/atomic-lock-free/atomic_lock_free.rs +++ b/src/test/run-make-fulldeps/atomic-lock-free/atomic_lock_free.rs @@ -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); diff --git a/src/test/run-pass/atomic-alignment.rs b/src/test/run-pass/atomic-alignment.rs new file mode 100644 index 00000000000..8771765de29 --- /dev/null +++ b/src/test/run-pass/atomic-alignment.rs @@ -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 or the MIT license +// , 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::(), size_of::()); + #[cfg(target_has_atomic = "ptr")] + assert_eq!(align_of::>(), size_of::>()); + #[cfg(target_has_atomic = "8")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "8")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "16")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "16")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "32")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "32")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "64")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "64")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "128")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "128")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "ptr")] + assert_eq!(align_of::(), size_of::()); + #[cfg(target_has_atomic = "ptr")] + assert_eq!(align_of::(), size_of::()); +} diff --git a/src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.rs b/src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.rs index aa27f8922c0..6b70c1ea294 100644 --- a/src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.rs +++ b/src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.rs @@ -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) } diff --git a/src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.stderr b/src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.stderr index f3975b7ce8b..81f20112a12 100644 --- a/src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.stderr +++ b/src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.stderr @@ -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: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:76:10 + --> $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`.