From b7f71e1ee661ea0d5d9731fcf4779a452bbee486 Mon Sep 17 00:00:00 2001 From: Tom Lee Date: Mon, 27 May 2013 18:33:57 -0700 Subject: [PATCH] Implementing suggestions from @nikomatsakis --- src/librustc/middle/trans/base.rs | 87 +++++++++++++++------------- src/librustc/middle/trans/cabi.rs | 3 +- src/librustc/middle/trans/common.rs | 45 +++++++------- src/librustc/middle/trans/foreign.rs | 51 +++++++--------- src/librustc/middle/trans/glue.rs | 5 +- src/librustc/middle/trans/reflect.rs | 12 ++-- 6 files changed, 102 insertions(+), 101 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index ed3ac5fadb4e..5bbfaac0792a 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1593,7 +1593,7 @@ pub fn new_fn_ctxt_w_id(ccx: @CrateContext, impl_id: Option, param_substs: Option<@param_substs>, sp: Option) - -> (fn_ctxt, bool) { + -> fn_ctxt { for param_substs.each |p| { p.validate(); } debug!("new_fn_ctxt_w_id(path=%s, id=%?, impl_id=%?, \ @@ -1611,12 +1611,11 @@ pub fn new_fn_ctxt_w_id(ccx: @CrateContext, ty::subst_tps(ccx.tcx, substs.tys, substs.self_ty, output_type) } }; - let imm = ty::type_is_immediate(substd_output_type); - + let is_immediate = ty::type_is_immediate(substd_output_type); let fcx = @mut fn_ctxt_ { llfn: llfndecl, llenv: unsafe { - llvm::LLVMGetParam(llfndecl, arg_env(imm) as c_uint) + llvm::LLVMGetUndef(T_ptr(T_i8())) }, llretptr: None, llstaticallocas: llbbs.sa, @@ -1625,7 +1624,7 @@ pub fn new_fn_ctxt_w_id(ccx: @CrateContext, llself: None, personality: None, loop_ret: None, - has_immediate_return_value: imm, + has_immediate_return_value: is_immediate, llargs: @mut HashMap::new(), lllocals: @mut HashMap::new(), llupvars: @mut HashMap::new(), @@ -1636,9 +1635,11 @@ pub fn new_fn_ctxt_w_id(ccx: @CrateContext, path: path, ccx: @ccx }; - + fcx.llenv = unsafe { + llvm::LLVMGetParam(llfndecl, fcx.env_arg_pos() as c_uint) + }; fcx.llretptr = Some(make_return_pointer(fcx, substd_output_type)); - (fcx, imm) + fcx } pub fn new_fn_ctxt(ccx: @CrateContext, @@ -1646,7 +1647,7 @@ pub fn new_fn_ctxt(ccx: @CrateContext, llfndecl: ValueRef, output_type: ty::t, sp: Option) - -> (fn_ctxt, bool) { + -> fn_ctxt { new_fn_ctxt_w_id(ccx, path, llfndecl, -1, output_type, None, None, sp) } @@ -1666,8 +1667,7 @@ pub fn new_fn_ctxt(ccx: @CrateContext, // field of the fn_ctxt with pub fn create_llargs_for_fn_args(cx: fn_ctxt, self_arg: self_arg, - args: &[ast::arg], - ret_imm: bool) + args: &[ast::arg]) -> ~[ValueRef] { let _icx = cx.insn_ctxt("create_llargs_for_fn_args"); @@ -1693,7 +1693,7 @@ pub fn create_llargs_for_fn_args(cx: fn_ctxt, // llvm::LLVMGetParam for each argument. vec::from_fn(args.len(), |i| { unsafe { - let arg_n = arg_pos(ret_imm, i); + let arg_n = cx.arg_pos(i); let arg = &args[i]; let llarg = llvm::LLVMGetParam(cx.llfn, arg_n as c_uint); @@ -1832,15 +1832,15 @@ pub fn trans_closure(ccx: @CrateContext, param_substs.repr(ccx.tcx)); // Set up arguments to the function. - let (fcx, imm) = new_fn_ctxt_w_id(ccx, - path, - llfndecl, - id, - output_type, - impl_id, - param_substs, - Some(body.span)); - let raw_llargs = create_llargs_for_fn_args(fcx, self_arg, decl.inputs, imm); + let fcx = new_fn_ctxt_w_id(ccx, + path, + llfndecl, + id, + output_type, + impl_id, + param_substs, + Some(body.span)); + let raw_llargs = create_llargs_for_fn_args(fcx, self_arg, decl.inputs); // Set the fixed stack segment flag if necessary. if attr::attrs_contains_name(attributes, "fixed_stack_segment") { @@ -1965,16 +1965,16 @@ pub fn trans_enum_variant(ccx: @CrateContext, ty_param_substs, None, ty::node_id_to_type(ccx.tcx, enum_id)); - let (fcx, imm) = new_fn_ctxt_w_id(ccx, - ~[], - llfndecl, - variant.node.id, - enum_ty, - None, - param_substs, - None); - - let raw_llargs = create_llargs_for_fn_args(fcx, no_self, fn_args, imm); + let fcx = new_fn_ctxt_w_id(ccx, + ~[], + llfndecl, + variant.node.id, + enum_ty, + None, + param_substs, + None); + + let raw_llargs = create_llargs_for_fn_args(fcx, no_self, fn_args); let bcx = top_scope_block(fcx, None), lltop = bcx.llbb; let arg_tys = ty::ty_fn_args(node_id_type(bcx, variant.node.id)); let bcx = copy_args_to_allocas(fcx, bcx, fn_args, raw_llargs, arg_tys); @@ -2044,16 +2044,16 @@ pub fn trans_tuple_struct(ccx: @CrateContext, ty_to_str(ccx.tcx, ctor_ty))) }; - let (fcx, imm) = new_fn_ctxt_w_id(ccx, - ~[], - llfndecl, - ctor_id, - tup_ty, - None, - param_substs, - None); + let fcx = new_fn_ctxt_w_id(ccx, + ~[], + llfndecl, + ctor_id, + tup_ty, + None, + param_substs, + None); - let raw_llargs = create_llargs_for_fn_args(fcx, no_self, fn_args, imm); + let raw_llargs = create_llargs_for_fn_args(fcx, no_self, fn_args); let bcx = top_scope_block(fcx, None); let lltop = bcx.llbb; @@ -2301,14 +2301,19 @@ pub fn create_entry_wrapper(ccx: @CrateContext, let llfdecl = decl_fn(ccx.llmod, "_rust_main", lib::llvm::CCallConv, llfty); - let (fcx, _) = new_fn_ctxt(ccx, ~[], llfdecl, nt, None); + let fcx = new_fn_ctxt(ccx, ~[], llfdecl, nt, None); + + // the args vector built in create_entry_fn will need + // be updated if this assertion starts to fail. + assert!(fcx.has_immediate_return_value); let bcx = top_scope_block(fcx, None); let lltop = bcx.llbb; // Call main. let llenvarg = unsafe { - llvm::LLVMGetParam(llfdecl, arg_env(true) as c_uint) + let env_arg = fcx.env_arg_pos(); + llvm::LLVMGetParam(llfdecl, env_arg as c_uint) }; let args = ~[llenvarg]; let llresult = Call(bcx, main_llfn, args); diff --git a/src/librustc/middle/trans/cabi.rs b/src/librustc/middle/trans/cabi.rs index ecf9963b68a7..c6f4d2304197 100644 --- a/src/librustc/middle/trans/cabi.rs +++ b/src/librustc/middle/trans/cabi.rs @@ -132,8 +132,7 @@ pub impl FnType { bcx: block, ret_ty: TypeRef, llwrapfn: ValueRef, - llargbundle: ValueRef, - ret_imm: bool) { + llargbundle: ValueRef) { let mut atys = /*bad*/copy self.arg_tys; let mut attrs = /*bad*/copy self.attrs; let mut j = 0u; diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index 00b679766839..ad5dadaf8701 100644 --- a/src/librustc/middle/trans/common.rs +++ b/src/librustc/middle/trans/common.rs @@ -351,6 +351,30 @@ pub struct fn_ctxt_ { ccx: @@CrateContext } +pub impl fn_ctxt_ { + pub fn arg_pos(&self, arg: uint) -> uint { + if self.has_immediate_return_value { + arg + 1u + } else { + arg + 2u + } + } + + pub fn out_arg_pos(&self) -> uint { + assert!(self.has_immediate_return_value); + 0u + } + + pub fn env_arg_pos(&self) -> uint { + if !self.has_immediate_return_value { + 1u + } else { + 0u + } + } + +} + pub type fn_ctxt = @mut fn_ctxt_; pub fn warn_not_to_commit(ccx: @CrateContext, msg: &str) { @@ -660,27 +684,6 @@ pub fn mk_block(llbb: BasicBlockRef, parent: Option, kind: block_kind, @mut block_(llbb, parent, kind, is_lpad, node_info, fcx) } -pub fn arg_pos(ret_imm: bool, arg: uint) -> uint { - if ret_imm { - arg + 1u - } else { - arg + 2u - } -} - -pub fn arg_out(ret_imm: bool) -> uint { - assert!(ret_imm); - 0u -} - -pub fn arg_env(ret_imm: bool) -> uint { - if !ret_imm { - 1u - } else { - 0u - } -} - pub struct Result { bcx: block, val: ValueRef diff --git a/src/librustc/middle/trans/foreign.rs b/src/librustc/middle/trans/foreign.rs index 08e17a4d2ec2..63356a135d05 100644 --- a/src/librustc/middle/trans/foreign.rs +++ b/src/librustc/middle/trans/foreign.rs @@ -150,14 +150,10 @@ fn build_shim_fn_(ccx: @CrateContext, ccx.llmod, shim_name, tys.shim_fn_ty); // Declare the body of the shim function: - let (fcx, imm) = new_fn_ctxt(ccx, ~[], llshimfn, tys.fn_sig.output, None); + let fcx = new_fn_ctxt(ccx, ~[], llshimfn, tys.fn_sig.output, None); let bcx = top_scope_block(fcx, None); let lltop = bcx.llbb; - // - // FIXME [#6575] this seems to be making the assumption that the first - // implicit argument is always available? - // let llargbundle = get_param(llshimfn, 0u); let llargvals = arg_builder(bcx, tys, llargbundle); @@ -179,8 +175,7 @@ fn build_shim_fn_(ccx: @CrateContext, type wrap_arg_builder<'self> = &'self fn(bcx: block, tys: &ShimTypes, llwrapfn: ValueRef, - llargbundle: ValueRef, - ret_imm: bool); + llargbundle: ValueRef); type wrap_ret_builder<'self> = &'self fn(bcx: block, tys: &ShimTypes, @@ -195,7 +190,7 @@ fn build_wrap_fn_(ccx: @CrateContext, arg_builder: wrap_arg_builder, ret_builder: wrap_ret_builder) { let _icx = ccx.insn_ctxt("foreign::build_wrap_fn_"); - let (fcx, imm) = new_fn_ctxt(ccx, ~[], llwrapfn, tys.fn_sig.output, None); + let fcx = new_fn_ctxt(ccx, ~[], llwrapfn, tys.fn_sig.output, None); // Patch up the return type if it's not immediate and we're returning via // the C ABI. @@ -210,7 +205,7 @@ fn build_wrap_fn_(ccx: @CrateContext, // Allocate the struct and write the arguments into it. let llargbundle = alloca(bcx, tys.bundle_ty); - arg_builder(bcx, tys, llwrapfn, llargbundle, imm); + arg_builder(bcx, tys, llwrapfn, llargbundle); // Create call itself. let llshimfnptr = PointerCast(bcx, llshimfn, T_ptr(T_i8())); @@ -438,14 +433,14 @@ pub fn trans_foreign_mod(ccx: @CrateContext, cc: lib::llvm::CallConv) { debug!("build_direct_fn(%s)", *link_name(ccx, item)); - let (fcx, imm) = new_fn_ctxt(ccx, ~[], decl, tys.fn_sig.output, None); + let fcx = new_fn_ctxt(ccx, ~[], decl, tys.fn_sig.output, None); let bcx = top_scope_block(fcx, None), lltop = bcx.llbb; let llbasefn = base_fn(ccx, *link_name(ccx, item), tys, cc); let ty = ty::lookup_item_type(ccx.tcx, ast_util::local_def(item.id)).ty; let ret_ty = ty::ty_fn_ret(ty); let args = vec::from_fn(ty::ty_fn_args(ty).len(), |i| { - get_param(decl, arg_pos(imm, i)) + get_param(decl, fcx.arg_pos(i)) }); let retval = Call(bcx, llbasefn, args); if !ty::type_is_nil(ret_ty) && !ty::type_is_bot(ret_ty) { @@ -464,7 +459,7 @@ pub fn trans_foreign_mod(ccx: @CrateContext, cc: lib::llvm::CallConv) { debug!("build_fast_ffi_fn(%s)", *link_name(ccx, item)); - let (fcx, imm) = new_fn_ctxt(ccx, ~[], decl, tys.fn_sig.output, None); + let fcx = new_fn_ctxt(ccx, ~[], decl, tys.fn_sig.output, None); let bcx = top_scope_block(fcx, None), lltop = bcx.llbb; let llbasefn = base_fn(ccx, *link_name(ccx, item), tys, cc); set_no_inline(fcx.llfn); @@ -473,7 +468,7 @@ pub fn trans_foreign_mod(ccx: @CrateContext, ast_util::local_def(item.id)).ty; let ret_ty = ty::ty_fn_ret(ty); let args = vec::from_fn(ty::ty_fn_args(ty).len(), |i| { - get_param(decl, arg_pos(imm, i)) + get_param(decl, fcx.arg_pos(i)) }); let retval = Call(bcx, llbasefn, args); if !ty::type_is_nil(ret_ty) && !ty::type_is_bot(ret_ty) { @@ -514,13 +509,13 @@ pub fn trans_foreign_mod(ccx: @CrateContext, fn build_args(bcx: block, tys: &ShimTypes, llwrapfn: ValueRef, - llargbundle: ValueRef, - ret_imm: bool) { + llargbundle: ValueRef) { let _icx = bcx.insn_ctxt("foreign::wrap::build_args"); let ccx = bcx.ccx(); let n = tys.llsig.llarg_tys.len(); for uint::range(0, n) |i| { - let mut llargval = get_param(llwrapfn, arg_pos(ret_imm, i)); + let arg_i = bcx.fcx.arg_pos(i); + let mut llargval = get_param(llwrapfn, arg_i); // In some cases, Rust will pass a pointer which the // native C type doesn't have. In that case, just @@ -558,14 +553,14 @@ pub fn trans_intrinsic(ccx: @CrateContext, let output_type = ty::ty_fn_ret(ty::node_id_to_type(ccx.tcx, item.id)); - let (fcx, imm) = new_fn_ctxt_w_id(ccx, - path, - decl, - item.id, - output_type, - None, - Some(substs), - Some(item.span)); + let fcx = new_fn_ctxt_w_id(ccx, + path, + decl, + item.id, + output_type, + None, + Some(substs), + Some(item.span)); // Set the fixed stack segment flag if necessary. if attr::attrs_contains_name(attributes, "fixed_stack_segment") { @@ -574,7 +569,7 @@ pub fn trans_intrinsic(ccx: @CrateContext, let mut bcx = top_scope_block(fcx, None); let lltop = bcx.llbb; - let first_real_arg = arg_pos(imm, 0u); + let first_real_arg = fcx.arg_pos(0u); match *ccx.sess.str_of(item.ident) { ~"atomic_cxchg" => { let old = AtomicCmpXchg(bcx, @@ -1356,14 +1351,12 @@ pub fn trans_foreign_fn(ccx: @CrateContext, fn build_args(bcx: block, tys: &ShimTypes, llwrapfn: ValueRef, - llargbundle: ValueRef, - ret_imm: bool) { + llargbundle: ValueRef) { let _icx = bcx.insn_ctxt("foreign::foreign::wrap::build_args"); tys.fn_ty.build_wrap_args(bcx, tys.llsig.llret_ty, llwrapfn, - llargbundle, - ret_imm); + llargbundle); } fn build_ret(bcx: block, tys: &ShimTypes, llargbundle: ValueRef) { diff --git a/src/librustc/middle/trans/glue.rs b/src/librustc/middle/trans/glue.rs index 29709cd553d5..405e5e36de79 100644 --- a/src/librustc/middle/trans/glue.rs +++ b/src/librustc/middle/trans/glue.rs @@ -727,7 +727,7 @@ pub fn make_generic_glue_inner(ccx: @CrateContext, helper: glue_helper) -> ValueRef { let _icx = ccx.insn_ctxt("make_generic_glue_inner"); - let (fcx, imm) = new_fn_ctxt(ccx, ~[], llfn, ty::mk_nil(), None); + let fcx = new_fn_ctxt(ccx, ~[], llfn, ty::mk_nil(), None); lib::llvm::SetLinkage(llfn, lib::llvm::InternalLinkage); ccx.stats.n_glues_created += 1u; // All glue functions take values passed *by alias*; this is a @@ -740,7 +740,8 @@ pub fn make_generic_glue_inner(ccx: @CrateContext, let bcx = top_scope_block(fcx, None); let lltop = bcx.llbb; - let llrawptr0 = unsafe { llvm::LLVMGetParam(llfn, arg_pos(true, 1u) as c_uint) }; + let rawptr0_arg = fcx.arg_pos(1u); + let llrawptr0 = unsafe { llvm::LLVMGetParam(llfn, rawptr0_arg as c_uint) }; helper(bcx, llrawptr0, t); finish_fn(fcx, lltop); return llfn; diff --git a/src/librustc/middle/trans/reflect.rs b/src/librustc/middle/trans/reflect.rs index 681b11423ddc..839c9a96b78e 100644 --- a/src/librustc/middle/trans/reflect.rs +++ b/src/librustc/middle/trans/reflect.rs @@ -286,19 +286,19 @@ pub impl Reflector { let llfty = type_of_fn(ccx, [opaqueptrty], ty::mk_int()); let llfdecl = decl_internal_cdecl_fn(ccx.llmod, sym, llfty); + let fcx = new_fn_ctxt(ccx, + ~[], + llfdecl, + ty::mk_uint(), + None); let arg = unsafe { // // we know the return type of llfdecl is an int here, so // no need for a special check to see if the return type // is immediate. // - llvm::LLVMGetParam(llfdecl, arg_pos(true, 0u) as c_uint) + llvm::LLVMGetParam(llfdecl, fcx.arg_pos(0u) as c_uint) }; - let (fcx, _) = new_fn_ctxt(ccx, - ~[], - llfdecl, - ty::mk_uint(), - None); let bcx = top_scope_block(fcx, None); let arg = BitCast(bcx, arg, llptrty); let ret = adt::trans_get_discr(bcx, repr, arg);