From 6ef08406dccdeeb828a6cfc7658ac149900e73ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Thu, 8 Jan 2015 16:28:07 +0100 Subject: [PATCH] Avoid loads/stores of first class aggregates Currently, small aggregates are passed to functions as immediate values as is. This has two consequences. One is that aggregates are passed component-wise by LLVM, so e.g. a struct containing four u8 values (e.g. an RGBA struct) will be passed as four individual values. The other is that LLVM isn't very good at optimizing loads/stores of first class attributes. What clang does is converting the aggregate to an appropriately sized integer type (e.g. i32 for the four u8 values), and using that for the function argument. This allows LLVM to create code that is a lot better. Fixes #20450 #20149 #16506 #13927 --- src/librustc_trans/trans/base.rs | 12 +++++++++++- src/librustc_trans/trans/foreign.rs | 11 +++++++++-- src/librustc_trans/trans/glue.rs | 2 +- src/librustc_trans/trans/intrinsic.rs | 13 +++++++++---- src/librustc_trans/trans/type_.rs | 12 ++++++++++++ src/librustc_trans/trans/type_of.rs | 15 +++++++++++++++ 6 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 88ce36a710a0c..ea98d6bb74e95 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -1054,6 +1054,11 @@ pub fn load_ty<'blk, 'tcx>(cx: Block<'blk, 'tcx>, C_undef(type_of::type_of(cx.ccx(), t)) } else if ty::type_is_bool(t) { Trunc(cx, LoadRangeAssert(cx, ptr, 0, 2, llvm::False), Type::i1(cx.ccx())) + } else if type_is_immediate(cx.ccx(), t) && type_of::type_of(cx.ccx(), t).is_aggregate() { + // We want to pass small aggregates as immediate values, but using an aggregate LLVM type + // for this leads to bad optimizations, so its arg type is an appropriately sized integer + // and we have to convert it + Load(cx, BitCast(cx, ptr, type_of::arg_type_of(cx.ccx(), t).ptr_to())) } else if ty::type_is_char(t) { // a char is a Unicode codepoint, and so takes values from 0 // to 0x10FFFF inclusive only. @@ -1065,9 +1070,14 @@ pub fn load_ty<'blk, 'tcx>(cx: Block<'blk, 'tcx>, /// Helper for storing values in memory. Does the necessary conversion if the in-memory type /// differs from the type used for SSA values. -pub fn store_ty(cx: Block, v: ValueRef, dst: ValueRef, t: Ty) { +pub fn store_ty<'blk, 'tcx>(cx: Block<'blk, 'tcx>, v: ValueRef, dst: ValueRef, t: Ty<'tcx>) { if ty::type_is_bool(t) { Store(cx, ZExt(cx, v, Type::i8(cx.ccx())), dst); + } else if type_is_immediate(cx.ccx(), t) && type_of::type_of(cx.ccx(), t).is_aggregate() { + // We want to pass small aggregates as immediate values, but using an aggregate LLVM type + // for this leads to bad optimizations, so its arg type is an appropriately sized integer + // and we have to convert it + Store(cx, v, BitCast(cx, dst, type_of::arg_type_of(cx.ccx(), t).ptr_to())); } else { Store(cx, v, dst); }; diff --git a/src/librustc_trans/trans/foreign.rs b/src/librustc_trans/trans/foreign.rs index 3dfb36c854b72..fb61bab6ade2c 100644 --- a/src/librustc_trans/trans/foreign.rs +++ b/src/librustc_trans/trans/foreign.rs @@ -736,6 +736,13 @@ pub fn trans_rust_fn_with_foreign_abi<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, if ty::type_is_bool(rust_ty) { let tmp = builder.load_range_assert(llforeign_arg, 0, 2, llvm::False); builder.trunc(tmp, Type::i1(ccx)) + } else if type_of::type_of(ccx, rust_ty).is_aggregate() { + // We want to pass small aggregates as immediate values, but using an aggregate + // LLVM type for this leads to bad optimizations, so its arg type is an + // appropriately sized integer and we have to convert it + let tmp = builder.bitcast(llforeign_arg, + type_of::arg_type_of(ccx, rust_ty).ptr_to()); + builder.load(tmp) } else { builder.load(llforeign_arg) } @@ -834,10 +841,10 @@ fn foreign_signature<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, fn_sig: &ty::FnSig<'tcx>, arg_tys: &[Ty<'tcx>]) -> LlvmSignature { - let llarg_tys = arg_tys.iter().map(|&arg| arg_type_of(ccx, arg)).collect(); + let llarg_tys = arg_tys.iter().map(|&arg| foreign_arg_type_of(ccx, arg)).collect(); let (llret_ty, ret_def) = match fn_sig.output { ty::FnConverging(ret_ty) => - (type_of::arg_type_of(ccx, ret_ty), !return_type_is_void(ccx, ret_ty)), + (type_of::foreign_arg_type_of(ccx, ret_ty), !return_type_is_void(ccx, ret_ty)), ty::FnDiverging => (Type::nil(ccx), false) }; diff --git a/src/librustc_trans/trans/glue.rs b/src/librustc_trans/trans/glue.rs index 2219cd59263cc..a79bb6ca16470 100644 --- a/src/librustc_trans/trans/glue.rs +++ b/src/librustc_trans/trans/glue.rs @@ -138,7 +138,7 @@ pub fn drop_ty_immediate<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, -> Block<'blk, 'tcx> { let _icx = push_ctxt("drop_ty_immediate"); let vp = alloca(bcx, type_of(bcx.ccx(), t), ""); - Store(bcx, v, vp); + store_ty(bcx, v, vp, t); drop_ty(bcx, vp, t, source_location) } diff --git a/src/librustc_trans/trans/intrinsic.rs b/src/librustc_trans/trans/intrinsic.rs index b22c7f763f035..91c7409182df6 100644 --- a/src/librustc_trans/trans/intrinsic.rs +++ b/src/librustc_trans/trans/intrinsic.rs @@ -357,11 +357,11 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, &ccx.link_meta().crate_hash); // NB: This needs to be kept in lockstep with the TypeId struct in // the intrinsic module - C_named_struct(llret_ty, &[C_u64(ccx, hash)]) + C_u64(ccx, hash) } (_, "init") => { let tp_ty = *substs.types.get(FnSpace, 0); - let lltp_ty = type_of::type_of(ccx, tp_ty); + let lltp_ty = type_of::arg_type_of(ccx, tp_ty); if return_type_is_void(ccx, tp_ty) { C_nil(ccx) } else { @@ -686,6 +686,11 @@ fn with_overflow_intrinsic<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, name: &'static st let ret = C_undef(type_of::type_of(bcx.ccx(), t)); let ret = InsertValue(bcx, ret, result, 0); let ret = InsertValue(bcx, ret, overflow, 1); - - ret + if type_is_immediate(bcx.ccx(), t) { + let tmp = alloc_ty(bcx, t, "tmp"); + Store(bcx, ret, tmp); + load_ty(bcx, tmp, t) + } else { + ret + } } diff --git a/src/librustc_trans/trans/type_.rs b/src/librustc_trans/trans/type_.rs index 71a7789eb3932..9cae142c03ac1 100644 --- a/src/librustc_trans/trans/type_.rs +++ b/src/librustc_trans/trans/type_.rs @@ -82,6 +82,11 @@ impl Type { ty!(llvm::LLVMInt64TypeInContext(ccx.llcx())) } + // Creates an integer type with the given number of bits, e.g. i24 + pub fn ix(ccx: &CrateContext, num_bits: u64) -> Type { + ty!(llvm::LLVMIntTypeInContext(ccx.llcx(), num_bits as c_uint)) + } + pub fn f32(ccx: &CrateContext) -> Type { ty!(llvm::LLVMFloatTypeInContext(ccx.llcx())) } @@ -260,6 +265,13 @@ impl Type { ty!(llvm::LLVMPointerType(self.to_ref(), 0)) } + pub fn is_aggregate(&self) -> bool { + match self.kind() { + TypeKind::Struct | TypeKind::Array => true, + _ => false + } + } + pub fn is_packed(&self) -> bool { unsafe { llvm::LLVMIsPackedStruct(self.to_ref()) == True diff --git a/src/librustc_trans/trans/type_of.rs b/src/librustc_trans/trans/type_of.rs index 9933079742229..76e0e0d054557 100644 --- a/src/librustc_trans/trans/type_of.rs +++ b/src/librustc_trans/trans/type_of.rs @@ -243,9 +243,24 @@ pub fn sizing_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>) -> Typ llsizingty } +pub fn foreign_arg_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>) -> Type { + if ty::type_is_bool(t) { + Type::i1(cx) + } else { + type_of(cx, t) + } +} + pub fn arg_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>) -> Type { if ty::type_is_bool(t) { Type::i1(cx) + } else if type_is_immediate(cx, t) && type_of(cx, t).is_aggregate() { + // We want to pass small aggregates as immediate values, but using an aggregate LLVM type + // for this leads to bad optimizations, so its arg type is an appropriately sized integer + match machine::llsize_of_alloc(cx, sizing_type_of(cx, t)) { + 0 => type_of(cx, t), + n => Type::ix(cx, n * 8), + } } else { type_of(cx, t) }