mirror of https://github.com/rust-lang/rust.git
make PassMode::Cast consistently copy between Rust/ABI representation
Previously, we did this slightly incorrectly for return values, and didn't do it at all for arguments.
This commit is contained in:
parent
74ef47e90c
commit
8841315d3e
|
@ -211,48 +211,34 @@ impl<'ll, 'tcx> ArgAbiExt<'ll, 'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
|
||||||
bug!("unsized `ArgAbi` must be handled through `store_fn_arg`");
|
bug!("unsized `ArgAbi` must be handled through `store_fn_arg`");
|
||||||
}
|
}
|
||||||
PassMode::Cast { cast, pad_i32: _ } => {
|
PassMode::Cast { cast, pad_i32: _ } => {
|
||||||
// FIXME(eddyb): Figure out when the simpler Store is safe, clang
|
// The ABI mandates that the value is passed as a different struct representation.
|
||||||
// uses it for i16 -> {i8, i8}, but not for i24 -> {i8, i8, i8}.
|
// Spill and reload it from the stack to convert from the ABI representation to
|
||||||
let can_store_through_cast_ptr = false;
|
// the Rust representation.
|
||||||
if can_store_through_cast_ptr {
|
|
||||||
bx.store(val, dst.llval, self.layout.align.abi);
|
|
||||||
} else {
|
|
||||||
// The actual return type is a struct, but the ABI
|
|
||||||
// adaptation code has cast it into some scalar type. The
|
|
||||||
// code that follows is the only reliable way I have
|
|
||||||
// found to do a transform like i64 -> {i32,i32}.
|
|
||||||
// Basically we dump the data onto the stack then memcpy it.
|
|
||||||
//
|
|
||||||
// Other approaches I tried:
|
|
||||||
// - Casting rust ret pointer to the foreign type and using Store
|
|
||||||
// is (a) unsafe if size of foreign type > size of rust type and
|
|
||||||
// (b) runs afoul of strict aliasing rules, yielding invalid
|
|
||||||
// assembly under -O (specifically, the store gets removed).
|
|
||||||
// - Truncating foreign type to correct integral type and then
|
|
||||||
// bitcasting to the struct type yields invalid cast errors.
|
|
||||||
|
|
||||||
// We instead thus allocate some scratch space...
|
|
||||||
let scratch_size = cast.size(bx);
|
let scratch_size = cast.size(bx);
|
||||||
let scratch_align = cast.align(bx);
|
let scratch_align = cast.align(bx);
|
||||||
|
// Note that the ABI type may be either larger or smaller than the Rust type,
|
||||||
|
// due to the presence or absence of trailing padding. For example:
|
||||||
|
// - On some ABIs, the Rust layout { f64, f32, <f32 padding> } may omit padding
|
||||||
|
// when passed by value, making it smaller.
|
||||||
|
// - On some ABIs, the Rust layout { u16, u16, u16 } may be padded up to 8 bytes
|
||||||
|
// when passed by value, making it larger.
|
||||||
|
let copy_bytes = cmp::min(scratch_size.bytes(), self.layout.size.bytes());
|
||||||
|
// Allocate some scratch space...
|
||||||
let llscratch = bx.alloca(cast.llvm_type(bx), scratch_align);
|
let llscratch = bx.alloca(cast.llvm_type(bx), scratch_align);
|
||||||
bx.lifetime_start(llscratch, scratch_size);
|
bx.lifetime_start(llscratch, scratch_size);
|
||||||
|
// ...store the value...
|
||||||
// ... where we first store the value...
|
|
||||||
bx.store(val, llscratch, scratch_align);
|
bx.store(val, llscratch, scratch_align);
|
||||||
|
|
||||||
// ... and then memcpy it to the intended destination.
|
// ... and then memcpy it to the intended destination.
|
||||||
bx.memcpy(
|
bx.memcpy(
|
||||||
dst.llval,
|
dst.llval,
|
||||||
self.layout.align.abi,
|
self.layout.align.abi,
|
||||||
llscratch,
|
llscratch,
|
||||||
scratch_align,
|
scratch_align,
|
||||||
bx.const_usize(self.layout.size.bytes()),
|
bx.const_usize(copy_bytes),
|
||||||
MemFlags::empty(),
|
MemFlags::empty(),
|
||||||
);
|
);
|
||||||
|
|
||||||
bx.lifetime_end(llscratch, scratch_size);
|
bx.lifetime_end(llscratch, scratch_size);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
_ => {
|
_ => {
|
||||||
OperandRef::from_immediate_or_packed_pair(bx, val, self.layout).val.store(bx, dst);
|
OperandRef::from_immediate_or_packed_pair(bx, val, self.layout).val.store(bx, dst);
|
||||||
}
|
}
|
||||||
|
|
|
@ -1471,9 +1471,35 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
|
||||||
|
|
||||||
if by_ref && !arg.is_indirect() {
|
if by_ref && !arg.is_indirect() {
|
||||||
// Have to load the argument, maybe while casting it.
|
// Have to load the argument, maybe while casting it.
|
||||||
if let PassMode::Cast { cast: ty, .. } = &arg.mode {
|
if let PassMode::Cast { cast, pad_i32: _ } = &arg.mode {
|
||||||
let llty = bx.cast_backend_type(ty);
|
// The ABI mandates that the value is passed as a different struct representation.
|
||||||
llval = bx.load(llty, llval, align.min(arg.layout.align.abi));
|
// Spill and reload it from the stack to convert from the Rust representation to
|
||||||
|
// the ABI representation.
|
||||||
|
let scratch_size = cast.size(bx);
|
||||||
|
let scratch_align = cast.align(bx);
|
||||||
|
// Note that the ABI type may be either larger or smaller than the Rust type,
|
||||||
|
// due to the presence or absence of trailing padding. For example:
|
||||||
|
// - On some ABIs, the Rust layout { f64, f32, <f32 padding> } may omit padding
|
||||||
|
// when passed by value, making it smaller.
|
||||||
|
// - On some ABIs, the Rust layout { u16, u16, u16 } may be padded up to 8 bytes
|
||||||
|
// when passed by value, making it larger.
|
||||||
|
let copy_bytes = cmp::min(scratch_size.bytes(), arg.layout.size.bytes());
|
||||||
|
// Allocate some scratch space...
|
||||||
|
let llscratch = bx.alloca(bx.cast_backend_type(cast), scratch_align);
|
||||||
|
bx.lifetime_start(llscratch, scratch_size);
|
||||||
|
// ...memcpy the value...
|
||||||
|
bx.memcpy(
|
||||||
|
llscratch,
|
||||||
|
scratch_align,
|
||||||
|
llval,
|
||||||
|
align,
|
||||||
|
bx.const_usize(copy_bytes),
|
||||||
|
MemFlags::empty(),
|
||||||
|
);
|
||||||
|
// ...and then load it with the ABI type.
|
||||||
|
let cast_ty = bx.cast_backend_type(cast);
|
||||||
|
llval = bx.load(cast_ty, llscratch, scratch_align);
|
||||||
|
bx.lifetime_end(llscratch, scratch_size);
|
||||||
} else {
|
} else {
|
||||||
// We can't use `PlaceRef::load` here because the argument
|
// We can't use `PlaceRef::load` here because the argument
|
||||||
// may have a type we don't treat as immediate, but the ABI
|
// may have a type we don't treat as immediate, but the ABI
|
||||||
|
|
Loading…
Reference in New Issue