Skip to content

Avoid double indirection for the "self" arg in methods #7452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 30, 2013
Merged

Avoid double indirection for the "self" arg in methods #7452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
08a5278
Add missing argument to a debug! call in trans_arg_expr
dotdash Jun 25, 2013
2a40c5d
Avoid extra casts for "self" arguments
dotdash Jun 29, 2013
ba922a4
Fix handling of temp cleanups for the "self" argument
dotdash Jun 29, 2013
765a290
Avoid double indirection for the "self" arg in methods
dotdash Jun 27, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/librustc/middle/trans/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
29 changes: 14 additions & 15 deletions src/librustc/middle/trans/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 => ()
Expand Down Expand Up @@ -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);
}
_ => {}
}
Expand Down Expand Up @@ -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
Expand Down
52 changes: 19 additions & 33 deletions src/librustc/middle/trans/callee.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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);
}
}
_ => {}
}
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -806,18 +803,16 @@ 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>,
autoref_arg: AutorefArg) -> Result {
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)));
Expand Down Expand Up @@ -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);

Expand All @@ -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",
Expand All @@ -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,
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 2 additions & 12 deletions src/librustc/middle/trans/glue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/middle/trans/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
}
}
};
Expand Down
Loading