Skip to content

Commit 1df69bc

Browse files
Rollup merge of #141803 - workingjubilee:remove-pref-align, r=bjorn3
Remove rustc's notion of "preferred" alignment AKA `__alignof` In PR #90877 T-lang decided not to remove `intrinsics::pref_align_of`. However, the intrinsic and its supporting code 1. is a nightly feature, so can be removed at compiler/libs discretion 2. requires considerable effort in the compiler to support, as it necessarily complicates every single site reasoning about alignment 3. has been justified based on relevance to codegen, but it is only a requirement for C++ (not C, not Rust) stack frame layout for AIX[^1], in ways Rust would not consider even with increased C++ interop 4. is only used by rustc to overalign some globals, not correctness[^2] 5. can be adequately replaced by other rules for globals, as it mostly affects alignments for a few types under 16 bytes of alignment 6. has only one clear beneficiary: automating C -> Rust translation for GNU extensions like `__alignof`[^3] 7. such code was likely intended to be `alignof` or `_Alignof`, because the GNU extension is a "false friend" of the C keyword, which makes the choice to support such a mapping very questionable 8. makes it easy to do incorrect codegen in the compiler by its mere presence as usual Rust rules of alignment (e.g. `size == align * N`) do not hold with preferred alignment[^4] Despite an automated translation tool like c2rust using it, we have made multiple attempts to find a crate that actually has been committed to public repositories, like GitHub or crates.io, using such translated code. We have found none. While it is possible someone privately uses this intrinsic, it seems unlikely, and it is behind a feature gate that will warn about using the internal features of rustc. The implementation is clearly damaging the code quality of the compiler. Thus it is within the compiler team's purview to simply rip it out. If T-lang wishes to have this intrinsic restored for c2rust's benefit, it would have to use a radically different implementation that somehow does not cause internal incorrectness. Until then, remove the intrinsic and its supporting code, as one tool and an ill-considered GCC extension cannot justify risking correctness. Because we touch a fair amount of the compiler to change this at all, and unfortunately the duplication of AbiAndPrefAlign is deep-rooted, we keep an "AbiAlign" type which we can wean code off later. [^1]: #91971 (comment) [^2]: as viewable in the code altered by this PR [^3]: c2rust: https://github.com/immunant/c2rust/blame/3b1ec86b9b0cf363adfd3178cc45a891a970eef2/c2rust-transpile/src/translator/mod.rs#L3175 [^4]: rust-lang/rustc_codegen_cranelift#1560
2 parents 6ccd447 + ec13ae6 commit 1df69bc

31 files changed

+239
-445
lines changed

compiler/rustc_abi/src/layout.rs

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_index::bit_set::BitMatrix;
88
use tracing::debug;
99

1010
use crate::{
11-
AbiAndPrefAlign, Align, BackendRepr, FieldsShape, HasDataLayout, IndexSlice, IndexVec, Integer,
11+
AbiAlign, Align, BackendRepr, FieldsShape, HasDataLayout, IndexSlice, IndexVec, Integer,
1212
LayoutData, Niche, NonZeroUsize, Primitive, ReprOptions, Scalar, Size, StructKind, TagEncoding,
1313
Variants, WrappingRange,
1414
};
@@ -173,13 +173,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
173173
// Non-power-of-two vectors have padding up to the next power-of-two.
174174
// If we're a packed repr, remove the padding while keeping the alignment as close
175175
// to a vector as possible.
176-
(
177-
BackendRepr::Memory { sized: true },
178-
AbiAndPrefAlign {
179-
abi: Align::max_aligned_factor(size),
180-
pref: dl.llvmlike_vector_align(size).pref,
181-
},
182-
)
176+
(BackendRepr::Memory { sized: true }, AbiAlign { abi: Align::max_aligned_factor(size) })
183177
} else {
184178
(BackendRepr::SimdVector { element: e_repr, count }, dl.llvmlike_vector_align(size))
185179
};
@@ -435,13 +429,13 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
435429
}
436430

437431
if let Some(pack) = repr.pack {
438-
align = align.min(AbiAndPrefAlign::new(pack));
432+
align = align.min(AbiAlign::new(pack));
439433
}
440434
// The unadjusted ABI alignment does not include repr(align), but does include repr(pack).
441435
// See documentation on `LayoutS::unadjusted_abi_align`.
442436
let unadjusted_abi_align = align.abi;
443437
if let Some(repr_align) = repr.align {
444-
align = align.max(AbiAndPrefAlign::new(repr_align));
438+
align = align.max(AbiAlign::new(repr_align));
445439
}
446440
// `align` must not be modified after this, or `unadjusted_abi_align` could be inaccurate.
447441
let align = align;
@@ -1289,7 +1283,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
12891283
if let StructKind::Prefixed(prefix_size, prefix_align) = kind {
12901284
let prefix_align =
12911285
if let Some(pack) = pack { prefix_align.min(pack) } else { prefix_align };
1292-
align = align.max(AbiAndPrefAlign::new(prefix_align));
1286+
align = align.max(AbiAlign::new(prefix_align));
12931287
offset = prefix_size.align_to(prefix_align);
12941288
}
12951289
for &i in &inverse_memory_index {
@@ -1308,7 +1302,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
13081302

13091303
// Invariant: offset < dl.obj_size_bound() <= 1<<61
13101304
let field_align = if let Some(pack) = pack {
1311-
field.align.min(AbiAndPrefAlign::new(pack))
1305+
field.align.min(AbiAlign::new(pack))
13121306
} else {
13131307
field.align
13141308
};
@@ -1342,7 +1336,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
13421336
// See documentation on `LayoutS::unadjusted_abi_align`.
13431337
let unadjusted_abi_align = align.abi;
13441338
if let Some(repr_align) = repr.align {
1345-
align = align.max(AbiAndPrefAlign::new(repr_align));
1339+
align = align.max(AbiAlign::new(repr_align));
13461340
}
13471341
// `align` must not be modified after this point, or `unadjusted_abi_align` could be inaccurate.
13481342
let align = align;

compiler/rustc_abi/src/layout/ty.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustc_data_structures::intern::Interned;
55
use rustc_macros::HashStable_Generic;
66

77
use crate::{
8-
AbiAndPrefAlign, Align, BackendRepr, FieldsShape, Float, HasDataLayout, LayoutData, Niche,
8+
AbiAlign, Align, BackendRepr, FieldsShape, Float, HasDataLayout, LayoutData, Niche,
99
PointeeInfo, Primitive, Scalar, Size, TargetDataLayout, Variants,
1010
};
1111

@@ -100,7 +100,7 @@ impl<'a> Layout<'a> {
100100
self.0.0.largest_niche
101101
}
102102

103-
pub fn align(self) -> AbiAndPrefAlign {
103+
pub fn align(self) -> AbiAlign {
104104
self.0.0.align
105105
}
106106

compiler/rustc_abi/src/lib.rs

Lines changed: 53 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use std::fmt;
4343
#[cfg(feature = "nightly")]
4444
use std::iter::Step;
4545
use std::num::{NonZeroUsize, ParseIntError};
46-
use std::ops::{Add, AddAssign, Mul, RangeInclusive, Sub};
46+
use std::ops::{Add, AddAssign, Deref, Mul, RangeInclusive, Sub};
4747
use std::str::FromStr;
4848

4949
use bitflags::bitflags;
@@ -226,22 +226,22 @@ pub const MAX_SIMD_LANES: u64 = 1 << 0xF;
226226
#[derive(Debug, PartialEq, Eq)]
227227
pub struct TargetDataLayout {
228228
pub endian: Endian,
229-
pub i1_align: AbiAndPrefAlign,
230-
pub i8_align: AbiAndPrefAlign,
231-
pub i16_align: AbiAndPrefAlign,
232-
pub i32_align: AbiAndPrefAlign,
233-
pub i64_align: AbiAndPrefAlign,
234-
pub i128_align: AbiAndPrefAlign,
235-
pub f16_align: AbiAndPrefAlign,
236-
pub f32_align: AbiAndPrefAlign,
237-
pub f64_align: AbiAndPrefAlign,
238-
pub f128_align: AbiAndPrefAlign,
229+
pub i1_align: AbiAlign,
230+
pub i8_align: AbiAlign,
231+
pub i16_align: AbiAlign,
232+
pub i32_align: AbiAlign,
233+
pub i64_align: AbiAlign,
234+
pub i128_align: AbiAlign,
235+
pub f16_align: AbiAlign,
236+
pub f32_align: AbiAlign,
237+
pub f64_align: AbiAlign,
238+
pub f128_align: AbiAlign,
239239
pub pointer_size: Size,
240-
pub pointer_align: AbiAndPrefAlign,
241-
pub aggregate_align: AbiAndPrefAlign,
240+
pub pointer_align: AbiAlign,
241+
pub aggregate_align: AbiAlign,
242242

243243
/// Alignments for vector types.
244-
pub vector_align: Vec<(Size, AbiAndPrefAlign)>,
244+
pub vector_align: Vec<(Size, AbiAlign)>,
245245

246246
pub instruction_address_space: AddressSpace,
247247

@@ -257,22 +257,22 @@ impl Default for TargetDataLayout {
257257
let align = |bits| Align::from_bits(bits).unwrap();
258258
TargetDataLayout {
259259
endian: Endian::Big,
260-
i1_align: AbiAndPrefAlign::new(align(8)),
261-
i8_align: AbiAndPrefAlign::new(align(8)),
262-
i16_align: AbiAndPrefAlign::new(align(16)),
263-
i32_align: AbiAndPrefAlign::new(align(32)),
264-
i64_align: AbiAndPrefAlign { abi: align(32), pref: align(64) },
265-
i128_align: AbiAndPrefAlign { abi: align(32), pref: align(64) },
266-
f16_align: AbiAndPrefAlign::new(align(16)),
267-
f32_align: AbiAndPrefAlign::new(align(32)),
268-
f64_align: AbiAndPrefAlign::new(align(64)),
269-
f128_align: AbiAndPrefAlign::new(align(128)),
260+
i1_align: AbiAlign::new(align(8)),
261+
i8_align: AbiAlign::new(align(8)),
262+
i16_align: AbiAlign::new(align(16)),
263+
i32_align: AbiAlign::new(align(32)),
264+
i64_align: AbiAlign::new(align(32)),
265+
i128_align: AbiAlign::new(align(32)),
266+
f16_align: AbiAlign::new(align(16)),
267+
f32_align: AbiAlign::new(align(32)),
268+
f64_align: AbiAlign::new(align(64)),
269+
f128_align: AbiAlign::new(align(128)),
270270
pointer_size: Size::from_bits(64),
271-
pointer_align: AbiAndPrefAlign::new(align(64)),
272-
aggregate_align: AbiAndPrefAlign { abi: align(0), pref: align(64) },
271+
pointer_align: AbiAlign::new(align(64)),
272+
aggregate_align: AbiAlign { abi: align(8) },
273273
vector_align: vec![
274-
(Size::from_bits(64), AbiAndPrefAlign::new(align(64))),
275-
(Size::from_bits(128), AbiAndPrefAlign::new(align(128))),
274+
(Size::from_bits(64), AbiAlign::new(align(64))),
275+
(Size::from_bits(128), AbiAlign::new(align(128))),
276276
],
277277
instruction_address_space: AddressSpace::DATA,
278278
c_enum_min_size: Integer::I32,
@@ -330,8 +330,7 @@ impl TargetDataLayout {
330330
.map_err(|err| TargetDataLayoutErrors::InvalidAlignment { cause, err })
331331
};
332332
let abi = parse_bits(s[0], "alignment", cause)?;
333-
let pref = s.get(1).map_or(Ok(abi), |pref| parse_bits(pref, "alignment", cause))?;
334-
Ok(AbiAndPrefAlign { abi: align_from_bits(abi)?, pref: align_from_bits(pref)? })
333+
Ok(AbiAlign::new(align_from_bits(abi)?))
335334
};
336335

337336
let mut dl = TargetDataLayout::default();
@@ -426,7 +425,7 @@ impl TargetDataLayout {
426425

427426
/// psABI-mandated alignment for a vector type, if any
428427
#[inline]
429-
fn cabi_vector_align(&self, vec_size: Size) -> Option<AbiAndPrefAlign> {
428+
fn cabi_vector_align(&self, vec_size: Size) -> Option<AbiAlign> {
430429
self.vector_align
431430
.iter()
432431
.find(|(size, _align)| *size == vec_size)
@@ -435,8 +434,8 @@ impl TargetDataLayout {
435434

436435
/// an alignment resembling the one LLVM would pick for a vector
437436
#[inline]
438-
pub fn llvmlike_vector_align(&self, vec_size: Size) -> AbiAndPrefAlign {
439-
self.cabi_vector_align(vec_size).unwrap_or(AbiAndPrefAlign::new(
437+
pub fn llvmlike_vector_align(&self, vec_size: Size) -> AbiAlign {
438+
self.cabi_vector_align(vec_size).unwrap_or(AbiAlign::new(
440439
Align::from_bytes(vec_size.bytes().next_power_of_two()).unwrap(),
441440
))
442441
}
@@ -864,25 +863,32 @@ impl Align {
864863
/// It is of effectively no consequence for layout in structs and on the stack.
865864
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
866865
#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
867-
pub struct AbiAndPrefAlign {
866+
pub struct AbiAlign {
868867
pub abi: Align,
869-
pub pref: Align,
870868
}
871869

872-
impl AbiAndPrefAlign {
870+
impl AbiAlign {
873871
#[inline]
874-
pub fn new(align: Align) -> AbiAndPrefAlign {
875-
AbiAndPrefAlign { abi: align, pref: align }
872+
pub fn new(align: Align) -> AbiAlign {
873+
AbiAlign { abi: align }
876874
}
877875

878876
#[inline]
879-
pub fn min(self, other: AbiAndPrefAlign) -> AbiAndPrefAlign {
880-
AbiAndPrefAlign { abi: self.abi.min(other.abi), pref: self.pref.min(other.pref) }
877+
pub fn min(self, other: AbiAlign) -> AbiAlign {
878+
AbiAlign { abi: self.abi.min(other.abi) }
881879
}
882880

883881
#[inline]
884-
pub fn max(self, other: AbiAndPrefAlign) -> AbiAndPrefAlign {
885-
AbiAndPrefAlign { abi: self.abi.max(other.abi), pref: self.pref.max(other.pref) }
882+
pub fn max(self, other: AbiAlign) -> AbiAlign {
883+
AbiAlign { abi: self.abi.max(other.abi) }
884+
}
885+
}
886+
887+
impl Deref for AbiAlign {
888+
type Target = Align;
889+
890+
fn deref(&self) -> &Self::Target {
891+
&self.abi
886892
}
887893
}
888894

@@ -945,7 +951,7 @@ impl Integer {
945951
}
946952
}
947953

948-
pub fn align<C: HasDataLayout>(self, cx: &C) -> AbiAndPrefAlign {
954+
pub fn align<C: HasDataLayout>(self, cx: &C) -> AbiAlign {
949955
use Integer::*;
950956
let dl = cx.data_layout();
951957

@@ -1058,7 +1064,7 @@ impl Float {
10581064
}
10591065
}
10601066

1061-
pub fn align<C: HasDataLayout>(self, cx: &C) -> AbiAndPrefAlign {
1067+
pub fn align<C: HasDataLayout>(self, cx: &C) -> AbiAlign {
10621068
use Float::*;
10631069
let dl = cx.data_layout();
10641070

@@ -1102,7 +1108,7 @@ impl Primitive {
11021108
}
11031109
}
11041110

1105-
pub fn align<C: HasDataLayout>(self, cx: &C) -> AbiAndPrefAlign {
1111+
pub fn align<C: HasDataLayout>(self, cx: &C) -> AbiAlign {
11061112
use Primitive::*;
11071113
let dl = cx.data_layout();
11081114

@@ -1225,7 +1231,7 @@ impl Scalar {
12251231
}
12261232
}
12271233

1228-
pub fn align(self, cx: &impl HasDataLayout) -> AbiAndPrefAlign {
1234+
pub fn align(self, cx: &impl HasDataLayout) -> AbiAlign {
12291235
self.primitive().align(cx)
12301236
}
12311237

@@ -1731,7 +1737,7 @@ pub struct LayoutData<FieldIdx: Idx, VariantIdx: Idx> {
17311737
/// especially in the case of by-pointer struct returns, which allocate stack even when unused.
17321738
pub uninhabited: bool,
17331739

1734-
pub align: AbiAndPrefAlign,
1740+
pub align: AbiAlign,
17351741
pub size: Size,
17361742

17371743
/// The largest alignment explicitly requested with `repr(align)` on this type or any field.

compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -812,11 +812,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
812812
dest.write_cvalue(fx, val);
813813
}
814814

815-
sym::pref_align_of
816-
| sym::needs_drop
817-
| sym::type_id
818-
| sym::type_name
819-
| sym::variant_count => {
815+
sym::needs_drop | sym::type_id | sym::type_name | sym::variant_count => {
820816
intrinsic_args!(fx, args => (); intrinsic);
821817

822818
let const_val = fx

compiler/rustc_codegen_ssa/src/mir/intrinsic.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
150150
}
151151
value
152152
}
153-
sym::pref_align_of
154-
| sym::needs_drop
155-
| sym::type_id
156-
| sym::type_name
157-
| sym::variant_count => {
153+
sym::needs_drop | sym::type_id | sym::type_name | sym::variant_count => {
158154
let value = bx.tcx().const_eval_instance(bx.typing_env(), instance, span).unwrap();
159155
OperandRef::from_const(bx, value, result.layout.ty).immediate_or_packed_pair(bx)
160156
}

compiler/rustc_const_eval/src/interpret/intrinsics.rs

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,6 @@ pub(crate) fn eval_nullary_intrinsic<'tcx>(
5050
ensure_monomorphic_enough(tcx, tp_ty)?;
5151
ConstValue::from_bool(tp_ty.needs_drop(tcx, typing_env))
5252
}
53-
sym::pref_align_of => {
54-
// Correctly handles non-monomorphic calls, so there is no need for ensure_monomorphic_enough.
55-
let layout = tcx
56-
.layout_of(typing_env.as_query_input(tp_ty))
57-
.map_err(|e| err_inval!(Layout(*e)))?;
58-
ConstValue::from_target_usize(layout.align.pref.bytes(), &tcx)
59-
}
6053
sym::type_id => {
6154
ensure_monomorphic_enough(tcx, tp_ty)?;
6255
ConstValue::from_u128(tcx.type_id_hash(tp_ty).as_u128())
@@ -144,14 +137,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
144137
self.write_scalar(Scalar::from_target_usize(result, self), dest)?;
145138
}
146139

147-
sym::pref_align_of
148-
| sym::needs_drop
149-
| sym::type_id
150-
| sym::type_name
151-
| sym::variant_count => {
140+
sym::needs_drop | sym::type_id | sym::type_name | sym::variant_count => {
152141
let gid = GlobalId { instance, promoted: None };
153142
let ty = match intrinsic_name {
154-
sym::pref_align_of | sym::variant_count => self.tcx.types.usize,
143+
sym::variant_count => self.tcx.types.usize,
155144
sym::needs_drop => self.tcx.types.bool,
156145
sym::type_id => self.tcx.types.u128,
157146
sym::type_name => Ty::new_static_str(self.tcx.tcx),

compiler/rustc_feature/src/removed.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,8 @@ declare_features! (
207207
/// Allows exhaustive integer pattern matching with `usize::MAX`/`isize::MIN`/`isize::MAX`.
208208
(removed, precise_pointer_size_matching, "1.32.0", Some(56354),
209209
Some("removed in favor of half-open ranges")),
210+
(removed, pref_align_of, "CURRENT_RUSTC_VERSION", Some(91971),
211+
Some("removed due to marginal use and inducing compiler complications")),
210212
(removed, proc_macro_expr, "1.27.0", Some(54727),
211213
Some("subsumed by `#![feature(proc_macro_hygiene)]`")),
212214
(removed, proc_macro_gen, "1.27.0", Some(54727),

library/core/src/intrinsics/mod.rs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2651,15 +2651,6 @@ pub const fn size_of<T>() -> usize;
26512651
#[rustc_intrinsic]
26522652
pub const fn min_align_of<T>() -> usize;
26532653

2654-
/// The preferred alignment of a type.
2655-
///
2656-
/// This intrinsic does not have a stable counterpart.
2657-
/// It's "tracking issue" is [#91971](https://github.com/rust-lang/rust/issues/91971).
2658-
#[rustc_nounwind]
2659-
#[unstable(feature = "core_intrinsics", issue = "none")]
2660-
#[rustc_intrinsic]
2661-
pub const unsafe fn pref_align_of<T>() -> usize;
2662-
26632654
/// Returns the number of variants of the type `T` cast to a `usize`;
26642655
/// if `T` has no variants, returns `0`. Uninhabited variants will be counted.
26652656
///

tests/ui/abi/c-zst.aarch64-darwin.stderr

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@ error: fn_abi_of(pass_zst) = FnAbi {
55
ty: (),
66
layout: Layout {
77
size: Size(0 bytes),
8-
align: AbiAndPrefAlign {
8+
align: AbiAlign {
99
abi: $SOME_ALIGN,
10-
pref: $SOME_ALIGN,
1110
},
1211
backend_repr: Memory {
1312
sized: true,
@@ -34,9 +33,8 @@ error: fn_abi_of(pass_zst) = FnAbi {
3433
ty: (),
3534
layout: Layout {
3635
size: Size(0 bytes),
37-
align: AbiAndPrefAlign {
36+
align: AbiAlign {
3837
abi: $SOME_ALIGN,
39-
pref: $SOME_ALIGN,
4038
},
4139
backend_repr: Memory {
4240
sized: true,

0 commit comments

Comments
 (0)