bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Auto merge of #122050 - erikdesjardins:sret, r=nikic

Browse source 
Stop using LLVM struct types for byval/sret

For `byval` and `sret`, the type has no semantic meaning, only the size matters\*†. Using `[N x i8]` is a more direct way to specify that we want `N` bytes, and avoids relying on LLVM's struct layout.

\*: The alignment would matter, if we didn't explicitly specify it. From what I can tell, we always specified the alignment for `sret`; for `byval`, we didn't until #112157.

†: For `byval`, the hidden copy may be impacted by padding in the LLVM struct type, i.e. padding bytes may not be copied. (I'm not sure if this is done today, but I think it would be legal.) But we manually pad our LLVM struct types specifically to avoid there ever being LLVM-visible padding, so that shouldn't be an issue.

Split out from #121577.

r? `@nikic`
This commit is contained in:
bors 2024-03-11 04:45:27 +00:00
commit a6d93acf5f
17 changed files with 327 additions and 130 deletions
Download patch file Download diff file Expand all files Collapse all files

20
compiler/rustc_codegen_llvm/src/abi.rs
View file

@ -424,7 +424,10 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Indirect { attrs, meta_attrs: _, on_stack } => {
assert!(!on_stack);
let i = apply(attrs);
let sret = llvm::CreateStructRetAttr(cx.llcx, self.ret.layout.llvm_type(cx));
let sret = llvm::CreateStructRetAttr(
cx.llcx,
cx.type_array(cx.type_i8(), self.ret.layout.size.bytes()),
);
attributes::apply_to_llfn(llfn, llvm::AttributePlace::Argument(i), &[sret]);
}
PassMode::Cast { cast, pad_i32: _ } => {
@ -437,7 +440,10 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Ignore => {}
PassMode::Indirect { attrs, meta_attrs: None, on_stack: true } => {
let i = apply(attrs);
let byval = llvm::CreateByValAttr(cx.llcx, arg.layout.llvm_type(cx));
let byval = llvm::CreateByValAttr(
cx.llcx,
cx.type_array(cx.type_i8(), arg.layout.size.bytes()),
);
attributes::apply_to_llfn(llfn, llvm::AttributePlace::Argument(i), &[byval]);
}
PassMode::Direct(attrs)
@ -486,7 +492,10 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Indirect { attrs, meta_attrs: _, on_stack } => {
assert!(!on_stack);
let i = apply(bx.cx, attrs);
let sret = llvm::CreateStructRetAttr(bx.cx.llcx, self.ret.layout.llvm_type(bx));
let sret = llvm::CreateStructRetAttr(
bx.cx.llcx,
bx.cx.type_array(bx.cx.type_i8(), self.ret.layout.size.bytes()),
);
attributes::apply_to_callsite(callsite, llvm::AttributePlace::Argument(i), &[sret]);
}
PassMode::Cast { cast, pad_i32: _ } => {
@ -513,7 +522,10 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Ignore => {}
PassMode::Indirect { attrs, meta_attrs: None, on_stack: true } => {
let i = apply(bx.cx, attrs);
let byval = llvm::CreateByValAttr(bx.cx.llcx, arg.layout.llvm_type(bx));
let byval = llvm::CreateByValAttr(
bx.cx.llcx,
bx.cx.type_array(bx.cx.type_i8(), arg.layout.size.bytes()),
);
attributes::apply_to_callsite(
callsite,
llvm::AttributePlace::Argument(i),

37
compiler/rustc_target/src/abi/call/mod.rs
View file

@ -46,16 +46,24 @@ pub enum PassMode {
///
/// The argument has a layout abi of `ScalarPair`.
Pair(ArgAttributes, ArgAttributes),
/// Pass the argument after casting it. See the `CastTarget` docs for details. The bool
/// indicates if a `Reg::i32()` dummy argument is emitted before the real argument.
/// Pass the argument after casting it. See the `CastTarget` docs for details.
///
/// `pad_i32` indicates if a `Reg::i32()` dummy argument is emitted before the real argument.
Cast { pad_i32: bool, cast: Box<CastTarget> },
/// Pass the argument indirectly via a hidden pointer.
///
/// The `meta_attrs` value, if any, is for the metadata (vtable or length) of an unsized
/// argument. (This is the only mode that supports unsized arguments.)
///
/// `on_stack` defines that the value should be passed at a fixed stack offset in accordance to
/// the ABI rather than passed using a pointer. This corresponds to the `byval` LLVM argument
/// attribute (using the Rust type of this argument). `on_stack` cannot be true for unsized
/// arguments, i.e., when `meta_attrs` is `Some`.
/// attribute. The `byval` argument will use a byte array with the same size as the Rust type
/// (which ensures that padding is preserved and that we do not rely on LLVM's struct layout),
/// and will use the alignment specified in `attrs.pointee_align` (if `Some`) or the type's
/// alignment (if `None`). This means that the alignment will not always
/// match the Rust type's alignment; see documentation of `make_indirect_byval` for more info.
///
/// `on_stack` cannot be true for unsized arguments, i.e., when `meta_attrs` is `Some`.
Indirect { attrs: ArgAttributes, meta_attrs: Option<ArgAttributes>, on_stack: bool },
}
@ -596,6 +604,8 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
}
}
/// Pass this argument indirectly, by passing a (thin or fat) pointer to the argument instead.
/// This is valid for both sized and unsized arguments.
pub fn make_indirect(&mut self) {
match self.mode {
PassMode::Direct(_) | PassMode::Pair(_, _) => {
@ -609,7 +619,26 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
}
}
/// Pass this argument indirectly, by placing it at a fixed stack offset.
/// This corresponds to the `byval` LLVM argument attribute.
/// This is only valid for sized arguments.
///
/// `byval_align` specifies the alignment of the `byval` stack slot, which does not need to
/// correspond to the type's alignment. This will be `Some` if the target's ABI specifies that
/// stack slots used for arguments passed by-value have specific alignment requirements which
/// differ from the alignment used in other situations.
///
/// If `None`, the type's alignment is used.
///
/// If the resulting alignment differs from the type's alignment,
/// the argument will be copied to an alloca with sufficient alignment,
/// either in the caller (if the type's alignment is lower than the byval alignment)
/// or in the callee† (if the type's alignment is higher than the byval alignment),
/// to ensure that Rust code never sees an underaligned pointer.
///
/// † This is currently broken, see <https://github.com/rust-lang/rust/pull/122212>.
pub fn make_indirect_byval(&mut self, byval_align: Option<Align>) {
assert!(!self.layout.is_unsized(), "used byval ABI for unsized layout");
self.make_indirect();
match self.mode {
PassMode::Indirect { ref mut attrs, meta_attrs: _, ref mut on_stack } => {

2
compiler/rustc_target/src/abi/call/x86_64.rs
View file

@ -217,6 +217,8 @@ where
match cls_or_mem {
Err(Memory) => {
if is_arg {
// The x86_64 ABI doesn't have any special requirements for `byval` alignment,
// the type's alignment is always used.
arg.make_indirect_byval(None);
} else {
// `sret` parameter thus one less integer register available