Skip to content

Commit

Permalink
Rollup merge of rust-lang#136985 - zachs18:backend-repr-remove-uninha…
Browse files Browse the repository at this point in the history
…bited, r=workingjubilee

Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited)

Accepted MCP: rust-lang/compiler-team#832

Fixes rust-lang#135802

Do not consider the inhabitedness of a type for function call ABI purposes.

* Remove the [`rustc_abi::BackendRepr::Uninhabited`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.BackendRepr.html) variant
  * Instead calculate the `BackendRepr` of uninhabited types "normally" (as though they were not uninhabited "at the top level", but still considering inhabitedness of variants to determine enum layout, etc)
* Add an `uninhabited: bool` field to [`rustc_abi::LayoutData`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/struct.LayoutData.html) so inhabitedness of a `LayoutData` can still be queried when necessary (e.g. when determining if an enum variant needs a tag value allocated to it).

This should not affect type layouts (size/align/field offset); this should only affect function call ABI, and only of uninhabited types.

cc ``@RalfJung``
  • Loading branch information
matthiaskrgr authored Feb 18, 2025
2 parents 3fc0115 + 8382b99 commit 5b12391
Show file tree
Hide file tree
Showing 49 changed files with 530 additions and 168 deletions.
2 changes: 0 additions & 2 deletions compiler/rustc_abi/src/callconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
Ty: TyAbiInterface<'a, C> + Copy,
{
match self.backend_repr {
BackendRepr::Uninhabited => Err(Heterogeneous),

// The primitive for this algorithm.
BackendRepr::Scalar(scalar) => {
let kind = match scalar.primitive() {
Expand Down
23 changes: 12 additions & 11 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
},
backend_repr: BackendRepr::ScalarPair(a, b),
largest_niche,
uninhabited: false,
align,
size,
max_repr_align: None,
Expand Down Expand Up @@ -221,8 +222,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
LayoutData {
variants: Variants::Empty,
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Uninhabited,
backend_repr: BackendRepr::Memory { sized: true },
largest_niche: None,
uninhabited: true,
align: dl.i8_align,
size: Size::ZERO,
max_repr_align: None,
Expand Down Expand Up @@ -400,6 +402,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
fields: FieldsShape::Union(union_field_count),
backend_repr: abi,
largest_niche: None,
uninhabited: false,
align,
size: size.align_to(align.abi),
max_repr_align,
Expand Down Expand Up @@ -447,7 +450,6 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
Scalar::Union { .. } => {}
};
match &mut st.backend_repr {
BackendRepr::Uninhabited => {}
BackendRepr::Scalar(scalar) => hide_niches(scalar),
BackendRepr::ScalarPair(a, b) => {
hide_niches(a);
Expand Down Expand Up @@ -639,9 +641,8 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
let same_size = size == variant_layouts[largest_variant_index].size;
let same_align = align == variant_layouts[largest_variant_index].align;

let abi = if variant_layouts.iter().all(|v| v.is_uninhabited()) {
BackendRepr::Uninhabited
} else if same_size && same_align && others_zst {
let uninhabited = variant_layouts.iter().all(|v| v.is_uninhabited());
let abi = if same_size && same_align && others_zst {
match variant_layouts[largest_variant_index].backend_repr {
// When the total alignment and size match, we can use the
// same ABI as the scalar variant with the reserved niche.
Expand Down Expand Up @@ -683,6 +684,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
},
backend_repr: abi,
largest_niche,
uninhabited,
size,
align,
max_repr_align,
Expand Down Expand Up @@ -853,9 +855,8 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
};
let mut abi = BackendRepr::Memory { sized: true };

if layout_variants.iter().all(|v| v.is_uninhabited()) {
abi = BackendRepr::Uninhabited;
} else if tag.size(dl) == size {
let uninhabited = layout_variants.iter().all(|v| v.is_uninhabited());
if tag.size(dl) == size {
// Make sure we only use scalar layout when the enum is entirely its
// own tag (i.e. it has no padding nor any non-ZST variant fields).
abi = BackendRepr::Scalar(tag);
Expand Down Expand Up @@ -995,6 +996,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
memory_index: [0].into(),
},
largest_niche,
uninhabited,
backend_repr: abi,
align,
size,
Expand Down Expand Up @@ -1355,9 +1357,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
_ => {}
}
}
if fields.iter().any(|f| f.is_uninhabited()) {
abi = BackendRepr::Uninhabited;
}
let uninhabited = fields.iter().any(|f| f.is_uninhabited());

let unadjusted_abi_align = if repr.transparent() {
match layout_of_single_non_zst_field {
Expand All @@ -1378,6 +1378,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
fields: FieldsShape::Arbitrary { offsets, memory_index },
backend_repr: abi,
largest_niche,
uninhabited,
align,
size,
max_repr_align,
Expand Down
35 changes: 16 additions & 19 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1404,7 +1404,6 @@ impl AddressSpace {
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
pub enum BackendRepr {
Uninhabited,
Scalar(Scalar),
ScalarPair(Scalar, Scalar),
Vector {
Expand All @@ -1423,10 +1422,9 @@ impl BackendRepr {
#[inline]
pub fn is_unsized(&self) -> bool {
match *self {
BackendRepr::Uninhabited
| BackendRepr::Scalar(_)
| BackendRepr::ScalarPair(..)
| BackendRepr::Vector { .. } => false,
BackendRepr::Scalar(_) | BackendRepr::ScalarPair(..) | BackendRepr::Vector { .. } => {
false
}
BackendRepr::Memory { sized } => !sized,
}
}
Expand All @@ -1445,12 +1443,6 @@ impl BackendRepr {
}
}

/// Returns `true` if this is an uninhabited type
#[inline]
pub fn is_uninhabited(&self) -> bool {
matches!(*self, BackendRepr::Uninhabited)
}

/// Returns `true` if this is a scalar type
#[inline]
pub fn is_scalar(&self) -> bool {
Expand All @@ -1471,7 +1463,7 @@ impl BackendRepr {
BackendRepr::Vector { element, count } => {
cx.data_layout().vector_align(element.size(cx) * count)
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } => return None,
BackendRepr::Memory { .. } => return None,
})
}

Expand All @@ -1492,7 +1484,7 @@ impl BackendRepr {
// to make the size a multiple of align (e.g. for vectors of size 3).
(element.size(cx) * count).align_to(self.inherent_align(cx)?.abi)
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } => return None,
BackendRepr::Memory { .. } => return None,
})
}

Expand All @@ -1506,9 +1498,7 @@ impl BackendRepr {
BackendRepr::Vector { element, count } => {
BackendRepr::Vector { element: element.to_union(), count }
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } => {
BackendRepr::Memory { sized: true }
}
BackendRepr::Memory { .. } => BackendRepr::Memory { sized: true },
}
}

Expand Down Expand Up @@ -1704,6 +1694,11 @@ pub struct LayoutData<FieldIdx: Idx, VariantIdx: Idx> {
/// The leaf scalar with the largest number of invalid values
/// (i.e. outside of its `valid_range`), if it exists.
pub largest_niche: Option<Niche>,
/// Is this type known to be uninhabted?
///
/// This is separate from BackendRepr because uninhabited return types can affect ABI,
/// especially in the case of by-pointer struct returns, which allocate stack even when unused.
pub uninhabited: bool,

pub align: AbiAndPrefAlign,
pub size: Size,
Expand Down Expand Up @@ -1735,14 +1730,14 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
/// Returns `true` if this is an aggregate type (including a ScalarPair!)
pub fn is_aggregate(&self) -> bool {
match self.backend_repr {
BackendRepr::Uninhabited | BackendRepr::Scalar(_) | BackendRepr::Vector { .. } => false,
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } => false,
BackendRepr::ScalarPair(..) | BackendRepr::Memory { .. } => true,
}
}

/// Returns `true` if this is an uninhabited type
pub fn is_uninhabited(&self) -> bool {
self.backend_repr.is_uninhabited()
self.uninhabited
}

pub fn scalar<C: HasDataLayout>(cx: &C, scalar: Scalar) -> Self {
Expand Down Expand Up @@ -1778,6 +1773,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Scalar(scalar),
largest_niche,
uninhabited: false,
size,
align,
max_repr_align: None,
Expand All @@ -1802,6 +1798,7 @@ where
backend_repr,
fields,
largest_niche,
uninhabited,
variants,
max_repr_align,
unadjusted_abi_align,
Expand All @@ -1813,6 +1810,7 @@ where
.field("abi", backend_repr)
.field("fields", fields)
.field("largest_niche", largest_niche)
.field("uninhabited", uninhabited)
.field("variants", variants)
.field("max_repr_align", max_repr_align)
.field("unadjusted_abi_align", unadjusted_abi_align)
Expand Down Expand Up @@ -1877,7 +1875,6 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
BackendRepr::Scalar(_) | BackendRepr::ScalarPair(..) | BackendRepr::Vector { .. } => {
false
}
BackendRepr::Uninhabited => self.size.bytes() == 0,
BackendRepr::Memory { sized } => sized && self.size.bytes() == 0,
}
}
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_cranelift/src/value_and_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,9 +638,7 @@ impl<'tcx> CPlace<'tcx> {
}
CPlaceInner::Addr(_, Some(_)) => bug!("Can't write value to unsized place {:?}", self),
CPlaceInner::Addr(to_ptr, None) => {
if dst_layout.size == Size::ZERO
|| dst_layout.backend_repr == BackendRepr::Uninhabited
{
if dst_layout.size == Size::ZERO {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc
let layout = self.layout_of(tp_ty).layout;
let _use_integer_compare = match layout.backend_repr() {
Scalar(_) | ScalarPair(_, _) => true,
Uninhabited | Vector { .. } => false,
Vector { .. } => false,
Memory { .. } => {
// For rusty ABIs, small aggregates are actually passed
// as `RegKind::Integer` (see `FnAbi::adjust_for_abi`),
Expand Down
13 changes: 5 additions & 8 deletions compiler/rustc_codegen_gcc/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn uncached_gcc_type<'gcc, 'tcx>(
false,
);
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } => {}
BackendRepr::Memory { .. } => {}
}

let name = match *layout.ty.kind() {
Expand Down Expand Up @@ -179,19 +179,16 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
fn is_gcc_immediate(&self) -> bool {
match self.backend_repr {
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } => true,
BackendRepr::ScalarPair(..) | BackendRepr::Uninhabited | BackendRepr::Memory { .. } => {
false
}
BackendRepr::ScalarPair(..) | BackendRepr::Memory { .. } => false,
}
}

fn is_gcc_scalar_pair(&self) -> bool {
match self.backend_repr {
BackendRepr::ScalarPair(..) => true,
BackendRepr::Uninhabited
| BackendRepr::Scalar(_)
| BackendRepr::Vector { .. }
| BackendRepr::Memory { .. } => false,
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } | BackendRepr::Memory { .. } => {
false
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
let layout = self.layout_of(tp_ty).layout;
let use_integer_compare = match layout.backend_repr() {
Scalar(_) | ScalarPair(_, _) => true,
Uninhabited | Vector { .. } => false,
Vector { .. } => false,
Memory { .. } => {
// For rusty ABIs, small aggregates are actually passed
// as `RegKind::Integer` (see `FnAbi::adjust_for_abi`),
Expand Down
13 changes: 5 additions & 8 deletions compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn uncached_llvm_type<'a, 'tcx>(
let element = layout.scalar_llvm_type_at(cx, element);
return cx.type_vector(element, count);
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } | BackendRepr::ScalarPair(..) => {}
BackendRepr::Memory { .. } | BackendRepr::ScalarPair(..) => {}
}

let name = match layout.ty.kind() {
Expand Down Expand Up @@ -172,19 +172,16 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {
fn is_llvm_immediate(&self) -> bool {
match self.backend_repr {
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } => true,
BackendRepr::ScalarPair(..) | BackendRepr::Uninhabited | BackendRepr::Memory { .. } => {
false
}
BackendRepr::ScalarPair(..) | BackendRepr::Memory { .. } => false,
}
}

fn is_llvm_scalar_pair(&self) -> bool {
match self.backend_repr {
BackendRepr::ScalarPair(..) => true,
BackendRepr::Uninhabited
| BackendRepr::Scalar(_)
| BackendRepr::Vector { .. }
| BackendRepr::Memory { .. } => false,
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } | BackendRepr::Memory { .. } => {
false
}
}
}

Expand Down
28 changes: 7 additions & 21 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use rustc_abi::{BackendRepr, ExternAbi, HasDataLayout, Reg, WrappingRange};
use rustc_ast as ast;
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
use rustc_hir::lang_items::LangItem;
use rustc_middle::mir::{
self, AssertKind, BasicBlock, InlineAsmMacro, SwitchTargets, UnwindTerminateReason,
};
use rustc_middle::mir::{self, AssertKind, InlineAsmMacro, SwitchTargets, UnwindTerminateReason};
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, ValidityRequirement};
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::{self, Instance, Ty};
Expand Down Expand Up @@ -942,7 +940,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
&fn_abi.ret,
&mut llargs,
Some(intrinsic),
target,
);
let dest = match ret_dest {
_ if fn_abi.ret.is_indirect() => llargs[0],
Expand Down Expand Up @@ -998,19 +995,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
};

let mut llargs = Vec::with_capacity(arg_count);
let destination = target.as_ref().map(|&target| {
(
self.make_return_dest(
bx,
destination,
&fn_abi.ret,
&mut llargs,
None,
Some(target),
),
target,
)
});

// We still need to call `make_return_dest` even if there's no `target`, since
// `fn_abi.ret` could be `PassMode::Indirect`, even if it is uninhabited,
// and `make_return_dest` adds the return-place indirect pointer to `llargs`.
let return_dest = self.make_return_dest(bx, destination, &fn_abi.ret, &mut llargs, None);
let destination = target.map(|target| (return_dest, target));

// Split the rust-call tupled arguments off.
let (first_args, untuple) = if abi == ExternAbi::RustCall && !args.is_empty() {
Expand Down Expand Up @@ -1813,11 +1803,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
fn_ret: &ArgAbi<'tcx, Ty<'tcx>>,
llargs: &mut Vec<Bx::Value>,
intrinsic: Option<ty::IntrinsicDef>,
target: Option<BasicBlock>,
) -> ReturnDest<'tcx, Bx::Value> {
if target.is_none() {
return ReturnDest::Nothing;
}
// If the return is ignored, we can just return a do-nothing `ReturnDest`.
if fn_ret.is_ignore() {
return ReturnDest::Nothing;
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
bx.store(*llval, llptr, field.align.abi);
*llval = bx.load(llfield_ty, llptr, field.align.abi);
}
(
OperandValue::Immediate(_),
BackendRepr::Uninhabited | BackendRepr::Memory { sized: false },
) => {
(OperandValue::Immediate(_), BackendRepr::Memory { sized: false }) => {
bug!()
}
(OperandValue::Pair(..), _) => bug!(),
Expand Down
Loading

0 comments on commit 5b12391

Please sign in to comment.