nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-26 00:34:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

auto merge of #7452 : dotdash/rust/self_indirection, r=cmr

Browse Source 
Currently we pass all "self" arguments by reference, for the pointer
variants this means that we end up with double indirection which causes
a unnecessary performance hit.

The fix itself is pretty straight-forward and just means that "self"
needs to be handled like any other argument, except for by-value "self"
which still needs to be passed by reference. This is because
non-pointer types can't just be stuffed into the environment slot which
is used to pass "self".

What made things tricky is that there was also a bug in the typechecker
where the method map entries are created. For type impls, that stored
the base type instead of the actual self-type in the method map, e.g.
Foo instead of &Foo for &self. That worked with pass-by-reference, but
fails with pass-by-value which needs the real type.

Code that makes use of methods seems to be about 10% faster with this
change. Also, build times are reduced by about 4%.

Fixes #4355, #4402, #5280, #4406 and #7285
This commit is contained in:
bors 2013-06-29 16:46:32 -07:00
commit df39932090
8 changed files with 82 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/librustc/middle/trans/asm.rs
View File

@ -41,7 +41,6 @@ pub fn trans_inline_asm(bcx: block, ia: &ast::inline_asm) -> block {
callee::trans_arg_expr(bcx,
expr_ty(bcx, out),
ty::ByCopy,
ast::sty_static,
out,
&mut cleanups,
None,
@ -57,7 +56,6 @@ pub fn trans_inline_asm(bcx: block, ia: &ast::inline_asm) -> block {
callee::trans_arg_expr(bcx,
expr_ty(bcx, e),
ty::ByCopy,
ast::sty_static,
e,
&mut cleanups,
None,
@ -79,7 +77,6 @@ pub fn trans_inline_asm(bcx: block, ia: &ast::inline_asm) -> block {
callee::trans_arg_expr(bcx,
expr_ty(bcx, in),
ty::ByCopy,
ast::sty_static,
in,
&mut cleanups,
None,

29
src/librustc/middle/trans/base.rs
View File

@ -1626,18 +1626,11 @@ pub fn create_llargs_for_fn_args(cx: fn_ctxt,
let _icx = push_ctxt("create_llargs_for_fn_args");
match self_arg {
impl_self(tt) => {
impl_self(tt, self_mode) => {
cx.llself = Some(ValSelfData {
v: cx.llenv,
t: tt,
is_owned: false
});
}
impl_owned_self(tt) => {
cx.llself = Some(ValSelfData {
v: cx.llenv,
t: tt,
is_owned: true
is_copy: self_mode == ty::ByCopy
});
}
no_self => ()
@ -1676,12 +1669,18 @@ pub fn copy_args_to_allocas(fcx: fn_ctxt,
match fcx.llself {
Some(slf) => {
let self_val = PointerCast(bcx, slf.v, type_of(bcx.ccx(), slf.t).ptr_to());
fcx.llself = Some(ValSelfData {v: self_val, ..slf});
let self_val = if slf.is_copy
&& datum::appropriate_mode(slf.t).is_by_value() {
let tmp = BitCast(bcx, slf.v, type_of(bcx.ccx(), slf.t));
let alloc = alloc_ty(bcx, slf.t);
Store(bcx, tmp, alloc);
alloc
} else {
PointerCast(bcx, slf.v, type_of(bcx.ccx(), slf.t).ptr_to())
};
if slf.is_owned {
add_clean(bcx, slf.v, slf.t);
}
fcx.llself = Some(ValSelfData {v: self_val, ..slf});
add_clean(bcx, self_val, slf.t);
}
_ => {}
}
@ -1758,7 +1757,7 @@ pub fn tie_up_header_blocks(fcx: fn_ctxt, lltop: BasicBlockRef) {
}
}
pub enum self_arg { impl_self(ty::t), impl_owned_self(ty::t), no_self, }
pub enum self_arg { impl_self(ty::t, ty::SelfMode), no_self, }
// trans_closure: Builds an LLVM function out of a source function.
// If the function closes over its environment a closure will be

52
src/librustc/middle/trans/callee.rs
View File

@ -62,9 +62,9 @@ pub struct FnData {
pub struct MethodData {
llfn: ValueRef,
llself: ValueRef,
temp_cleanup: Option<ValueRef>,
self_ty: ty::t,
self_mode: ty::SelfMode,
explicit_self: ast::explicit_self_
}
pub enum CalleeData {
@ -615,10 +615,7 @@ pub fn trans_call_inner(in_cx: block,
}
Method(d) => {
// Weird but true: we pass self in the *environment* slot!
let llself = PointerCast(bcx,
d.llself,
Type::opaque_box(ccx).ptr_to());
(d.llfn, llself)
(d.llfn, d.llself)
}
Closure(d) => {
// Closures are represented as (llfn, llclosure) pair:
@ -647,11 +644,12 @@ pub fn trans_call_inner(in_cx: block,
// Now that the arguments have finished evaluating, we need to revoke
// the cleanup for the self argument, if it exists
// the cleanup for the self argument
match callee.data {
Method(d) if d.self_mode == ty::ByCopy ||
d.explicit_self == ast::sty_value => {
revoke_clean(bcx, d.llself);
Method(d) => {
for d.temp_cleanup.iter().advance |&v| {
revoke_clean(bcx, v);
}
}
_ => {}
}
@ -772,7 +770,6 @@ pub fn trans_args(cx: block,
trans_arg_expr(bcx,
arg_tys[i],
ty::ByCopy,
ast::sty_static,
*arg_expr,
&mut temp_cleanups,
if i == last { ret_flag } else { None },
@ -806,7 +803,6 @@ pub enum AutorefArg {
pub fn trans_arg_expr(bcx: block,
formal_arg_ty: ty::t,
self_mode: ty::SelfMode,
ex_self: ast::explicit_self_,
arg_expr: @ast::expr,
temp_cleanups: &mut ~[ValueRef],
ret_flag: Option<ValueRef>,
@ -814,10 +810,9 @@ pub fn trans_arg_expr(bcx: block,
let _icx = push_ctxt("trans_arg_expr");
let ccx = bcx.ccx();
debug!("trans_arg_expr(formal_arg_ty=(%s), explicit_self=%? self_mode=%?, arg_expr=%s, \
debug!("trans_arg_expr(formal_arg_ty=(%s), self_mode=%?, arg_expr=%s, \
ret_flag=%?)",
formal_arg_ty.repr(bcx.tcx()),
ex_self,
self_mode,
arg_expr.repr(bcx.tcx()),
ret_flag.map(|v| bcx.val_to_str(*v)));
@ -877,9 +872,15 @@ pub fn trans_arg_expr(bcx: block,
val = arg_datum.to_ref_llval(bcx);
}
DontAutorefArg => {
match (self_mode, ex_self) {
(ty::ByRef, ast::sty_value) => {
debug!("by value self with type %s, storing to scratch",
match self_mode {
ty::ByRef => {
// This assertion should really be valid, but because
// the explicit self code currently passes by-ref, it
// does not hold.
//
//assert !bcx.ccx().maps.moves_map.contains_key(
// &arg_expr.id);
debug!("by ref arg with type %s, storing to scratch",
bcx.ty_to_str(arg_datum.ty));
let scratch = scratch_datum(bcx, arg_datum.ty, false);
@ -896,18 +897,7 @@ pub fn trans_arg_expr(bcx: block,
val = scratch.to_ref_llval(bcx);
}
(ty::ByRef, _) => {
// This assertion should really be valid, but because
// the explicit self code currently passes by-ref, it
// does not hold.
//
//assert !bcx.ccx().maps.moves_map.contains_key(
// &arg_expr.id);
debug!("by ref arg with type %s",
bcx.ty_to_str(arg_datum.ty));
val = arg_datum.to_ref_llval(bcx);
}
(ty::ByCopy, _) => {
ty::ByCopy => {
if ty::type_needs_drop(bcx.tcx(), arg_datum.ty) ||
arg_datum.appropriate_mode().is_by_ref() {
debug!("by copy arg with type %s, storing to scratch",
@ -930,7 +920,7 @@ pub fn trans_arg_expr(bcx: block,
ByRef(_) => val = scratch.val,
}
} else {
debug!("by copy arg with type %s");
debug!("by copy arg with type %s", bcx.ty_to_str(arg_datum.ty));
match arg_datum.mode {
ByRef(_) => val = Load(bcx, arg_datum.val),
ByValue => val = arg_datum.val,
@ -944,10 +934,6 @@ pub fn trans_arg_expr(bcx: block,
if formal_arg_ty != arg_datum.ty {
// this could happen due to e.g. subtyping
let llformal_arg_ty = type_of::type_of_explicit_arg(ccx, &formal_arg_ty);
let llformal_arg_ty = match self_mode {
ty::ByRef => llformal_arg_ty.ptr_to(),
ty::ByCopy => llformal_arg_ty,
};
debug!("casting actual type (%s) to match formal (%s)",
bcx.val_to_str(val), bcx.llty_str(llformal_arg_ty));
val = PointerCast(bcx, val, llformal_arg_ty);

2
src/librustc/middle/trans/common.rs
View File

@ -125,7 +125,7 @@ pub type ExternMap = HashMap<@str, ValueRef>;
pub struct ValSelfData {
v: ValueRef,
t: ty::t,
is_owned: bool
is_copy: bool,
}
// Here `self_ty` is the real type of the self parameter to this method. It

14
src/librustc/middle/trans/glue.rs
View File

@ -425,12 +425,7 @@ pub fn trans_struct_drop_flag(bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast:
// just consist of the environment (self)
assert_eq!(params.len(), 1);
// Take a reference to the class (because it's using the Drop trait),
// do so now.
let llval = alloca(bcx, val_ty(v0));
Store(bcx, v0, llval);
let self_arg = PointerCast(bcx, llval, params[0]);
let self_arg = PointerCast(bcx, v0, params[0]);
let args = ~[self_arg];
Call(bcx, dtor_addr, args);
@ -465,12 +460,7 @@ pub fn trans_struct_drop(mut bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast::
// just consist of the environment (self)
assert_eq!(params.len(), 1);
// Take a reference to the class (because it's using the Drop trait),
// do so now.
let llval = alloca(bcx, val_ty(v0));
Store(bcx, v0, llval);
let self_arg = PointerCast(bcx, llval, params[0]);
let self_arg = PointerCast(bcx, v0, params[0]);
let args = ~[self_arg];
Call(bcx, dtor_addr, args);

6
src/librustc/middle/trans/inline.rs
View File

@ -12,7 +12,7 @@ use core::prelude::*;
use metadata::csearch;
use middle::astencode;
use middle::trans::base::{push_ctxt,impl_owned_self, impl_self, no_self};
use middle::trans::base::{push_ctxt, impl_self, no_self};
use middle::trans::base::{trans_item, get_item_val, trans_fn};
use middle::trans::common::*;
use middle::ty;
@ -114,8 +114,8 @@ pub fn maybe_instantiate_inline(ccx: @mut CrateContext, fn_id: ast::def_id,
debug!("calling inline trans_fn with self_ty %s",
ty_to_str(ccx.tcx, self_ty));
match mth.explicit_self.node {
ast::sty_value => impl_owned_self(self_ty),
_ => impl_self(self_ty),
ast::sty_value => impl_self(self_ty, ty::ByRef),
_ => impl_self(self_ty, ty::ByCopy),
}
}
};

90
src/librustc/middle/trans/meth.rs
View File

@ -20,6 +20,7 @@ use middle::trans::build::*;
use middle::trans::callee::*;
use middle::trans::callee;
use middle::trans::common::*;
use middle::trans::datum::*;
use middle::trans::expr::{SaveIn, Ignore};
use middle::trans::expr;
use middle::trans::glue;
@ -107,10 +108,8 @@ pub fn trans_method(ccx: @mut CrateContext,
debug!("calling trans_fn with self_ty %s",
self_ty.repr(ccx.tcx));
match method.explicit_self.node {
ast::sty_value => impl_owned_self(self_ty),
_ => {
impl_self(self_ty)
}
ast::sty_value => impl_self(self_ty, ty::ByRef),
_ => impl_self(self_ty, ty::ByCopy),
}
}
};
@ -129,28 +128,19 @@ pub fn trans_method(ccx: @mut CrateContext,
pub fn trans_self_arg(bcx: block,
base: @ast::expr,
temp_cleanups: &mut ~[ValueRef],
mentry: typeck::method_map_entry) -> Result {
let _icx = push_ctxt("impl::trans_self_arg");
let mut temp_cleanups = ~[];
// Compute the type of self.
let self_ty = monomorphize_type(bcx, mentry.self_ty);
let result = trans_arg_expr(bcx,
self_ty,
mentry.self_mode,
mentry.explicit_self,
base,
&mut temp_cleanups,
None,
DontAutorefArg);
// FIXME(#3446)---this is wrong, actually. The temp_cleanups
// should be revoked only after all arguments have been passed.
for temp_cleanups.iter().advance |c| {
revoke_clean(bcx, *c)
}
return result;
// self is passed as an opaque box in the environment slot
let self_ty = ty::mk_opaque_box(bcx.tcx());
trans_arg_expr(bcx,
self_ty,
mentry.self_mode,
base,
temp_cleanups,
None,
DontAutorefArg)
}
pub fn trans_method_callee(bcx: block,
@ -203,15 +193,16 @@ pub fn trans_method_callee(bcx: block,
match origin {
typeck::method_static(did) => {
let callee_fn = callee::trans_fn_ref(bcx, did, callee_id);
let Result {bcx, val} = trans_self_arg(bcx, this, mentry);
let mut temp_cleanups = ~[];
let Result {bcx, val} = trans_self_arg(bcx, this, &mut temp_cleanups, mentry);
Callee {
bcx: bcx,
data: Method(MethodData {
llfn: callee_fn.llfn,
llself: val,
temp_cleanup: temp_cleanups.head_opt().map(|&v| *v),
self_ty: node_id_type(bcx, this.id),
self_mode: mentry.self_mode,
explicit_self: mentry.explicit_self
})
}
}
@ -254,9 +245,8 @@ pub fn trans_method_callee(bcx: block,
store,
mentry.explicit_self)
}
typeck::method_super(*) => {
fail!("method_super should have been handled \
above")
typeck::method_super(*) => {
fail!("method_super should have been handled above")
}
}
}
@ -413,8 +403,9 @@ pub fn trans_monomorphized_callee(bcx: block,
bcx.ccx(), impl_did, mname);
// obtain the `self` value:
let mut temp_cleanups = ~[];
let Result {bcx, val: llself_val} =
trans_self_arg(bcx, base, mentry);
trans_self_arg(bcx, base, &mut temp_cleanups, mentry);
// create a concatenated set of substitutions which includes
// those from the impl and those from the method:
@ -441,9 +432,9 @@ pub fn trans_monomorphized_callee(bcx: block,
data: Method(MethodData {
llfn: llfn_val,
llself: llself_val,
temp_cleanup: temp_cleanups.head_opt().map(|&v| *v),
self_ty: node_id_type(bcx, base.id),
self_mode: mentry.self_mode,
explicit_self: mentry.explicit_self
})
}
}
@ -573,10 +564,10 @@ pub fn trans_trait_callee_from_llval(bcx: block,
// necessary:
let mut llself;
debug!("(translating trait callee) loading second index from pair");
let llbox = Load(bcx, GEPi(bcx, llpair, [0u, abi::trt_field_box]));
let llboxptr = GEPi(bcx, llpair, [0u, abi::trt_field_box]);
let llbox = Load(bcx, llboxptr);
// Munge `llself` appropriately for the type of `self` in the method.
let self_mode;
match explicit_self {
ast::sty_static => {
bcx.tcx().sess.bug("shouldn't see static method here");
@ -586,8 +577,6 @@ pub fn trans_trait_callee_from_llval(bcx: block,
called on objects");
}
ast::sty_region(*) => {
// As before, we need to pass a pointer to a pointer to the
// payload.
match store {
ty::BoxTraitStore |
ty::UniqTraitStore => {
@ -597,30 +586,18 @@ pub fn trans_trait_callee_from_llval(bcx: block,
llself = llbox;
}
}
let llscratch = alloca(bcx, val_ty(llself));
Store(bcx, llself, llscratch);
llself = llscratch;
self_mode = ty::ByRef;
}
ast::sty_box(_) => {
// Bump the reference count on the box.
debug!("(translating trait callee) callee type is `%s`",
bcx.ty_to_str(callee_ty));
bcx = glue::take_ty(bcx, llbox, callee_ty);
glue::incr_refcnt_of_boxed(bcx, llbox);
// Pass a pointer to the box.
match store {
ty::BoxTraitStore => llself = llbox,
_ => bcx.tcx().sess.bug("@self receiver with non-@Trait")
}
let llscratch = alloca(bcx, val_ty(llself));
Store(bcx, llself, llscratch);
llself = llscratch;
self_mode = ty::ByRef;
}
ast::sty_uniq => {
// Pass the unique pointer.
@ -629,14 +606,15 @@ pub fn trans_trait_callee_from_llval(bcx: block,
_ => bcx.tcx().sess.bug("~self receiver with non-~Trait")
}
let llscratch = alloca(bcx, val_ty(llself));
Store(bcx, llself, llscratch);
llself = llscratch;
self_mode = ty::ByRef;
zero_mem(bcx, llboxptr, ty::mk_opaque_box(bcx.tcx()));
}
}
llself = PointerCast(bcx, llself, Type::opaque_box(ccx).ptr_to());
let scratch = scratch_datum(bcx, ty::mk_opaque_box(bcx.tcx()), false);
Store(bcx, llself, scratch.val);
scratch.add_clean(bcx);
// Load the function from the vtable and cast it to the expected type.
debug!("(translating trait callee) loading method");
let llcallee_ty = type_of_fn_from_ty(ccx, callee_ty);
@ -650,10 +628,10 @@ pub fn trans_trait_callee_from_llval(bcx: block,
bcx: bcx,
data: Method(MethodData {
llfn: mptr,
llself: llself,
self_ty: ty::mk_opaque_box(bcx.tcx()),
self_mode: self_mode,
explicit_self: explicit_self
llself: scratch.to_value_llval(bcx),
temp_cleanup: Some(scratch.val),
self_ty: scratch.ty,
self_mode: ty::ByCopy,
/* XXX: Some(llbox) */
})
};

13
src/librustc/middle/typeck/check/method.rs
View File

@ -976,9 +976,7 @@ impl<'self> LookupContext<'self> {
let fty = ty::mk_bare_fn(tcx, ty::BareFnTy {sig: fn_sig, ..bare_fn_ty});
debug!("after replacing bound regions, fty=%s", self.ty_to_str(fty));
// FIXME(#7411): We always pass self by-ref since we stuff it in the environment slot.
// Eventually that should not be the case
let self_mode = ty::ByRef;
let self_mode = get_mode_from_explicit_self(candidate.method_ty.explicit_self);
// before we only checked whether self_ty could be a subtype
// of rcvr_ty; now we actually make it so (this may cause
@ -998,7 +996,7 @@ impl<'self> LookupContext<'self> {
self.fcx.write_ty(self.callee_id, fty);
self.fcx.write_substs(self.callee_id, all_substs);
method_map_entry {
self_ty: candidate.rcvr_ty,
self_ty: rcvr_ty,
self_mode: self_mode,
explicit_self: candidate.method_ty.explicit_self,
origin: candidate.origin,
@ -1253,3 +1251,10 @@ impl<'self> LookupContext<'self> {
self.tcx().sess.bug(s)
}
}
pub fn get_mode_from_explicit_self(explicit_self: ast::explicit_self_) -> SelfMode {
match explicit_self {
sty_value => ty::ByRef,
_ => ty::ByCopy,
}
}