Skip to content

Commit e252865

Browse files
eddybeddyb
committed
trans: always use a memcpy for ABI argument/return casts.
1 parent 9b2beca commit e252865

File tree

6 files changed

+137
-194
lines changed

6 files changed

+137
-194
lines changed

‎src/librustc_trans/abi.rs

+53-15
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010

1111
use llvm::{self, ValueRef};
1212
use base;
13-
use builder::Builder;
14-
use common::{type_is_fat_ptr, BlockAndBuilder};
13+
use build::AllocaFcx;
14+
use common::{type_is_fat_ptr, BlockAndBuilder, C_uint};
1515
use context::CrateContext;
1616
use cabi_x86;
1717
use cabi_x86_64;
@@ -22,14 +22,15 @@ use cabi_powerpc;
2222
use cabi_powerpc64;
2323
use cabi_mips;
2424
use cabi_asmjs;
25-
use machine::{llalign_of_min, llsize_of, llsize_of_real};
25+
use machine::{llalign_of_min, llsize_of, llsize_of_real, llsize_of_store};
2626
use type_::Type;
2727
use type_of;
2828

2929
use rustc::hir;
3030
use rustc::ty::{self, Ty};
3131

3232
use libc::c_uint;
33+
use std::cmp;
3334

3435
pub use syntax::abi::Abi;
3536
pub use rustc::ty::layout::{FAT_PTR_ADDR, FAT_PTR_EXTRA};
@@ -150,26 +151,63 @@ impl ArgType {
150151
/// lvalue for the original Rust type of this argument/return.
151152
/// Can be used for both storing formal arguments into Rust variables
152153
/// or results of call/invoke instructions into their destinations.
153-
pub fn store(&self, b: &Builder, mut val: ValueRef, dst: ValueRef) {
154+
pub fn store(&self, bcx: &BlockAndBuilder, mut val: ValueRef, dst: ValueRef) {
154155
if self.is_ignore() {
155156
return;
156157
}
158+
let ccx = bcx.ccx();
157159
if self.is_indirect() {
158-
let llsz = llsize_of(b.ccx, self.ty);
159-
let llalign = llalign_of_min(b.ccx, self.ty);
160-
base::call_memcpy(b, dst, val, llsz, llalign as u32);
160+
let llsz = llsize_of(ccx, self.ty);
161+
let llalign = llalign_of_min(ccx, self.ty);
162+
base::call_memcpy(bcx, dst, val, llsz, llalign as u32);
161163
} else if let Some(ty) = self.cast {
162-
let cast_dst = b.pointercast(dst, ty.ptr_to());
163-
let store = b.store(val, cast_dst);
164-
let llalign = llalign_of_min(b.ccx, self.ty);
165-
unsafe {
166-
llvm::LLVMSetAlignment(store, llalign);
164+
// FIXME(eddyb): Figure out when the simpler Store is safe, clang
165+
// uses it for i16 -> {i8, i8}, but not for i24 -> {i8, i8, i8}.
166+
let can_store_through_cast_ptr = false;
167+
if can_store_through_cast_ptr {
168+
let cast_dst = bcx.pointercast(dst, ty.ptr_to());
169+
let store = bcx.store(val, cast_dst);
170+
let llalign = llalign_of_min(ccx, self.ty);
171+
unsafe {
172+
llvm::LLVMSetAlignment(store, llalign);
173+
}
174+
} else {
175+
// The actual return type is a struct, but the ABI
176+
// adaptation code has cast it into some scalar type. The
177+
// code that follows is the only reliable way I have
178+
// found to do a transform like i64 -> {i32,i32}.
179+
// Basically we dump the data onto the stack then memcpy it.
180+
//
181+
// Other approaches I tried:
182+
// - Casting rust ret pointer to the foreign type and using Store
183+
// is (a) unsafe if size of foreign type > size of rust type and
184+
// (b) runs afoul of strict aliasing rules, yielding invalid
185+
// assembly under -O (specifically, the store gets removed).
186+
// - Truncating foreign type to correct integral type and then
187+
// bitcasting to the struct type yields invalid cast errors.
188+
189+
// We instead thus allocate some scratch space...
190+
let llscratch = AllocaFcx(bcx.fcx(), ty, "abi_cast");
191+
base::Lifetime::Start.call(bcx, llscratch);
192+
193+
// ...where we first store the value...
194+
bcx.store(val, llscratch);
195+
196+
// ...and then memcpy it to the intended destination.
197+
base::call_memcpy(bcx,
198+
bcx.pointercast(dst, Type::i8p(ccx)),
199+
bcx.pointercast(llscratch, Type::i8p(ccx)),
200+
C_uint(ccx, llsize_of_store(ccx, self.ty)),
201+
cmp::min(llalign_of_min(ccx, self.ty),
202+
llalign_of_min(ccx, ty)) as u32);
203+
204+
base::Lifetime::End.call(bcx, llscratch);
167205
}
168206
} else {
169-
if self.original_ty == Type::i1(b.ccx) {
170-
val = b.zext(val, Type::i8(b.ccx));
207+
if self.original_ty == Type::i1(ccx) {
208+
val = bcx.zext(val, Type::i8(ccx));
171209
}
172-
b.store(val, dst);
210+
bcx.store(val, dst);
173211
}
174212
}
175213

‎src/librustc_trans/base.rs

+42-52
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,7 @@ pub fn with_cond<'blk, 'tcx, F>(bcx: Block<'blk, 'tcx>, val: ValueRef, f: F) ->
10431043
next_cx
10441044
}
10451045

1046-
enum Lifetime { Start, End }
1046+
pub enum Lifetime { Start, End }
10471047

10481048
// If LLVM lifetime intrinsic support is enabled (i.e. optimizations
10491049
// on), and `ptr` is nonzero-sized, then extracts the size of `ptr`
@@ -1080,24 +1080,25 @@ fn core_lifetime_emit<'blk, 'tcx, F>(ccx: &'blk CrateContext<'blk, 'tcx>,
10801080
emit(ccx, size, lifetime_intrinsic)
10811081
}
10821082

1083-
pub fn call_lifetime_start(cx: Block, ptr: ValueRef) {
1084-
core_lifetime_emit(cx.ccx(), ptr, Lifetime::Start, |ccx, size, lifetime_start| {
1085-
let ptr = PointerCast(cx, ptr, Type::i8p(ccx));
1086-
Call(cx,
1087-
lifetime_start,
1088-
&[C_u64(ccx, size), ptr],
1089-
DebugLoc::None);
1090-
})
1083+
impl Lifetime {
1084+
pub fn call(self, b: &Builder, ptr: ValueRef) {
1085+
core_lifetime_emit(b.ccx, ptr, self, |ccx, size, lifetime_intrinsic| {
1086+
let ptr = b.pointercast(ptr, Type::i8p(ccx));
1087+
b.call(lifetime_intrinsic, &[C_u64(ccx, size), ptr], None);
1088+
});
1089+
}
10911090
}
10921091

1093-
pub fn call_lifetime_end(cx: Block, ptr: ValueRef) {
1094-
core_lifetime_emit(cx.ccx(), ptr, Lifetime::End, |ccx, size, lifetime_end| {
1095-
let ptr = PointerCast(cx, ptr, Type::i8p(ccx));
1096-
Call(cx,
1097-
lifetime_end,
1098-
&[C_u64(ccx, size), ptr],
1099-
DebugLoc::None);
1100-
})
1092+
pub fn call_lifetime_start(bcx: Block, ptr: ValueRef) {
1093+
if !bcx.unreachable.get() {
1094+
Lifetime::Start.call(&bcx.build(), ptr);
1095+
}
1096+
}
1097+
1098+
pub fn call_lifetime_end(bcx: Block, ptr: ValueRef) {
1099+
if !bcx.unreachable.get() {
1100+
Lifetime::End.call(&bcx.build(), ptr);
1101+
}
11011102
}
11021103

11031104
// Generates code for resumption of unwind at the end of a landing pad.
@@ -1664,29 +1665,21 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
16641665
arg_ty,
16651666
datum::Lvalue::new("FunctionContext::bind_args"))
16661667
} else {
1667-
let lltmp = if common::type_is_fat_ptr(bcx.tcx(), arg_ty) {
1668-
let lltemp = alloc_ty(bcx, arg_ty, "");
1669-
let b = &bcx.build();
1670-
// we pass fat pointers as two words, but we want to
1671-
// represent them internally as a pointer to two words,
1672-
// so make an alloca to store them in.
1673-
let meta = &self.fn_ty.args[idx];
1674-
idx += 1;
1675-
arg.store_fn_arg(b, &mut llarg_idx, expr::get_dataptr(bcx, lltemp));
1676-
meta.store_fn_arg(b, &mut llarg_idx, expr::get_meta(bcx, lltemp));
1677-
lltemp
1678-
} else {
1679-
// otherwise, arg is passed by value, so store it into a temporary.
1680-
let llarg_ty = arg.cast.unwrap_or(arg.memory_ty(bcx.ccx()));
1681-
let lltemp = alloca(bcx, llarg_ty, "");
1668+
unpack_datum!(bcx, datum::lvalue_scratch_datum(bcx, arg_ty, "",
1669+
uninit_reason,
1670+
arg_scope_id, |bcx, dst| {
1671+
debug!("FunctionContext::bind_args: {:?}: {:?}", hir_arg, arg_ty);
16821672
let b = &bcx.build();
1683-
arg.store_fn_arg(b, &mut llarg_idx, lltemp);
1684-
// And coerce the temporary into the type we expect.
1685-
b.pointercast(lltemp, arg.memory_ty(bcx.ccx()).ptr_to())
1686-
};
1687-
bcx.fcx.schedule_drop_mem(arg_scope_id, lltmp, arg_ty, None);
1688-
datum::Datum::new(lltmp, arg_ty,
1689-
datum::Lvalue::new("bind_args"))
1673+
if common::type_is_fat_ptr(bcx.tcx(), arg_ty) {
1674+
let meta = &self.fn_ty.args[idx];
1675+
idx += 1;
1676+
arg.store_fn_arg(b, &mut llarg_idx, expr::get_dataptr(bcx, dst));
1677+
meta.store_fn_arg(b, &mut llarg_idx, expr::get_meta(bcx, dst));
1678+
} else {
1679+
arg.store_fn_arg(b, &mut llarg_idx, dst);
1680+
}
1681+
bcx
1682+
}))
16901683
}
16911684
} else {
16921685
// FIXME(pcwalton): Reduce the amount of code bloat this is responsible for.
@@ -1721,19 +1714,16 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
17211714
};
17221715

17231716
let pat = &hir_arg.pat;
1724-
bcx = match simple_name(pat) {
1725-
// The check for alloca is necessary because above for the immediate argument case
1726-
// we had to cast. At this point arg_datum is not an alloca anymore and thus
1727-
// breaks debuginfo if we allow this optimisation.
1728-
Some(name)
1729-
if unsafe { llvm::LLVMIsAAllocaInst(arg_datum.val) != ::std::ptr::null_mut() } => {
1730-
// Generate nicer LLVM for the common case of fn a pattern
1731-
// like `x: T`
1732-
set_value_name(arg_datum.val, &bcx.name(name));
1733-
self.lllocals.borrow_mut().insert(pat.id, arg_datum);
1734-
bcx
1735-
},
1736-
_ => _match::bind_irrefutable_pat(bcx, pat, arg_datum.match_input(), arg_scope_id)
1717+
bcx = if let Some(name) = simple_name(pat) {
1718+
// Generate nicer LLVM for the common case of fn a pattern
1719+
// like `x: T`
1720+
set_value_name(arg_datum.val, &bcx.name(name));
1721+
self.lllocals.borrow_mut().insert(pat.id, arg_datum);
1722+
bcx
1723+
} else {
1724+
// General path. Copy out the values that are used in the
1725+
// pattern.
1726+
_match::bind_irrefutable_pat(bcx, pat, arg_datum.match_input(), arg_scope_id)
17371727
};
17381728
debuginfo::create_argument_metadata(bcx, hir_arg);
17391729
}

‎src/librustc_trans/callee.rs

+10-46
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ use build::*;
3434
use cleanup;
3535
use cleanup::CleanupMethods;
3636
use closure;
37-
use common::{self, Block, Result, CrateContext, FunctionContext};
38-
use common::{C_uint, C_undef};
37+
use common::{self, Block, Result, CrateContext, FunctionContext, C_undef};
3938
use consts;
4039
use datum::*;
4140
use debuginfo::DebugLoc;
@@ -44,7 +43,7 @@ use expr;
4443
use glue;
4544
use inline;
4645
use intrinsic;
47-
use machine::{llalign_of_min, llsize_of_store};
46+
use machine::llalign_of_min;
4847
use meth;
4948
use monomorphize::{self, Instance};
5049
use type_::Type;
@@ -58,8 +57,6 @@ use syntax::codemap::DUMMY_SP;
5857
use syntax::errors;
5958
use syntax::ptr::P;
6059

61-
use std::cmp;
62-
6360
#[derive(Debug)]
6461
pub enum CalleeData {
6562
/// Constructor for enum variant/tuple-like-struct.
@@ -689,49 +686,16 @@ fn trans_call_inner<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
689686
let (llret, mut bcx) = base::invoke(bcx, llfn, &llargs, debug_loc);
690687
if !bcx.unreachable.get() {
691688
fn_ty.apply_attrs_callsite(llret);
692-
}
693689

694-
// If the function we just called does not use an outpointer,
695-
// store the result into the rust outpointer. Cast the outpointer
696-
// type to match because some ABIs will use a different type than
697-
// the Rust type. e.g., a {u32,u32} struct could be returned as
698-
// u64.
699-
if !fn_ty.ret.is_ignore() && !fn_ty.ret.is_indirect() {
700-
if let Some(llforeign_ret_ty) = fn_ty.ret.cast {
701-
let llrust_ret_ty = fn_ty.ret.original_ty;
702-
let llretslot = opt_llretslot.unwrap();
703-
704-
// The actual return type is a struct, but the ABI
705-
// adaptation code has cast it into some scalar type. The
706-
// code that follows is the only reliable way I have
707-
// found to do a transform like i64 -> {i32,i32}.
708-
// Basically we dump the data onto the stack then memcpy it.
709-
//
710-
// Other approaches I tried:
711-
// - Casting rust ret pointer to the foreign type and using Store
712-
// is (a) unsafe if size of foreign type > size of rust type and
713-
// (b) runs afoul of strict aliasing rules, yielding invalid
714-
// assembly under -O (specifically, the store gets removed).
715-
// - Truncating foreign type to correct integral type and then
716-
// bitcasting to the struct type yields invalid cast errors.
717-
let llscratch = base::alloca(bcx, llforeign_ret_ty, "__cast");
718-
base::call_lifetime_start(bcx, llscratch);
719-
Store(bcx, llret, llscratch);
720-
let llscratch_i8 = PointerCast(bcx, llscratch, Type::i8(ccx).ptr_to());
721-
let llretptr_i8 = PointerCast(bcx, llretslot, Type::i8(ccx).ptr_to());
722-
let llrust_size = llsize_of_store(ccx, llrust_ret_ty);
723-
let llforeign_align = llalign_of_min(ccx, llforeign_ret_ty);
724-
let llrust_align = llalign_of_min(ccx, llrust_ret_ty);
725-
let llalign = cmp::min(llforeign_align, llrust_align);
726-
debug!("llrust_size={}", llrust_size);
727-
728-
if !bcx.unreachable.get() {
729-
base::call_memcpy(&B(bcx), llretptr_i8, llscratch_i8,
730-
C_uint(ccx, llrust_size), llalign as u32);
690+
// If the function we just called does not use an outpointer,
691+
// store the result into the rust outpointer. Cast the outpointer
692+
// type to match because some ABIs will use a different type than
693+
// the Rust type. e.g., a {u32,u32} struct could be returned as
694+
// u64.
695+
if !fn_ty.ret.is_indirect() {
696+
if let Some(llretslot) = opt_llretslot {
697+
fn_ty.ret.store(&bcx.build(), llret, llretslot);
731698
}
732-
base::call_lifetime_end(bcx, llscratch);
733-
} else if let Some(llretslot) = opt_llretslot {
734-
base::store_ty(bcx, llret, llretslot, output.unwrap());
735699
}
736700
}
737701

0 commit comments

Comments
 (0)