Skip to content

Commit

Permalink
Improve usage of lifetime intrinsics in match expressions
Browse files Browse the repository at this point in the history
The allocas used in match expression currently don't get good lifetime
markers, in fact they only get lifetime start markers, because their
lifetimes don't match to cleanup scopes.

While the bindings themselves are bog standard and just need a matching
pair of start and end markers, they might need them twice, once for a
guard clause and once for the match body.

The __llmatch alloca OTOH needs a single lifetime start marker, but
when there's a guard clause, it needs two end markers, because its
lifetime ends either when the guard doesn't match or after the match
body.

With these intrinsics in place, LLVM can now, for example, optimize
code like this:

````rust
enum E {
  A1(int),
  A2(int),
  A3(int),
  A4(int),
}

pub fn variants(x: E) {
  match x {
    A1(m) => bar(&m),
    A2(m) => bar(&m),
    A3(m) => bar(&m),
    A4(m) => bar(&m),
  }
}
````

To a single call to bar, using only a single stack slot. It still fails
to eliminate some of checks.

````gas
.Ltmp5:
	.cfi_def_cfa_offset 16
	movb	(%rdi), %al
	testb	%al, %al
	je	.LBB3_5
	movzbl	%al, %eax
	cmpl	$1, %eax
	je	.LBB3_5
	cmpl	$2, %eax
.LBB3_5:
	movq	8(%rdi), %rax
	movq	%rax, (%rsp)
	leaq	(%rsp), %rdi
	callq	_ZN3bar20hcb7a0d8be8e17e37daaE@PLT
	popq	%rax
	retq
````
  • Loading branch information
dotdash committed Jul 23, 2014
1 parent b10dcbe commit 0d6f257
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 16 deletions.
27 changes: 22 additions & 5 deletions src/librustc/middle/trans/_match.rs
Expand Up @@ -892,7 +892,12 @@ fn insert_lllocals<'a>(mut bcx: &'a Block<'a>, bindings_map: &BindingsMap,
TrByCopy(llbinding) => {
let llval = Load(bcx, binding_info.llmatch);
let datum = Datum::new(llval, binding_info.ty, Lvalue);
call_lifetime_start(bcx, llbinding);
bcx = datum.store_to(bcx, llbinding);
match cs {
Some(cs) => bcx.fcx.schedule_lifetime_end(cs, llbinding),
_ => {}
}

llbinding
},
Expand All @@ -906,7 +911,10 @@ fn insert_lllocals<'a>(mut bcx: &'a Block<'a>, bindings_map: &BindingsMap,

let datum = Datum::new(llval, binding_info.ty, Lvalue);
match cs {
Some(cs) => bcx.fcx.schedule_drop_and_zero_mem(cs, llval, binding_info.ty),
Some(cs) => {
bcx.fcx.schedule_drop_and_zero_mem(cs, llval, binding_info.ty);
bcx.fcx.schedule_lifetime_end(cs, binding_info.llmatch);
}
_ => {}
}

Expand Down Expand Up @@ -945,9 +953,17 @@ fn compile_guard<'a, 'b>(
let val = unpack_datum!(bcx, expr::trans(bcx, guard_expr));
let val = val.to_llbool(bcx);

for (_, &binding_info) in data.bindings_map.iter() {
match binding_info.trmode {
TrByCopy(llbinding) => call_lifetime_end(bcx, llbinding),
_ => {}
}
}

return with_cond(bcx, Not(bcx, val), |bcx| {
// Guard does not match: remove all bindings from the lllocals table
for (_, &binding_info) in data.bindings_map.iter() {
call_lifetime_end(bcx, binding_info.llmatch);
bcx.fcx.lllocals.borrow_mut().remove(&binding_info.id);
}
match chk {
Expand Down Expand Up @@ -988,6 +1004,7 @@ fn compile_submatch<'a, 'b>(
let data = &m[0].data;
for &(ref ident, ref value_ptr) in m[0].bound_ptrs.iter() {
let llmatch = data.bindings_map.get(ident).llmatch;
call_lifetime_start(bcx, llmatch);
Store(bcx, *value_ptr, llmatch);
}
match data.arm.guard {
Expand Down Expand Up @@ -1294,24 +1311,24 @@ fn create_bindings_map(bcx: &Block, pat: Gc<ast::Pat>) -> BindingsMap {
match bm {
ast::BindByValue(_)
if !ty::type_moves_by_default(tcx, variable_ty) => {
llmatch = alloca(bcx,
llmatch = alloca_no_lifetime(bcx,
llvariable_ty.ptr_to(),
"__llmatch");
trmode = TrByCopy(alloca(bcx,
trmode = TrByCopy(alloca_no_lifetime(bcx,
llvariable_ty,
bcx.ident(ident).as_slice()));
}
ast::BindByValue(_) => {
// in this case, the final type of the variable will be T,
// but during matching we need to store a *T as explained
// above
llmatch = alloca(bcx,
llmatch = alloca_no_lifetime(bcx,
llvariable_ty.ptr_to(),
bcx.ident(ident).as_slice());
trmode = TrByMove;
}
ast::BindByRef(_) => {
llmatch = alloca(bcx,
llmatch = alloca_no_lifetime(bcx,
llvariable_ty,
bcx.ident(ident).as_slice());
trmode = TrByRef;
Expand Down
25 changes: 16 additions & 9 deletions src/librustc/middle/trans/base.rs
Expand Up @@ -1169,25 +1169,32 @@ pub fn alloc_ty(bcx: &Block, t: ty::t, name: &str) -> ValueRef {
}

pub fn alloca(cx: &Block, ty: Type, name: &str) -> ValueRef {
alloca_maybe_zeroed(cx, ty, name, false)
let p = alloca_no_lifetime(cx, ty, name);
call_lifetime_start(cx, p);
p
}

pub fn alloca_maybe_zeroed(cx: &Block, ty: Type, name: &str, zero: bool) -> ValueRef {
pub fn alloca_no_lifetime(cx: &Block, ty: Type, name: &str) -> ValueRef {
let _icx = push_ctxt("alloca");
if cx.unreachable.get() {
unsafe {
return llvm::LLVMGetUndef(ty.ptr_to().to_ref());
}
}
debuginfo::clear_source_location(cx.fcx);
let p = Alloca(cx, ty, name);
if zero {
let b = cx.fcx.ccx.builder();
b.position_before(cx.fcx.alloca_insert_pt.get().unwrap());
memzero(&b, p, ty);
} else {
call_lifetime_start(cx, p);
Alloca(cx, ty, name)
}

pub fn alloca_zeroed(cx: &Block, ty: Type, name: &str) -> ValueRef {
if cx.unreachable.get() {
unsafe {
return llvm::LLVMGetUndef(ty.ptr_to().to_ref());
}
}
let p = alloca_no_lifetime(cx, ty, name);
let b = cx.fcx.ccx.builder();
b.position_before(cx.fcx.alloca_insert_pt.get().unwrap());
memzero(&b, p, ty);
p
}

Expand Down
8 changes: 6 additions & 2 deletions src/librustc/middle/trans/datum.rs
Expand Up @@ -120,7 +120,11 @@ pub fn lvalue_scratch_datum<'a, A>(bcx: &'a Block<'a>,
*/

let llty = type_of::type_of(bcx.ccx(), ty);
let scratch = alloca_maybe_zeroed(bcx, llty, name, zero);
let scratch = if zero {
alloca_zeroed(bcx, llty, name)
} else {
alloca(bcx, llty, name)
};

// Subtle. Populate the scratch memory *before* scheduling cleanup.
let bcx = populate(arg, bcx, scratch);
Expand All @@ -145,7 +149,7 @@ pub fn rvalue_scratch_datum(bcx: &Block,
*/

let llty = type_of::type_of(bcx.ccx(), ty);
let scratch = alloca_maybe_zeroed(bcx, llty, name, false);
let scratch = alloca(bcx, llty, name);
Datum::new(scratch, ty, Rvalue::new(ByRef))
}

Expand Down

5 comments on commit 0d6f257

@bors
Copy link
Contributor

@bors bors commented on 0d6f257 Jul 24, 2014

Choose a reason for hiding this comment

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

saw approval from pcwalton
at dotdash@0d6f257

@bors
Copy link
Contributor

@bors bors commented on 0d6f257 Jul 24, 2014

Choose a reason for hiding this comment

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

merging dotdash/rust/match_lifetimes = 0d6f257 into auto

@bors
Copy link
Contributor

@bors bors commented on 0d6f257 Jul 24, 2014

Choose a reason for hiding this comment

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

dotdash/rust/match_lifetimes = 0d6f257 merged ok, testing candidate = e70ee12

@bors
Copy link
Contributor

@bors bors commented on 0d6f257 Jul 24, 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 = e70ee12

Please sign in to comment.