Skip to content

Commit

Permalink
Implement scopes independent of LLVM basic blocks
Browse files Browse the repository at this point in the history 
Currently, scopes are tied to LLVM basic blocks. For each scope, there
are two new basic blocks, which means two extra jumps in the unoptimized
IR. These blocks aren't actually required, but only used to act as the
boundary for cleanups.

By keeping track of the current scope within a single basic block, we
can avoid those extra blocks and jumps, shrinking the pre-optimization
IR quite considerably. For example, the IR for trans_intrinsic goes
from ~22k lines to ~16k lines, almost 30% less.

The impact on the build times of optimized builds is rather small (about
1%), but unoptimized builds are about 11% faster. The testsuite for
unoptimized builds runs between 15% (CPU time) and 7.5% (wallclock time on
my i7) faster.

Also, in some situations this helps LLVM to generate better code by
inlining functions that it previously considered to be too large.
Likely because of the pointless blocks/jumps that were still present at
the time the inlining pass runs.

Refs rust-lang#7462
  • Loading branch information
@dotdash
dotdash committed Jul 7, 2013
1 parent 6595c42 commit e41e435
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 161 deletions.
196 changes: 109 additions & 87 deletions src/librustc/middle/trans/base.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -863,10 +863,10 @@ pub fn need_invoke(bcx: block) -> bool {

// Walk the scopes to look for cleanups
let mut cur = bcx;
let mut cur_scope = cur.scope;
loop {
match cur.kind {
block_scope(inf) => {
let inf = &mut *inf; // FIXME(#5074) workaround old borrowck
cur_scope = match cur_scope {
Some(inf) => {
for inf.cleanups.iter().advance |cleanup| {
match *cleanup {
clean(_, cleanup_type) | clean_temp(_, _, cleanup_type) => {
Expand All @@ -876,12 +876,15 @@ pub fn need_invoke(bcx: block) -> bool {
}
}
}
inf.parent
}
None => {
cur = match cur.parent {
Some(next) => next,
None => return false
};
cur.scope
}
_ => ()
}
cur = match cur.parent {
Some(next) => next,
None => return false
}
}
}
Expand All @@ -899,23 +902,21 @@ pub fn have_cached_lpad(bcx: block) -> bool {

pub fn in_lpad_scope_cx(bcx: block, f: &fn(si: &mut scope_info)) {
let mut bcx = bcx;
let mut cur_scope = bcx.scope;
loop {
{
match bcx.kind {
block_scope(inf) => {
let len = { // FIXME(#5074) workaround old borrowck
let inf = &mut *inf;
inf.cleanups.len()
};
if len > 0u || bcx.parent.is_none() {
f(inf);
return;
}
cur_scope = match cur_scope {
Some(inf) => {
if !inf.empty_cleanups() || (inf.parent.is_none() && bcx.parent.is_none()) {
f(inf);
return;
}
_ => ()
inf.parent
}
None => {
bcx = block_parent(bcx);
bcx.scope
}
}
bcx = block_parent(bcx);
}
}

Expand Down Expand Up @@ -972,27 +973,31 @@ pub fn get_landing_pad(bcx: block) -> BasicBlockRef {

pub fn find_bcx_for_scope(bcx: block, scope_id: ast::node_id) -> block {
let mut bcx_sid = bcx;
let mut cur_scope = bcx_sid.scope;
loop {
bcx_sid = match bcx_sid.node_info {
Some(NodeInfo { id, _ }) if id == scope_id => {
return bcx_sid
}

// FIXME(#6268, #6248) hacky cleanup for nested method calls
Some(NodeInfo { callee_id: Some(id), _ }) if id == scope_id => {
return bcx_sid
}

_ => {
match bcx_sid.parent {
None => bcx.tcx().sess.bug(
fmt!("no enclosing scope with id %d", scope_id)),
Some(bcx_par) => bcx_par
cur_scope = match cur_scope {
Some(inf) => {
match inf.node_info {
Some(NodeInfo { id, _ }) if id == scope_id => {
return bcx_sid
}
// FIXME(#6268, #6248) hacky cleanup for nested method calls
Some(NodeInfo { callee_id: Some(id), _ }) if id == scope_id => {
return bcx_sid
}
_ => inf.parent
}
}
None => {
bcx_sid = match bcx_sid.parent {
None => bcx.tcx().sess.bug(fmt!("no enclosing scope with id %d", scope_id)),
Some(bcx_par) => bcx_par
};
bcx_sid.scope
}
}
}
}


pub fn do_spill(bcx: block, v: ValueRef, t: ty::t) -> ValueRef {
Expand Down Expand Up @@ -1145,7 +1150,7 @@ pub fn trans_stmt(cx: block, s: &ast::stmt) -> block {

// You probably don't want to use this one. See the
// next three functions instead.
pub fn new_block(cx: fn_ctxt, parent: Option<block>, kind: block_kind,
pub fn new_block(cx: fn_ctxt, parent: Option<block>, scope: Option<@mut scope_info>,
is_lpad: bool, name: &str, opt_node_info: Option<NodeInfo>)
-> block {

Expand All @@ -1155,10 +1160,10 @@ pub fn new_block(cx: fn_ctxt, parent: Option<block>, kind: block_kind,
};
let bcx = mk_block(llbb,
parent,
kind,
is_lpad,
opt_node_info,
cx);
bcx.scope = scope;
for parent.iter().advance |cx| {
if cx.unreachable {
Unreachable(bcx);
Expand All @@ -1169,27 +1174,30 @@ pub fn new_block(cx: fn_ctxt, parent: Option<block>, kind: block_kind,
}
}

pub fn simple_block_scope() -> block_kind {
block_scope(@mut scope_info {
pub fn simple_block_scope(parent: Option<@mut scope_info>,
node_info: Option<NodeInfo>) -> @mut scope_info {
@mut scope_info {
parent: parent,
loop_break: None,
loop_label: None,
cleanups: ~[],
cleanup_paths: ~[],
landing_pad: None
})
landing_pad: None,
node_info: node_info,
}
}

// Use this when you're at the top block of a function or the like.
pub fn top_scope_block(fcx: fn_ctxt, opt_node_info: Option<NodeInfo>)
-> block {
return new_block(fcx, None, simple_block_scope(), false,
return new_block(fcx, None, Some(simple_block_scope(None, opt_node_info)), false,
"function top level", opt_node_info);
}

pub fn scope_block(bcx: block,
opt_node_info: Option<NodeInfo>,
n: &str) -> block {
return new_block(bcx.fcx, Some(bcx), simple_block_scope(), bcx.is_lpad,
return new_block(bcx.fcx, Some(bcx), Some(simple_block_scope(None, opt_node_info)), bcx.is_lpad,
n, opt_node_info);
}

Expand All @@ -1198,27 +1206,29 @@ pub fn loop_scope_block(bcx: block,
loop_label: Option<ident>,
n: &str,
opt_node_info: Option<NodeInfo>) -> block {
return new_block(bcx.fcx, Some(bcx), block_scope(@mut scope_info {
return new_block(bcx.fcx, Some(bcx), Some(@mut scope_info {
parent: None,
loop_break: Some(loop_break),
loop_label: loop_label,
cleanups: ~[],
cleanup_paths: ~[],
landing_pad: None
landing_pad: None,
node_info: opt_node_info,
}), bcx.is_lpad, n, opt_node_info);
}

// Use this when creating a block for the inside of a landing pad.
pub fn lpad_block(bcx: block, n: &str) -> block {
new_block(bcx.fcx, Some(bcx), block_non_scope, true, n, None)
new_block(bcx.fcx, Some(bcx), None, true, n, None)
}

// Use this when you're making a general CFG BB within a scope.
pub fn sub_block(bcx: block, n: &str) -> block {
new_block(bcx.fcx, Some(bcx), block_non_scope, bcx.is_lpad, n, None)
new_block(bcx.fcx, Some(bcx), None, bcx.is_lpad, n, None)
}

pub fn raw_block(fcx: fn_ctxt, is_lpad: bool, llbb: BasicBlockRef) -> block {
mk_block(llbb, None, block_non_scope, is_lpad, None, fcx)
mk_block(llbb, None, is_lpad, None, fcx)
}


Expand Down Expand Up @@ -1277,42 +1287,47 @@ pub fn cleanup_and_leave(bcx: block,
(fmt!("cleanup_and_leave(%s)", cur.to_str())).to_managed());
}

match cur.kind {
block_scope(inf) if !inf.empty_cleanups() => {
let (sub_cx, dest, inf_cleanups) = {
let inf = &mut *inf;
let mut skip = 0;
let mut dest = None;
{
let r = (*inf).cleanup_paths.rev_iter().find_(|cp| cp.target == leave);
for r.iter().advance |cp| {
if cp.size == inf.cleanups.len() {
Br(bcx, cp.dest);
return;
let mut cur_scope = cur.scope;
loop {
cur_scope = match cur_scope {
Some (inf) if !inf.empty_cleanups() => {
let (sub_cx, dest, inf_cleanups) = {
let inf = &mut *inf;
let mut skip = 0;
let mut dest = None;
{
let r = (*inf).cleanup_paths.rev_iter().find_(|cp| cp.target == leave);
for r.iter().advance |cp| {
if cp.size == inf.cleanups.len() {
Br(bcx, cp.dest);
return;
}

skip = cp.size;
dest = Some(cp.dest);
}

skip = cp.size;
dest = Some(cp.dest);
}
let sub_cx = sub_block(bcx, "cleanup");
Br(bcx, sub_cx.llbb);
inf.cleanup_paths.push(cleanup_path {
target: leave,
size: inf.cleanups.len(),
dest: sub_cx.llbb
});
(sub_cx, dest, inf.cleanups.tailn(skip).to_owned())
};
bcx = trans_block_cleanups_(sub_cx,
inf_cleanups,
is_lpad);
for dest.iter().advance |&dest| {
Br(bcx, dest);
return;
}
let sub_cx = sub_block(bcx, "cleanup");
Br(bcx, sub_cx.llbb);
inf.cleanup_paths.push(cleanup_path {
target: leave,
size: inf.cleanups.len(),
dest: sub_cx.llbb
});
(sub_cx, dest, inf.cleanups.tailn(skip).to_owned())
};
bcx = trans_block_cleanups_(sub_cx,
inf_cleanups,
is_lpad);
for dest.iter().advance |&dest| {
Br(bcx, dest);
return;
inf.parent
}
Some(inf) => inf.parent,
None => break
}
_ => ()
}

match upto {
Expand Down Expand Up @@ -1353,20 +1368,27 @@ pub fn with_scope(bcx: block,
bcx.to_str(), opt_node_info, name);
let _indenter = indenter();

let scope_cx = scope_block(bcx, opt_node_info, name);
Br(bcx, scope_cx.llbb);
leave_block(f(scope_cx), scope_cx)
let scope = simple_block_scope(bcx.scope, opt_node_info);
bcx.scope = Some(scope);
let ret = f(bcx);
let ret = trans_block_cleanups_(ret, /*bad*/copy scope.cleanups, false);
bcx.scope = scope.parent;
ret
}

pub fn with_scope_result(bcx: block,
opt_node_info: Option<NodeInfo>,
name: &str,
f: &fn(block) -> Result) -> Result {
let _icx = push_ctxt("with_scope_result");
let scope_cx = scope_block(bcx, opt_node_info, name);
Br(bcx, scope_cx.llbb);
let Result {bcx, val} = f(scope_cx);
rslt(leave_block(bcx, scope_cx), val)

let scope = simple_block_scope(bcx.scope, opt_node_info);
bcx.scope = Some(scope);
let Result { bcx: out_bcx, val } = f(bcx);
let out_bcx = trans_block_cleanups_(out_bcx, /*bad*/copy scope.cleanups, false);
bcx.scope = scope.parent;

rslt(out_bcx, val)
}

pub fn with_scope_datumblock(bcx: block, opt_node_info: Option<NodeInfo>,
Expand Down
Loading

1 comment on commit e41e435

@graydon
Copy link

@graydon graydon commented on e41e435 Jul 7, 2013

Choose a reason for hiding this comment

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

r+ p=999 thank you!

Please sign in to comment.