Skip to content

Commit

Permalink
avoid redundant translation of items during monomorphization
Browse files Browse the repository at this point in the history
  • Loading branch information
spernsteiner committed Jul 30, 2014
1 parent 3ab170f commit f97f65f
Show file tree
Hide file tree
Showing 13 changed files with 81 additions and 21 deletions.
23 changes: 15 additions & 8 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,8 @@ pub fn new_fn_ctxt<'a>(ccx: &'a CrateContext,
output_type: ty::t,
param_substs: &'a param_substs,
sp: Option<Span>,
block_arena: &'a TypedArena<Block<'a>>)
block_arena: &'a TypedArena<Block<'a>>,
handle_items: HandleItemsFlag)
-> FunctionContext<'a> {
param_substs.validate();

Expand Down Expand Up @@ -1268,7 +1269,8 @@ pub fn new_fn_ctxt<'a>(ccx: &'a CrateContext,
block_arena: block_arena,
ccx: ccx,
debug_context: debug_context,
scopes: RefCell::new(Vec::new())
scopes: RefCell::new(Vec::new()),
handle_items: handle_items,
};

if has_env {
Expand Down Expand Up @@ -1579,7 +1581,8 @@ pub fn trans_closure(ccx: &CrateContext,
abi: Abi,
has_env: bool,
is_unboxed_closure: IsUnboxedClosureFlag,
maybe_load_env: <'a> |&'a Block<'a>| -> &'a Block<'a>) {
maybe_load_env: <'a> |&'a Block<'a>| -> &'a Block<'a>,
handle_items: HandleItemsFlag) {
ccx.stats.n_closures.set(ccx.stats.n_closures.get() + 1);

let _icx = push_ctxt("trans_closure");
Expand All @@ -1596,7 +1599,8 @@ pub fn trans_closure(ccx: &CrateContext,
output_type,
param_substs,
Some(body.span),
&arena);
&arena,
handle_items);
let mut bcx = init_function(&fcx, false, output_type);

// cleanup scope for the incoming arguments
Expand Down Expand Up @@ -1698,7 +1702,8 @@ pub fn trans_fn(ccx: &CrateContext,
llfndecl: ValueRef,
param_substs: &param_substs,
id: ast::NodeId,
attrs: &[ast::Attribute]) {
attrs: &[ast::Attribute],
handle_items: HandleItemsFlag) {
let _s = StatRecorder::new(ccx, ccx.tcx.map.path_to_string(id).to_string());
debug!("trans_fn(param_substs={})", param_substs.repr(ccx.tcx()));
let _icx = push_ctxt("trans_fn");
Expand All @@ -1718,7 +1723,8 @@ pub fn trans_fn(ccx: &CrateContext,
abi,
false,
NotUnboxedClosure,
|bcx| bcx);
|bcx| bcx,
handle_items);
}

pub fn trans_enum_variant(ccx: &CrateContext,
Expand Down Expand Up @@ -1824,7 +1830,7 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext,

let arena = TypedArena::new();
let fcx = new_fn_ctxt(ccx, llfndecl, ctor_id, false, result_ty,
param_substs, None, &arena);
param_substs, None, &arena, TranslateItems);
let bcx = init_function(&fcx, false, result_ty);

let arg_tys = ty::ty_fn_args(ctor_ty);
Expand Down Expand Up @@ -1925,7 +1931,8 @@ pub fn trans_item(ccx: &CrateContext, item: &ast::Item) {
llfn,
&param_substs::empty(),
item.id,
item.attrs.as_slice());
item.attrs.as_slice(),
TranslateItems);
} else {
// Be sure to travel more than just one layer deep to catch nested
// items in blocks and such.
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/middle/trans/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,8 @@ pub fn trans_unboxing_shim(bcx: &Block,
return_type,
&empty_param_substs,
None,
&block_arena);
&block_arena,
TranslateItems);
let mut bcx = init_function(&fcx, false, return_type);

// Create the substituted versions of the self type.
Expand Down
8 changes: 5 additions & 3 deletions src/librustc/middle/trans/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ pub fn trans_expr_fn<'a>(
ty::ty_fn_abi(fty),
true,
NotUnboxedClosure,
|bcx| load_environment(bcx, cdata_ty, &freevars, store));
|bcx| load_environment(bcx, cdata_ty, &freevars, store),
bcx.fcx.handle_items);
fill_fn_pair(bcx, dest_addr, llfn, llbox);
bcx
}
Expand Down Expand Up @@ -486,7 +487,8 @@ pub fn trans_unboxed_closure<'a>(
ty::ty_fn_abi(function_type),
true,
IsUnboxedClosure,
|bcx| load_unboxed_closure_environment(bcx, freevars_ptr));
|bcx| load_unboxed_closure_environment(bcx, freevars_ptr),
bcx.fcx.handle_items);

// Don't hoist this to the top of the function. It's perfectly legitimate
// to have a zero-size unboxed closure (in which case dest will be
Expand Down Expand Up @@ -573,7 +575,7 @@ pub fn get_wrapper_for_bare_fn(ccx: &CrateContext,
let arena = TypedArena::new();
let empty_param_substs = param_substs::empty();
let fcx = new_fn_ctxt(ccx, llfn, -1, true, f.sig.output,
&empty_param_substs, None, &arena);
&empty_param_substs, None, &arena, TranslateItems);
let bcx = init_function(&fcx, true, f.sig.output);

let args = create_datums_for_fn_args(&fcx,
Expand Down
9 changes: 9 additions & 0 deletions src/librustc/middle/trans/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ impl<T:Subst+Clone> SubstP for T {
pub type RvalueDatum = datum::Datum<datum::Rvalue>;
pub type LvalueDatum = datum::Datum<datum::Lvalue>;

#[deriving(Clone, Eq, PartialEq)]
pub enum HandleItemsFlag {
IgnoreItems,
TranslateItems,
}

// Function context. Every LLVM function we create will have one of
// these.
pub struct FunctionContext<'a> {
Expand Down Expand Up @@ -289,6 +295,9 @@ pub struct FunctionContext<'a> {

// Cleanup scopes.
pub scopes: RefCell<Vec<cleanup::CleanupScope<'a>> >,

// How to handle items encountered during translation of this function.
pub handle_items: HandleItemsFlag,
}

impl<'a> FunctionContext<'a> {
Expand Down
7 changes: 6 additions & 1 deletion src/librustc/middle/trans/controlflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,12 @@ pub fn trans_stmt<'a>(cx: &'a Block<'a>,
debuginfo::create_local_var_metadata(bcx, &**local);
}
}
ast::DeclItem(ref i) => trans_item(cx.fcx.ccx, &**i)
ast::DeclItem(ref i) => {
match fcx.handle_items {
TranslateItems => trans_item(cx.fcx.ccx, &**i),
IgnoreItems => {}
}
}
}
}
ast::StmtMac(..) => cx.tcx().sess.bug("unexpanded macro")
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/middle/trans/foreign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,8 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: &CrateContext,

let llfn = base::decl_internal_rust_fn(ccx, t, ps.as_slice());
base::set_llvm_fn_attrs(attrs, llfn);
base::trans_fn(ccx, decl, body, llfn, &param_substs::empty(), id, []);
base::trans_fn(ccx, decl, body, llfn, &param_substs::empty(), id, [],
TranslateItems);
llfn
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ fn make_generic_glue(ccx: &CrateContext,
let arena = TypedArena::new();
let empty_param_substs = param_substs::empty();
let fcx = new_fn_ctxt(ccx, llfn, -1, false, ty::mk_nil(),
&empty_param_substs, None, &arena);
&empty_param_substs, None, &arena, TranslateItems);

let bcx = init_function(&fcx, false, ty::mk_nil());

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ pub fn maybe_instantiate_inline(ccx: &CrateContext, fn_id: ast::DefId)
if unparameterized {
let llfn = get_item_val(ccx, mth.id);
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), llfn,
&param_substs::empty(), mth.id, []);
&param_substs::empty(), mth.id, [], TranslateItems);
}
local_def(mth.id)
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/middle/trans/meth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ pub fn trans_impl(ccx: &CrateContext,
llfn,
&param_substs::empty(),
method.id,
[]);
[],
TranslateItems);
} else {
let mut v = TransItemVisitor{ ccx: ccx };
visit::walk_method_helper(&mut v, &**method, ());
Expand Down
8 changes: 5 additions & 3 deletions src/librustc/middle/trans/monomorphize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ pub fn monomorphic_fn(ccx: &CrateContext,
} => {
let d = mk_lldecl();
set_llvm_fn_attrs(i.attrs.as_slice(), d);
trans_fn(ccx, &**decl, &**body, d, &psubsts, fn_id.node, []);
trans_fn(ccx, &**decl, &**body, d, &psubsts, fn_id.node, [],
IgnoreItems);
d
}
_ => {
Expand Down Expand Up @@ -181,7 +182,8 @@ pub fn monomorphic_fn(ccx: &CrateContext,
ast_map::NodeMethod(mth) => {
let d = mk_lldecl();
set_llvm_fn_attrs(mth.attrs.as_slice(), d);
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), d, &psubsts, mth.id, []);
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), d, &psubsts, mth.id, [],
IgnoreItems);
d
}
ast_map::NodeTraitMethod(method) => {
Expand All @@ -190,7 +192,7 @@ pub fn monomorphic_fn(ccx: &CrateContext,
let d = mk_lldecl();
set_llvm_fn_attrs(mth.attrs.as_slice(), d);
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), d,
&psubsts, mth.id, []);
&psubsts, mth.id, [], IgnoreItems);
d
}
_ => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ impl<'a, 'b> Reflector<'a, 'b> {
let empty_param_substs = param_substs::empty();
let fcx = new_fn_ctxt(ccx, llfdecl, -1, false,
ty::mk_u64(), &empty_param_substs,
None, &arena);
None, &arena, TranslateItems);
let bcx = init_function(&fcx, false, ty::mk_u64());

// we know the return type of llfdecl is an int here, so
Expand Down
11 changes: 11 additions & 0 deletions src/test/run-make/issue-7349/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-include ../tools.mk

# Test to make sure that inner functions within a polymorphic outer function
# don't get re-translated when the outer function is monomorphized. The test
# code monomorphizes the outer function several times, but the magic constant
# `8675309` used in the inner function should appear only once in the generated
# IR.

all:
$(RUSTC) foo.rs --emit=ir
[ "$$(grep -c 8675309 "$(TMPDIR)/foo.ll")" -eq "1" ]
21 changes: 21 additions & 0 deletions src/test/run-make/issue-7349/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn outer<T>() {
#[allow(dead_code)]
fn inner() -> uint {
8675309
}
}

fn main() {
outer::<int>();
outer::<uint>();
}

5 comments on commit f97f65f

@bors
Copy link
Contributor

@bors bors commented on f97f65f Jul 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on f97f65f Jul 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging epdtry/rust/mono-item-dedup = f97f65f into auto

@bors
Copy link
Contributor

@bors bors commented on f97f65f Jul 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

epdtry/rust/mono-item-dedup = f97f65f merged ok, testing candidate = 7c28dd0

@bors
Copy link
Contributor

@bors bors commented on f97f65f Jul 31, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 7c28dd0

Please sign in to comment.