Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,12 +477,14 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
Ok(Some((repr, _))) => match repr {
// Mismatched alignment (e.g. union is #[repr(packed)]): disable opt
BackendRepr::Scalar(_) | BackendRepr::ScalarPair(_, _)
if repr.scalar_align(dl).unwrap() != align =>
if repr.scalar_platform_align(dl).unwrap() != align =>
{
BackendRepr::Memory { sized: true }
}
// Vectors require at least element alignment, else disable the opt
BackendRepr::SimdVector { element, count: _ } if element.align(dl).abi > align => {
BackendRepr::SimdVector { element, count: _ }
if element.default_align(dl).abi > align =>
{
BackendRepr::Memory { sized: true }
}
// the alignment tests passed and we can use this
Expand Down Expand Up @@ -986,7 +988,8 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
// roundtripping pointers through ptrtoint/inttoptr.
(p @ Primitive::Pointer(_), i @ Primitive::Int(..))
| (i @ Primitive::Int(..), p @ Primitive::Pointer(_))
if p.size(dl) == i.size(dl) && p.align(dl) == i.align(dl) =>
if p.size(dl) == i.size(dl)
&& p.default_align(dl) == i.default_align(dl) =>
{
p
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_abi/src/layout/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
pub fn scalar<C: HasDataLayout>(cx: &C, scalar: Scalar) -> Self {
let largest_niche = Niche::from_scalar(cx, Size::ZERO, scalar);
let size = scalar.size(cx);
let align = scalar.align(cx);
let align = scalar.default_align(cx);

let range = scalar.valid_range(cx);

Expand Down Expand Up @@ -90,8 +90,8 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {

pub fn scalar_pair<C: HasDataLayout>(cx: &C, a: Scalar, b: Scalar) -> Self {
let dl = cx.data_layout();
let b_align = b.align(dl).abi;
let align = a.align(dl).abi.max(b_align).max(dl.aggregate_align);
let b_align = b.default_align(dl).abi;
let align = a.default_align(dl).abi.max(b_align).max(dl.aggregate_align);
let b_offset = a.size(dl).align_to(b_align);
let size = (b_offset + b.size(dl)).align_to(align);

Expand Down
38 changes: 30 additions & 8 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1404,7 +1404,11 @@ impl Primitive {
}
}

pub fn align<C: HasDataLayout>(self, cx: &C) -> AbiAlign {
/// The *platform-specific* ABI alignment of this primitive.
///
/// This is the type alignment for the corresponding built-in.
/// In other contexts it might have different alignment.
pub fn default_align<C: HasDataLayout>(self, cx: &C) -> AbiAlign {

@workingjubilee workingjubilee Jun 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be changing this into Align now that we're diffing every line where this is called anyways?

(feel free to answer "no", I haven't checked whether that's a reasonable, nevermind good idea. I only raise it because it should be functionally no-op)

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm inclined to leave it matching https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html#structfield.align as AbiAlign for now. Arguably the default alignment is an ABI alignment (as opposed to a preferred one or an arbitrary one) so it's not unreasonable.

And really I'm hoping to actually just delete the vast majority of these anyway by not needing to do that align_to all over the place anyway 🙂

I'll add it to the list of things to think about if I can't just delete it, though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started on the next PR to do this:

-    ScalarPair(Scalar, Scalar),
+    ScalarPair {
+        a: Scalar,
+        b: Scalar,
+        b_offset: Size,
+    },

and it looks like it removes all the default_align calls from codegen and CTFE -- it's only left in rustc_abi and the checks in compiler/rustc_ty_utils\src\layout\invariant.rs (the latter of which the MCP would probably also remove).

So looking promising.

use Primitive::*;
let dl = cx.data_layout();

Expand Down Expand Up @@ -1579,8 +1583,12 @@ impl Scalar {
}
}

pub fn align(self, cx: &impl HasDataLayout) -> AbiAlign {
self.primitive().align(cx)
/// The *platform-specific* ABI alignment of this scalar.
///
/// This is the type alignment for the corresponding built-in.
/// This is *not* necessarily the correct alignment for a type that has this `BackendRepr::Scalar`!
pub fn default_align(self, cx: &impl HasDataLayout) -> AbiAlign {
self.primitive().default_align(cx)
}

pub fn size(self, cx: &impl HasDataLayout) -> Size {
Expand Down Expand Up @@ -1792,6 +1800,14 @@ impl IntoDiagArg for NumScalableVectors {
#[cfg_attr(feature = "nightly", derive(StableHash))]
pub enum BackendRepr {
Scalar(Scalar),
/// The data contained in this type can be entirely represented by two scalars.
/// The two scalars are listed in *memory* order, so the first is at offset zero
/// and the second at a non-zero offset.
/// These need not be `FieldIdx(0)` and `FieldIdx(1)`.
///
/// As of June 2026 the offset to the second scalar is the size of the first
/// scalar rounded up to the platform alignment of the second scalar.
/// That may soon change, however; see MCP#1007.
ScalarPair(Scalar, Scalar),
SimdScalableVector {
element: Scalar,
Expand Down Expand Up @@ -1857,10 +1873,16 @@ impl BackendRepr {
/// The psABI alignment for a `Scalar` or `ScalarPair`
///
/// `None` for other variants.
pub fn scalar_align<C: HasDataLayout>(&self, cx: &C) -> Option<Align> {
///
/// It's unclear whether this is a meaningful operation, and MCP#1007 proposes changes.
/// You should generally be using the alignment of the place or the type,
/// not calculating something from the `Scalar`s.
pub fn scalar_platform_align<C: HasDataLayout>(&self, cx: &C) -> Option<Align> {
match *self {
BackendRepr::Scalar(s) => Some(s.align(cx).abi),
BackendRepr::ScalarPair(s1, s2) => Some(s1.align(cx).max(s2.align(cx)).abi),
BackendRepr::Scalar(s) => Some(s.default_align(cx).abi),
BackendRepr::ScalarPair(s1, s2) => {
Some(s1.default_align(cx).max(s2.default_align(cx)).abi)
}
// The align of a Vector can vary in surprising ways
BackendRepr::SimdVector { .. }
| BackendRepr::Memory { .. }
Expand All @@ -1877,9 +1899,9 @@ impl BackendRepr {
BackendRepr::Scalar(s) => Some(s.size(cx)),
// May have some padding between the pair.
BackendRepr::ScalarPair(s1, s2) => {
let field2_offset = s1.size(cx).align_to(s2.align(cx).abi);
let field2_offset = s1.size(cx).align_to(s2.default_align(cx).abi);
let size = (field2_offset + s2.size(cx)).align_to(
self.scalar_align(cx)
self.scalar_platform_align(cx)
// We absolutely must have an answer here or everything is FUBAR.
.unwrap(),
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/value_and_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn codegen_field<'tcx>(
}

fn scalar_pair_calculate_b_offset(tcx: TyCtxt<'_>, a_scalar: Scalar, b_scalar: Scalar) -> Offset32 {
let b_offset = a_scalar.size(&tcx).align_to(b_scalar.align(&tcx).abi);
let b_offset = a_scalar.size(&tcx).align_to(b_scalar.default_align(&tcx).abi);
Offset32::new(b_offset.bytes().try_into().unwrap())
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
},
)
} else if let abi::BackendRepr::ScalarPair(ref a, ref b) = place.layout.backend_repr {
let b_offset = a.size(self).align_to(b.align(self).abi);
let b_offset = a.size(self).align_to(b.default_align(self).abi);

let mut load = |i, scalar: &abi::Scalar, align| {
let ptr = if i == 0 {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_gcc/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,8 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
return cx.type_i1();
}

let offset = if index == 0 { Size::ZERO } else { a.size(cx).align_to(b.align(cx).abi) };
let offset =
if index == 0 { Size::ZERO } else { a.size(cx).align_to(b.default_align(cx).abi) };
self.scalar_gcc_type_at(cx, scalar, offset)
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
});
OperandValue::Immediate(llval)
} else if let abi::BackendRepr::ScalarPair(a, b) = place.layout.backend_repr {
let b_offset = a.size(self).align_to(b.align(self).abi);
let b_offset = a.size(self).align_to(b.default_align(self).abi);

let mut load = |i, scalar: abi::Scalar, layout, align, offset| {
let llptr = if i == 0 {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
b @ abi::Scalar::Initialized { .. },
) => {
let (a_size, b_size) = (a.size(bx), b.size(bx));
let b_offset = (offset + a_size).align_to(b.align(bx).abi);
let b_offset = (offset + a_size).align_to(b.default_align(bx).abi);
assert!(b_offset.bytes() > 0);
let a_val = read_scalar(
offset,
Expand Down Expand Up @@ -388,7 +388,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
assert_eq!(field.size, a.size(bx.cx()));
(Some(a), a_llval)
} else {
assert_eq!(offset, a.size(bx.cx()).align_to(b.align(bx.cx()).abi));
assert_eq!(offset, a.size(bx.cx()).align_to(b.default_align(bx.cx()).abi));
assert_eq!(field.size, b.size(bx.cx()));
(Some(b), b_llval)
}
Expand Down Expand Up @@ -962,7 +962,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
let BackendRepr::ScalarPair(a_scalar, b_scalar) = dest.layout.backend_repr else {
bug!("store_with_flags: invalid ScalarPair layout: {:#?}", dest.layout);
};
let b_offset = a_scalar.size(bx).align_to(b_scalar.align(bx).abi);
let b_offset = a_scalar.size(bx).align_to(b_scalar.default_align(bx).abi);

let val = bx.from_immediate(a);
let align = dest.val.align;
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ impl<'tcx, Prov: Provenance> ImmTy<'tcx, Prov> {
Immediate::from(if offset.bytes() == 0 {
a_val
} else {
assert_eq!(offset, a.size(cx).align_to(b.align(cx).abi));
assert_eq!(offset, a.size(cx).align_to(b.default_align(cx).abi));
b_val
})
}
Expand Down Expand Up @@ -614,7 +614,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// We would anyway check against `ptr_align.restrict_for_offset(b_offset)`,
// which `ptr.offset(b_offset)` cannot possibly fail to satisfy.
let (a_size, b_size) = (a.size(self), b.size(self));
let b_offset = a_size.align_to(b.align(self).abi);
let b_offset = a_size.align_to(b.default_align(self).abi);
assert!(b_offset.bytes() > 0); // in `operand_field` we use the offset to tell apart the fields
let a_val = alloc.read_scalar(
alloc_range(Size::ZERO, a_size),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ where
)
};
let a_size = a_val.size();
let b_offset = a_size.align_to(b.align(&tcx).abi);
let b_offset = a_size.align_to(b.default_align(&tcx).abi);
assert!(b_offset.bytes() > 0); // in `operand_field` we use the offset to tell apart the fields

// It is tempting to verify `b_offset` against `layout.fields.offset(1)`,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
a1.size(&self.ecx) == a2.size(&self.ecx)
&& b1.size(&self.ecx) == b2.size(&self.ecx)
// The alignment of the second component determines its offset, so that also needs to match.
&& b1.align(&self.ecx) == b2.align(&self.ecx)
&& b1.default_align(&self.ecx) == b2.default_align(&self.ecx)
// None of the inputs may be a pointer.
&& !matches!(a1.primitive(), Primitive::Pointer(..))
&& !matches!(b1.primitive(), Primitive::Pointer(..))
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/callconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
BackendRepr::Scalar(scalar) => PassMode::Direct(scalar_attrs(scalar, Size::ZERO)),
BackendRepr::ScalarPair(a, b) => PassMode::Pair(
scalar_attrs(a, Size::ZERO),
scalar_attrs(b, a.size(cx).align_to(b.align(cx).abi)),
scalar_attrs(b, a.size(cx).align_to(b.default_align(cx).abi)),
),
BackendRepr::SimdVector { .. } => PassMode::Direct(ArgAttributes::new()),
BackendRepr::Memory { .. } => Self::indirect_pass_mode(&layout),
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_ty_utils/src/layout/invariant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub(super) fn layout_sanity_check<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayou

fn check_layout_abi<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayout<'tcx>) {
// Verify the ABI-mandated alignment and size for scalars.
let align = layout.backend_repr.scalar_align(cx);
let align = layout.backend_repr.scalar_platform_align(cx);
let size = layout.backend_repr.scalar_size(cx);
if let Some(align) = align {
assert_eq!(
Expand Down Expand Up @@ -208,9 +208,9 @@ pub(super) fn layout_sanity_check<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayou
};
// The fields should be at the right offset, and match the `scalar` layout.
let size1 = scalar1.size(cx);
let align1 = scalar1.align(cx).abi;
let align1 = scalar1.default_align(cx).abi;
let size2 = scalar2.size(cx);
let align2 = scalar2.align(cx).abi;
let align2 = scalar2.default_align(cx).abi;
assert_eq!(
offset1,
Size::ZERO,
Expand Down Expand Up @@ -251,7 +251,7 @@ pub(super) fn layout_sanity_check<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayou
BackendRepr::SimdVector { element, count } => {
let align = layout.align.abi;
let size = layout.size;
let element_align = element.align(cx).abi;
let element_align = element.default_align(cx).abi;
let element_size = element.size(cx);
// Currently, vectors must always be aligned to at least their elements:
assert!(align >= element_align);
Expand Down Expand Up @@ -321,7 +321,7 @@ pub(super) fn layout_sanity_check<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayou
}
// The top-level ABI and the ABI of the variants should be coherent.
let scalar_coherent = |s1: Scalar, s2: Scalar| {
s1.size(cx) == s2.size(cx) && s1.align(cx) == s2.align(cx)
s1.size(cx) == s2.size(cx) && s1.default_align(cx) == s2.default_align(cx)
};
let abi_coherent = match (layout.backend_repr, variant.backend_repr) {
(BackendRepr::Scalar(s1), BackendRepr::Scalar(s2)) => scalar_coherent(s1, s2),
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/native_lib/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
imm.layout
)
};
a.size(this).align_to(b.align(this).abi).bytes_usize()
a.size(this).align_to(b.default_align(this).abi).bytes_usize()
};

write_scalar(this, sc_first, 0)?;
Expand Down
Loading