Skip to content
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.

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

avoid redundant translation of items during monomorphization #16059

Merged
merged 1 commit into from
Jul 31, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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>();
}
Copy link
Member

Choose a reason for hiding this comment

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

This will probably need a license to get past make tidy