Skip to content

Commit 01674fb

Browse files
ollie27nagisa
ollie27
authored and
nagisa
committed
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.
1 parent 10f42cb commit 01674fb

File tree

8 files changed

+169
-20
lines changed

8 files changed

+169
-20
lines changed

‎src/libcore/sync/atomic.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ pub fn spin_loop_hint() {
124124
/// [`bool`]: ../../../std/primitive.bool.html
125125
#[cfg(target_has_atomic = "8")]
126126
#[stable(feature = "rust1", since = "1.0.0")]
127+
#[repr(align(1))]
127128
pub struct AtomicBool {
128129
v: UnsafeCell<u8>,
129130
}
@@ -147,6 +148,9 @@ unsafe impl Sync for AtomicBool {}
147148
/// This type has the same in-memory representation as a `*mut T`.
148149
#[cfg(target_has_atomic = "ptr")]
149150
#[stable(feature = "rust1", since = "1.0.0")]
151+
#[cfg_attr(target_pointer_width = "16", repr(align(2)))]
152+
#[cfg_attr(target_pointer_width = "32", repr(align(4)))]
153+
#[cfg_attr(target_pointer_width = "64", repr(align(8)))]
150154
pub struct AtomicPtr<T> {
151155
p: UnsafeCell<*mut T>,
152156
}
@@ -1088,6 +1092,7 @@ macro_rules! atomic_int {
10881092
$s_int_type:expr, $int_ref:expr,
10891093
$extra_feature:expr,
10901094
$min_fn:ident, $max_fn:ident,
1095+
$align:expr,
10911096
$int_type:ident $atomic_type:ident $atomic_init:ident) => {
10921097
/// An integer type which can be safely shared between threads.
10931098
///
@@ -1101,6 +1106,7 @@ macro_rules! atomic_int {
11011106
///
11021107
/// [module-level documentation]: index.html
11031108
#[$stable]
1109+
#[repr(align($align))]
11041110
pub struct $atomic_type {
11051111
v: UnsafeCell<$int_type>,
11061112
}
@@ -1831,6 +1837,7 @@ atomic_int! {
18311837
"i8", "../../../std/primitive.i8.html",
18321838
"#![feature(integer_atomics)]\n\n",
18331839
atomic_min, atomic_max,
1840+
1,
18341841
i8 AtomicI8 ATOMIC_I8_INIT
18351842
}
18361843
#[cfg(target_has_atomic = "8")]
@@ -1844,6 +1851,7 @@ atomic_int! {
18441851
"u8", "../../../std/primitive.u8.html",
18451852
"#![feature(integer_atomics)]\n\n",
18461853
atomic_umin, atomic_umax,
1854+
1,
18471855
u8 AtomicU8 ATOMIC_U8_INIT
18481856
}
18491857
#[cfg(target_has_atomic = "16")]
@@ -1857,6 +1865,7 @@ atomic_int! {
18571865
"i16", "../../../std/primitive.i16.html",
18581866
"#![feature(integer_atomics)]\n\n",
18591867
atomic_min, atomic_max,
1868+
2,
18601869
i16 AtomicI16 ATOMIC_I16_INIT
18611870
}
18621871
#[cfg(target_has_atomic = "16")]
@@ -1870,6 +1879,7 @@ atomic_int! {
18701879
"u16", "../../../std/primitive.u16.html",
18711880
"#![feature(integer_atomics)]\n\n",
18721881
atomic_umin, atomic_umax,
1882+
2,
18731883
u16 AtomicU16 ATOMIC_U16_INIT
18741884
}
18751885
#[cfg(target_has_atomic = "32")]
@@ -1883,6 +1893,7 @@ atomic_int! {
18831893
"i32", "../../../std/primitive.i32.html",
18841894
"#![feature(integer_atomics)]\n\n",
18851895
atomic_min, atomic_max,
1896+
4,
18861897
i32 AtomicI32 ATOMIC_I32_INIT
18871898
}
18881899
#[cfg(target_has_atomic = "32")]
@@ -1896,6 +1907,7 @@ atomic_int! {
18961907
"u32", "../../../std/primitive.u32.html",
18971908
"#![feature(integer_atomics)]\n\n",
18981909
atomic_umin, atomic_umax,
1910+
4,
18991911
u32 AtomicU32 ATOMIC_U32_INIT
19001912
}
19011913
#[cfg(target_has_atomic = "64")]
@@ -1909,6 +1921,7 @@ atomic_int! {
19091921
"i64", "../../../std/primitive.i64.html",
19101922
"#![feature(integer_atomics)]\n\n",
19111923
atomic_min, atomic_max,
1924+
8,
19121925
i64 AtomicI64 ATOMIC_I64_INIT
19131926
}
19141927
#[cfg(target_has_atomic = "64")]
@@ -1922,8 +1935,49 @@ atomic_int! {
19221935
"u64", "../../../std/primitive.u64.html",
19231936
"#![feature(integer_atomics)]\n\n",
19241937
atomic_umin, atomic_umax,
1938+
8,
19251939
u64 AtomicU64 ATOMIC_U64_INIT
19261940
}
1941+
#[cfg(target_has_atomic = "128")]
1942+
atomic_int! {
1943+
unstable(feature = "integer_atomics", issue = "32976"),
1944+
unstable(feature = "integer_atomics", issue = "32976"),
1945+
unstable(feature = "integer_atomics", issue = "32976"),
1946+
unstable(feature = "integer_atomics", issue = "32976"),
1947+
unstable(feature = "integer_atomics", issue = "32976"),
1948+
unstable(feature = "integer_atomics", issue = "32976"),
1949+
"i128", "../../../std/primitive.i128.html",
1950+
"#![feature(integer_atomics)]\n\n",
1951+
atomic_min, atomic_max,
1952+
16,
1953+
i128 AtomicI128 ATOMIC_I128_INIT
1954+
}
1955+
#[cfg(target_has_atomic = "128")]
1956+
atomic_int! {
1957+
unstable(feature = "integer_atomics", issue = "32976"),
1958+
unstable(feature = "integer_atomics", issue = "32976"),
1959+
unstable(feature = "integer_atomics", issue = "32976"),
1960+
unstable(feature = "integer_atomics", issue = "32976"),
1961+
unstable(feature = "integer_atomics", issue = "32976"),
1962+
unstable(feature = "integer_atomics", issue = "32976"),
1963+
"u128", "../../../std/primitive.u128.html",
1964+
"#![feature(integer_atomics)]\n\n",
1965+
atomic_umin, atomic_umax,
1966+
16,
1967+
u128 AtomicU128 ATOMIC_U128_INIT
1968+
}
1969+
#[cfg(target_pointer_width = "16")]
1970+
macro_rules! ptr_width {
1971+
() => { 2 }
1972+
}
1973+
#[cfg(target_pointer_width = "32")]
1974+
macro_rules! ptr_width {
1975+
() => { 4 }
1976+
}
1977+
#[cfg(target_pointer_width = "64")]
1978+
macro_rules! ptr_width {
1979+
() => { 8 }
1980+
}
19271981
#[cfg(target_has_atomic = "ptr")]
19281982
atomic_int!{
19291983
stable(feature = "rust1", since = "1.0.0"),
@@ -1935,6 +1989,7 @@ atomic_int!{
19351989
"isize", "../../../std/primitive.isize.html",
19361990
"",
19371991
atomic_min, atomic_max,
1992+
ptr_width!(),
19381993
isize AtomicIsize ATOMIC_ISIZE_INIT
19391994
}
19401995
#[cfg(target_has_atomic = "ptr")]
@@ -1948,6 +2003,7 @@ atomic_int!{
19482003
"usize", "../../../std/primitive.usize.html",
19492004
"",
19502005
atomic_umin, atomic_umax,
2006+
ptr_width!(),
19512007
usize AtomicUsize ATOMIC_USIZE_INIT
19522008
}
19532009

‎src/librustc_codegen_llvm/builder.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -482,14 +482,12 @@ impl Builder<'a, 'll, 'tcx> {
482482
}
483483
}
484484

485-
pub fn atomic_load(&self, ptr: &'ll Value, order: AtomicOrdering, align: Align) -> &'ll Value {
485+
pub fn atomic_load(&self, ptr: &'ll Value, order: AtomicOrdering, size: Size) -> &'ll Value {
486486
self.count_insn("load.atomic");
487487
unsafe {
488488
let load = llvm::LLVMRustBuildAtomicLoad(self.llbuilder, ptr, noname(), order);
489-
// FIXME(eddyb) Isn't it UB to use `pref` instead of `abi` here?
490-
// However, 64-bit atomic loads on `i686-apple-darwin` appear to
491-
// require `___atomic_load` with ABI-alignment, so it's staying.
492-
llvm::LLVMSetAlignment(load, align.pref() as c_uint);
489+
// LLVM requires the alignment of atomic loads to be at least the size of the type.
490+
llvm::LLVMSetAlignment(load, size.bytes() as c_uint);
493491
load
494492
}
495493
}
@@ -564,15 +562,14 @@ impl Builder<'a, 'll, 'tcx> {
564562
}
565563

566564
pub fn atomic_store(&self, val: &'ll Value, ptr: &'ll Value,
567-
order: AtomicOrdering, align: Align) {
565+
order: AtomicOrdering, size: Size) {
568566
debug!("Store {:?} -> {:?}", val, ptr);
569567
self.count_insn("store.atomic");
570568
let ptr = self.check_store(val, ptr);
571569
unsafe {
572570
let store = llvm::LLVMRustBuildAtomicStore(self.llbuilder, val, ptr, order);
573-
// FIXME(eddyb) Isn't it UB to use `pref` instead of `abi` here?
574-
// Also see `atomic_load` for more context.
575-
llvm::LLVMSetAlignment(store, align.pref() as c_uint);
571+
// LLVM requires the alignment of atomic stores to be at least the size of the type.
572+
llvm::LLVMSetAlignment(store, size.bytes() as c_uint);
576573
}
577574
}
578575

‎src/librustc_codegen_llvm/intrinsic.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,8 @@ pub fn codegen_intrinsic_call(
477477
"load" => {
478478
let ty = substs.type_at(0);
479479
if int_type_width_signed(ty, cx).is_some() {
480-
let align = cx.align_of(ty);
481-
bx.atomic_load(args[0].immediate(), order, align)
480+
let size = cx.size_of(ty);
481+
bx.atomic_load(args[0].immediate(), order, size)
482482
} else {
483483
return invalid_monomorphization(ty);
484484
}
@@ -487,8 +487,8 @@ pub fn codegen_intrinsic_call(
487487
"store" => {
488488
let ty = substs.type_at(0);
489489
if int_type_width_signed(ty, cx).is_some() {
490-
let align = cx.align_of(ty);
491-
bx.atomic_store(args[1].immediate(), args[0].immediate(), order, align);
490+
let size = cx.size_of(ty);
491+
bx.atomic_store(args[1].immediate(), args[0].immediate(), order, size);
492492
return;
493493
} else {
494494
return invalid_monomorphization(ty);

‎src/libstd/panic.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@ impl RefUnwindSafe for atomic::AtomicI32 {}
264264
#[cfg(target_has_atomic = "64")]
265265
#[unstable(feature = "integer_atomics", issue = "32976")]
266266
impl RefUnwindSafe for atomic::AtomicI64 {}
267+
#[cfg(target_has_atomic = "128")]
268+
#[unstable(feature = "integer_atomics", issue = "32976")]
269+
impl RefUnwindSafe for atomic::AtomicI128 {}
267270

268271
#[cfg(target_has_atomic = "ptr")]
269272
#[stable(feature = "unwind_safe_atomic_refs", since = "1.14.0")]
@@ -280,6 +283,9 @@ impl RefUnwindSafe for atomic::AtomicU32 {}
280283
#[cfg(target_has_atomic = "64")]
281284
#[unstable(feature = "integer_atomics", issue = "32976")]
282285
impl RefUnwindSafe for atomic::AtomicU64 {}
286+
#[cfg(target_has_atomic = "128")]
287+
#[unstable(feature = "integer_atomics", issue = "32976")]
288+
impl RefUnwindSafe for atomic::AtomicU128 {}
283289

284290
#[cfg(target_has_atomic = "8")]
285291
#[stable(feature = "unwind_safe_atomic_refs", since = "1.14.0")]

‎src/test/run-make-fulldeps/atomic-lock-free/atomic_lock_free.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@ pub unsafe fn atomic_u64(x: *mut u64) {
5858
pub unsafe fn atomic_i64(x: *mut i64) {
5959
atomic_xadd(x, 1);
6060
}
61+
#[cfg(target_has_atomic = "128")]
62+
pub unsafe fn atomic_u128(x: *mut u128) {
63+
atomic_xadd(x, 1);
64+
}
65+
#[cfg(target_has_atomic = "128")]
66+
pub unsafe fn atomic_i128(x: *mut i128) {
67+
atomic_xadd(x, 1);
68+
}
6169
#[cfg(target_has_atomic = "ptr")]
6270
pub unsafe fn atomic_usize(x: *mut usize) {
6371
atomic_xadd(x, 1);

‎src/test/run-pass/atomic-alignment.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(cfg_target_has_atomic)]
12+
#![feature(integer_atomics)]
13+
14+
use std::mem::{align_of, size_of};
15+
use std::sync::atomic::*;
16+
17+
fn main() {
18+
#[cfg(target_has_atomic = "8")]
19+
assert_eq!(align_of::<AtomicBool>(), size_of::<AtomicBool>());
20+
#[cfg(target_has_atomic = "ptr")]
21+
assert_eq!(align_of::<AtomicPtr<u8>>(), size_of::<AtomicPtr<u8>>());
22+
#[cfg(target_has_atomic = "8")]
23+
assert_eq!(align_of::<AtomicU8>(), size_of::<AtomicU8>());
24+
#[cfg(target_has_atomic = "8")]
25+
assert_eq!(align_of::<AtomicI8>(), size_of::<AtomicI8>());
26+
#[cfg(target_has_atomic = "16")]
27+
assert_eq!(align_of::<AtomicU16>(), size_of::<AtomicU16>());
28+
#[cfg(target_has_atomic = "16")]
29+
assert_eq!(align_of::<AtomicI16>(), size_of::<AtomicI16>());
30+
#[cfg(target_has_atomic = "32")]
31+
assert_eq!(align_of::<AtomicU32>(), size_of::<AtomicU32>());
32+
#[cfg(target_has_atomic = "32")]
33+
assert_eq!(align_of::<AtomicI32>(), size_of::<AtomicI32>());
34+
#[cfg(target_has_atomic = "64")]
35+
assert_eq!(align_of::<AtomicU64>(), size_of::<AtomicU64>());
36+
#[cfg(target_has_atomic = "64")]
37+
assert_eq!(align_of::<AtomicI64>(), size_of::<AtomicI64>());
38+
#[cfg(target_has_atomic = "128")]
39+
assert_eq!(align_of::<AtomicU128>(), size_of::<AtomicU128>());
40+
#[cfg(target_has_atomic = "128")]
41+
assert_eq!(align_of::<AtomicI128>(), size_of::<AtomicI128>());
42+
#[cfg(target_has_atomic = "ptr")]
43+
assert_eq!(align_of::<AtomicUsize>(), size_of::<AtomicUsize>());
44+
#[cfg(target_has_atomic = "ptr")]
45+
assert_eq!(align_of::<AtomicIsize>(), size_of::<AtomicIsize>());
46+
}

‎src/test/ui/feature-gates/feature-gate-cfg-target-has-atomic.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,16 @@ pub unsafe fn atomic_u64(x: *mut u64) {
6161
pub unsafe fn atomic_i64(x: *mut i64) {
6262
atomic_xadd(x, 1);
6363
}
64+
#[cfg(target_has_atomic = "128")]
65+
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
66+
pub unsafe fn atomic_u128(x: *mut u128) {
67+
atomic_xadd(x, 1);
68+
}
69+
#[cfg(target_has_atomic = "128")]
70+
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
71+
pub unsafe fn atomic_i128(x: *mut i128) {
72+
atomic_xadd(x, 1);
73+
}
6474
#[cfg(target_has_atomic = "ptr")]
6575
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
6676
pub unsafe fn atomic_usize(x: *mut usize) {
@@ -81,6 +91,8 @@ fn main() {
8191
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
8292
cfg!(target_has_atomic = "64");
8393
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
94+
cfg!(target_has_atomic = "128");
95+
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
8496
cfg!(target_has_atomic = "ptr");
8597
//~^ ERROR `cfg(target_has_atomic)` is experimental and subject to change (see issue #32976)
8698
}

0 commit comments

Comments
 (0)