Skip to content

Don't require extern "llvm-intrinsic" return structs to be #[repr(packed)] #164878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
beetrees wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Don't require extern "llvm-intrinsic" return structs to be #[repr(packed)] #164878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 29 additions & 2 deletions compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use rustc_codegen_ssa::{MemFlags, RetagInfo};
use rustc_data_structures::fx::FxHashSet;
#[cfg(feature = "master")]
use rustc_middle::ty::layout::FnAbiOf;
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::layout::{HasTypingEnv, LayoutOf};
use rustc_middle::ty::{self, Instance, Ty};
use rustc_session::config::OptLevel;
use rustc_span::{Span, Symbol, bug, span_bug, sym};
Expand Down Expand Up @@ -556,6 +556,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc
&mut self,
instance: ty::Instance<'tcx>,
args: &[OperandRef<'tcx, Self::Value>],
memory_return_place: Option<PlaceValue<Self::Value>>,
is_cleanup: bool,
) -> Self::Value {
let func = if let Some(&func) = self.intrinsic_instances.borrow().get(&instance) {
Expand Down Expand Up @@ -602,7 +603,33 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc
self.apply_attrs_to_cleanup_callsite(llret);
}

llret
let fn_sig = match *instance.ty(self.tcx, self.typing_env()).kind() {
ty::FnDef(def_id, args) => self.tcx.instantiate_bound_regions_with_erased(
self.tcx
.fn_sig(def_id)
.instantiate(self.tcx, args.no_bound_vars().unwrap())
.skip_norm_wip(),
),
_ => unreachable!(),
};
let ret_layout = self.layout_of(fn_sig.output());

if !ret_layout.is_zst() && ret_layout.is_aggregate() {
let fields =
rustc_codegen_ssa::mir::intrinsic_return_struct_non_zst_fields(self.cx, ret_layout)
.into_iter()
.enumerate()
.map(|(i, (_, offset))| (offset, self.extract_value(llret, i as u64)))
.collect();
rustc_codegen_ssa::mir::store_intrinsic_return_struct(
self,
ret_layout,
memory_return_place,
fields,
)
} else {
llret
}
}

fn abort_immediate(&mut self) {
Expand Down
85 changes: 44 additions & 41 deletions compiler/rustc_codegen_llvm/src/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,7 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
&mut self,
instance: ty::Instance<'tcx>,
args: &[OperandRef<'tcx, Self::Value>],
memory_return_place: Option<PlaceValue<Self::Value>>,
_is_cleanup: bool,
) -> Self::Value {
let tcx = self.tcx();
Expand All @@ -911,10 +912,15 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
assert!(!fn_sig.c_variadic());

let ret_layout = self.layout_of(fn_sig.output());
let llreturn_ty = if ret_layout.is_zst() {
self.type_void()
let (llreturn_ty, return_struct_fields) = if ret_layout.is_zst() {
(self.type_void(), vec![])
} else if ret_layout.is_aggregate() {
let fields =
rustc_codegen_ssa::mir::intrinsic_return_struct_non_zst_fields(self.cx, ret_layout);
let llvm_fields = fields.iter().copied().map(|(t, _)| t).collect::<Vec<_>>();
(self.type_struct(&llvm_fields, false), fields)
} else {
ret_layout.immediate_llvm_type(self)
(ret_layout.immediate_llvm_type(self), vec![])
};

let mut llargument_tys = Vec::with_capacity(fn_sig.inputs().len());
Expand Down Expand Up @@ -970,14 +976,33 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
)
};

let src_ty = self.val_ty(llret);
let dest_ty = llreturn_ty;
assert!(
can_autocast(self, dest_ty, src_ty),
"Cannot match `{src_ty:?}` (expected) with `{dest_ty:?}` (found) in `{fn_ptr:?}`"
);
if return_struct_fields.is_empty() {
let src_ty = self.val_ty(llret);
let dest_ty = llreturn_ty;
assert!(
can_autocast(self, dest_ty, src_ty),
"Cannot match `{src_ty:?}` (expected) with `{dest_ty:?}` (found) in `{fn_ptr:?}`"
);

autocast(self, llret, src_ty, dest_ty)
autocast(self, llret, src_ty, dest_ty)
} else {
let fields = return_struct_fields.into_iter().enumerate().map(|(i, (dest_ty, offset))| {
let value = self.extract_value(llret, i as u64);
let src_ty = self.val_ty(value);
assert!(
can_autocast(self, dest_ty, src_ty),
"Cannot match `{src_ty:?}` (expected) with `{dest_ty:?}` (found) in `{fn_ptr:?}`"
);
let value = autocast(self, value, src_ty, dest_ty);
(offset, value)
}).collect();
rustc_codegen_ssa::mir::store_intrinsic_return_struct(
self,
ret_layout,
memory_return_place,
fields,
)
}
}

fn abort_immediate(&mut self) {
Expand Down Expand Up @@ -1063,23 +1088,6 @@ fn can_autocast<'ll>(cx: &CodegenCx<'ll, '_>, rust_ty: &'ll Type, llvm_ty: &'ll
}

match cx.type_kind(llvm_ty) {
// Some LLVM intrinsics return **non-packed** structs, but they can't be mimicked from Rust
// due to auto field-alignment in non-packed structs (packed structs are represented in LLVM
// as, well, packed structs, so they won't match with those either)
TypeKind::Struct if cx.type_kind(rust_ty) == TypeKind::Struct => {
let rust_element_tys = cx.struct_element_types(rust_ty);
let llvm_element_tys = cx.struct_element_types(llvm_ty);

if rust_element_tys.len() != llvm_element_tys.len() {
return false;
}

iter::zip(rust_element_tys, llvm_element_tys).all(
|(rust_element_ty, llvm_element_ty)| {
can_autocast(cx, rust_element_ty, llvm_element_ty)
},
)
}
_ if let Some((llvm_element_ty, element_count)) =
cx.vector_element_type_and_length(llvm_ty) =>
{
Expand Down Expand Up @@ -1123,19 +1131,6 @@ fn autocast<'ll>(
return val;
}
match (bx.type_kind(src_ty), bx.type_kind(dest_ty)) {
// re-pack structs
(TypeKind::Struct, TypeKind::Struct) => {
let mut ret = bx.const_poison(dest_ty);
for (idx, (src_element_ty, dest_element_ty)) in
iter::zip(bx.struct_element_types(src_ty), bx.struct_element_types(dest_ty))
.enumerate()
{
let elt = bx.extract_value(val, idx as u64);
let casted_elt = autocast(bx, elt, src_element_ty, dest_element_ty);
ret = bx.insert_value(ret, casted_elt, idx as u64);
}
ret
}
// cast from the i1xN vector type to the primitive type
(TypeKind::Vector, TypeKind::Integer)
if let Some((elem, vector_length)) = bx.vector_element_type_and_length(src_ty)
Expand Down Expand Up @@ -1233,9 +1228,17 @@ fn intrinsic_fn<'ll, 'tcx>(
let llvm_argument_tys = bx.func_params_types(llvm_fn_ty);
let llvm_is_variadic = bx.func_is_variadic(llvm_fn_ty);

let (rust_return_tys, llvm_return_tys) = if bx.type_kind(rust_return_ty) == TypeKind::Struct
&& bx.type_kind(llvm_return_ty) == TypeKind::Struct
{
(bx.struct_element_types(rust_return_ty), bx.struct_element_types(llvm_return_ty))
} else {
(vec![rust_return_ty], vec![llvm_return_ty])
};
let is_correct_signature = !llvm_is_variadic
&& rust_argument_tys.len() == llvm_argument_tys.len()
&& iter::once((rust_return_ty, llvm_return_ty))
&& rust_return_tys.len() == llvm_return_tys.len()
&& iter::zip(rust_return_tys, llvm_return_tys)
.chain(iter::zip(rust_argument_tys, llvm_argument_tys))
.all(|(rust_ty, llvm_ty)| can_autocast(bx, rust_ty, llvm_ty));

Expand Down
139 changes: 108 additions & 31 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use std::cmp;
use std::ops::Range;

use rustc_abi::{
Align, ArmCall, CanonAbi, ExternAbi, FieldsShape, HasDataLayout, Size, VariantIdx, Variants,
Align, ArmCall, BackendRepr, CanonAbi, ExternAbi, FieldsShape, HasDataLayout, Size, VariantIdx,
Variants,
};
use rustc_ast as ast;
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
Expand Down Expand Up @@ -1166,38 +1167,42 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let result_layout =
self.cx.layout_of(self.monomorphized_place_ty(destination.as_ref()));

let return_dest = if result_layout.is_zst() {
ReturnDest::Nothing
} else if let Some(index) = destination.as_local() {
match self.locals[index] {
LocalRef::Place(dest) => ReturnDest::Store(dest),
LocalRef::UnsizedPlace(_) => bug!("return type must be sized"),
LocalRef::PendingOperand => {
// Handle temporary places, specifically `Operand` ones, as
// they don't have `alloca`s.
ReturnDest::DirectOperand(index)
}
LocalRef::Operand(_) => bug!("place local already assigned to"),
let return_mode = if result_layout.is_zst() {
PassMode::Ignore
} else if let BackendRepr::ScalarPair { .. } = result_layout.backend_repr {
PassMode::Pair(ArgAttributes::new(), ArgAttributes::new())
} else if result_layout.is_aggregate() {
PassMode::Indirect {
attrs: ArgAttributes::new(),
address_space: None,
mode: IndirectMode::Pointer,
}
} else {
ReturnDest::Store(self.codegen_place(bx, destination.as_ref()))
PassMode::Direct(ArgAttributes::new())
};
let return_abi = ArgAbi { layout: result_layout, mode: return_mode };
let (return_dest, memory_return_place) =
self.make_return_place(bx, destination, &return_abi);
assert_eq!(
memory_return_place.is_some(),
!result_layout.is_zst()
&& matches!(result_layout.backend_repr, BackendRepr::Memory { sized: true })
);

let args =
args.into_iter().map(|arg| self.codegen_operand(bx, &arg.node)).collect::<Vec<_>>();

self.set_debug_loc(bx, source_info);

let llret =
bx.codegen_llvm_intrinsic_call(instance, &args, self.mir[helper.bb].is_cleanup);
let llret = bx.codegen_llvm_intrinsic_call(
instance,
&args,
memory_return_place,
self.mir[helper.bb].is_cleanup,
);

if let Some(target) = target {
self.store_return(
bx,
return_dest,
&ArgAbi { layout: result_layout, mode: PassMode::Direct(ArgAttributes::new()) },
llret,
);
self.store_return(bx, return_dest, &return_abi, llret);
return helper.funclet_br(self, bx, target, mergeable_succ, &[]);
} else {
bx.unreachable();
Expand Down Expand Up @@ -2424,9 +2429,25 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
dest: mir::Place<'tcx>,
fn_ret: &ArgAbi<'tcx, Ty<'tcx>>,
) -> (ReturnDest<'tcx, Bx::Value>, ReturnSlot<Bx::Value>) {
let (dest, place) = self.make_return_place(bx, dest, fn_ret);
(
dest,
match place {
None => ReturnSlot::Direct,
Some(place) => ReturnSlot::Indirect(place.llval),
},
)
}

fn make_return_place(
&mut self,
bx: &mut Bx,
dest: mir::Place<'tcx>,
fn_ret: &ArgAbi<'tcx, Ty<'tcx>>,
) -> (ReturnDest<'tcx, Bx::Value>, Option<PlaceValue<Bx::Value>>) {
// If the return is ignored, we can just return a do-nothing `ReturnDest`.
if fn_ret.is_ignore() {
return (ReturnDest::Nothing, ReturnSlot::Direct);
return (ReturnDest::Nothing, None);
}
let dest = if let Some(index) = dest.as_local() {
match self.locals[index] {
Expand All @@ -2440,12 +2461,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// but the calling convention has an indirect return.
let tmp = PlaceRef::alloca(bx, fn_ret.layout);
tmp.storage_live(bx);
(
ReturnDest::IndirectOperand(tmp, index),
ReturnSlot::Indirect(tmp.val.llval),
)
(ReturnDest::IndirectOperand(tmp, index), Some(tmp.val))
} else {
(ReturnDest::DirectOperand(index), ReturnSlot::Direct)
(ReturnDest::DirectOperand(index), None)
};
}
LocalRef::Operand(_) => {
Expand All @@ -2465,13 +2483,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// to create a temporary.
span_bug!(self.mir.span, "can't directly store to unaligned value");
}
(ReturnDest::Nothing, ReturnSlot::Indirect(dest.val.llval))
(ReturnDest::Nothing, Some(dest.val))
} else {
(ReturnDest::Store(dest), ReturnSlot::Direct)
(ReturnDest::Store(dest), None)
}
}

// Stores the return value of a function call into it's final location.
/// Stores the return value of a function call into it's final location.
fn store_return(
&mut self,
bx: &mut Bx,
Expand Down Expand Up @@ -2573,3 +2591,62 @@ pub fn store_cast<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx.store(value, ptr, align);
};
}

pub fn intrinsic_return_struct_non_zst_fields<'tcx, Cx: CodegenMethods<'tcx>>(
cx: &Cx,
ret_layout: TyAndLayout<'tcx>,
) -> Vec<(Cx::Type, Size)> {
let ret_layout = ret_layout.peel_transparent_wrappers_from_non_1zst(cx);
match ret_layout.ty.kind() {
ty::Adt(adt_def, _) if adt_def.is_struct() => {}
ty::Tuple(_) => {}
_ => bug!("invalid intrinsic return type: {ret_layout:?}"),
}
let fields = (0..ret_layout.fields.count())
.filter_map(|i| {
let field = ret_layout.field(cx, i);
assert!(
field.is_zst() || !field.is_aggregate(),
"invalid intrinsic return field: {field:?}"
);
(!field.is_zst())
.then(|| (cx.immediate_backend_type(field), ret_layout.fields.offset(i)))
})
.collect::<Vec<_>>();
assert!(fields.len() >= 2, "invalid intrinsic return type: {ret_layout:?}");
fields
}

pub fn store_intrinsic_return_struct<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
ret_layout: TyAndLayout<'tcx>,
memory_return_place: Option<PlaceValue<Bx::Value>>,
fields: Vec<(Size, Bx::Value)>,
) -> Bx::Value {
match ret_layout.backend_repr {
BackendRepr::ScalarPair { a: _, b: _, b_offset } => {
assert!(memory_return_place.is_none());
let a_index = if fields[0].0 == b_offset { 1 } else { 0 };
let b_index = if fields[1].0 == b_offset { 1 } else { 0 };
assert_ne!(a_index, b_index);
let mut pair = bx.cx().const_poison(bx.immediate_backend_type(ret_layout));
pair = bx.insert_value(pair, fields[a_index].1, 0);
bx.insert_value(pair, fields[b_index].1, 1)
}
BackendRepr::Memory { sized: true } => {
let dest =
memory_return_place.expect("BackendRepr::Memory return with no return place");
for (offset, value) in fields.into_iter() {
let pointer = if offset.bytes() != 0 {
bx.inbounds_ptradd(dest.llval, bx.const_usize(offset.bytes()))
} else {
dest.llval
};
bx.store(value, pointer, dest.align.restrict_for_offset(offset));
}
// Return value is ignored as result was stored in `memory_return_place`
dest.llval
}
_ => bug!("unexpected aggregate return layout: {ret_layout:?}"),
}
}
4 changes: 3 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ mod retag;
mod rvalue;
mod statement;

pub use self::block::store_cast;
pub use self::block::{
intrinsic_return_struct_non_zst_fields, store_cast, store_intrinsic_return_struct,
};
use self::debuginfo::{FunctionDebugContext, PerLocalVarDebugInfo};
use self::operand::{OperandRef, OperandValue};
use self::place::PlaceRef;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/traits/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub trait IntrinsicCallBuilderMethods<'tcx>: BackendTypes {
&mut self,
instance: ty::Instance<'tcx>,
args: &[OperandRef<'tcx, Self::Value>],
memory_return_place: Option<PlaceValue<Self::Value>>,
is_cleanup: bool,
) -> Self::Value;

Expand Down
Loading
Loading