Skip to content

Commit 94a56a3

Browse files
committed
librustc: Don't create extra alloca slot for by value bindings in match.
1 parent 67776ba commit 94a56a3

File tree

2 files changed

+56
-116
lines changed

2 files changed

+56
-116
lines changed

src/librustc/middle/trans/_match.rs

Lines changed: 34 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,8 @@
6464
* We store information about the bound variables for each arm as part of the
6565
* per-arm `ArmData` struct. There is a mapping from identifiers to
6666
* `BindingInfo` structs. These structs contain the mode/id/type of the
67-
* binding, but they also contain up to two LLVM values, called `llmatch` and
68-
* `llbinding` respectively (the `llbinding`, as will be described shortly, is
69-
* optional and only present for by-value bindings---therefore it is bundled
70-
* up as part of the `TransBindingMode` type). Both point at allocas.
67+
* binding, but they also contain an LLVM value which points at an alloca
68+
* called `llmatch`.
7169
*
7270
* The `llmatch` binding always stores a pointer into the value being matched
7371
* which points at the data for the binding. If the value being matched has
@@ -83,32 +81,26 @@
8381
* up against an identifier, we store the current pointer into the
8482
* corresponding alloca.
8583
*
86-
* In addition, for each by-value binding (copy or move), we will create a
87-
* second alloca (`llbinding`) that will hold the final value. In this
88-
* example, that means that `d` would have this second alloca of type `D` (and
89-
* hence `llbinding` has type `D*`).
90-
*
9184
* Once a pattern is completely matched, and assuming that there is no guard
9285
* pattern, we will branch to a block that leads to the body itself. For any
9386
* by-value bindings, this block will first load the ptr from `llmatch` (the
94-
* one of type `D*`) and copy/move the value into `llbinding` (the one of type
95-
* `D`). The second alloca then becomes the value of the local variable. For
96-
* by ref bindings, the value of the local variable is simply the first
97-
* alloca.
87+
* one of type `D*`) and then load a second time to get the actual value (the
88+
* one of type `D`). For by ref bindings, the value of the local variable is
89+
* simply the first alloca.
9890
*
9991
* So, for the example above, we would generate a setup kind of like this:
10092
*
10193
* +-------+
10294
* | Entry |
10395
* +-------+
10496
* |
105-
* +-------------------------------------------+
106-
* | llmatch_c = (addr of first half of tuple) |
107-
* | llmatch_d = (addr of first half of tuple) |
108-
* +-------------------------------------------+
97+
* +--------------------------------------------+
98+
* | llmatch_c = (addr of first half of tuple) |
99+
* | llmatch_d = (addr of second half of tuple) |
100+
* +--------------------------------------------+
109101
* |
110102
* +--------------------------------------+
111-
* | *llbinding_d = **llmatch_dlbinding_d |
103+
* | *llbinding_d = **llmatch_d |
112104
* +--------------------------------------+
113105
*
114106
* If there is a guard, the situation is slightly different, because we must
@@ -127,22 +119,20 @@
127119
* +-------------------------------------------+
128120
* |
129121
* +-------------------------------------------------+
130-
* | *llbinding_d = **llmatch_dlbinding_d |
122+
* | *llbinding_d = **llmatch_d |
131123
* | check condition |
132-
* | if false { free *llbinding_d, goto next case } |
124+
* | if false { goto next case } |
133125
* | if true { goto body } |
134126
* +-------------------------------------------------+
135127
*
136128
* The handling for the cleanups is a bit... sensitive. Basically, the body
137129
* is the one that invokes `add_clean()` for each binding. During the guard
138130
* evaluation, we add temporary cleanups and revoke them after the guard is
139-
* evaluated (it could fail, after all). Presuming the guard fails, we drop
140-
* the various values we copied explicitly. Note that guards and moves are
131+
* evaluated (it could fail, after all). Note that guards and moves are
141132
* just plain incompatible.
142133
*
143134
* Some relevant helper functions that manage bindings:
144135
* - `create_bindings_map()`
145-
* - `store_non_ref_bindings()`
146136
* - `insert_lllocals()`
147137
*
148138
*
@@ -215,7 +205,6 @@ use middle::trans::datum;
215205
use middle::trans::datum::*;
216206
use middle::trans::expr::Dest;
217207
use middle::trans::expr;
218-
use middle::trans::glue;
219208
use middle::trans::tvec;
220209
use middle::trans::type_of;
221210
use middle::trans::debuginfo;
@@ -362,8 +351,8 @@ fn variant_opt(bcx: &Block, pat_id: ast::NodeId) -> Opt {
362351
}
363352

364353
#[deriving(Clone)]
365-
enum TransBindingMode {
366-
TrByValue(/*llbinding:*/ ValueRef),
354+
pub enum TransBindingMode {
355+
TrByValue,
367356
TrByRef,
368357
}
369358

@@ -376,12 +365,12 @@ enum TransBindingMode {
376365
* - `id` is the node id of the binding
377366
* - `ty` is the Rust type of the binding */
378367
#[deriving(Clone)]
379-
struct BindingInfo {
380-
llmatch: ValueRef,
381-
trmode: TransBindingMode,
382-
id: ast::NodeId,
383-
span: Span,
384-
ty: ty::t,
368+
pub struct BindingInfo {
369+
pub llmatch: ValueRef,
370+
pub trmode: TransBindingMode,
371+
pub id: ast::NodeId,
372+
pub span: Span,
373+
pub ty: ty::t,
385374
}
386375

387376
type BindingsMap = HashMap<Ident, BindingInfo>;
@@ -1260,41 +1249,6 @@ fn compare_values<'a>(
12601249
}
12611250
}
12621251

1263-
fn store_non_ref_bindings<'a>(
1264-
bcx: &'a Block<'a>,
1265-
bindings_map: &BindingsMap,
1266-
opt_cleanup_scope: Option<cleanup::ScopeId>)
1267-
-> &'a Block<'a>
1268-
{
1269-
/*!
1270-
* For each copy/move binding, copy the value from the value being
1271-
* matched into its final home. This code executes once one of
1272-
* the patterns for a given arm has completely matched. It adds
1273-
* cleanups to the `opt_cleanup_scope`, if one is provided.
1274-
*/
1275-
1276-
let fcx = bcx.fcx;
1277-
let mut bcx = bcx;
1278-
for (_, &binding_info) in bindings_map.iter() {
1279-
match binding_info.trmode {
1280-
TrByValue(lldest) => {
1281-
let llval = Load(bcx, binding_info.llmatch); // get a T*
1282-
let datum = Datum::new(llval, binding_info.ty, Lvalue);
1283-
bcx = datum.store_to(bcx, lldest);
1284-
1285-
match opt_cleanup_scope {
1286-
None => {}
1287-
Some(s) => {
1288-
fcx.schedule_drop_mem(s, lldest, binding_info.ty);
1289-
}
1290-
}
1291-
}
1292-
TrByRef => {}
1293-
}
1294-
}
1295-
return bcx;
1296-
}
1297-
12981252
fn insert_lllocals<'a>(bcx: &'a Block<'a>,
12991253
bindings_map: &BindingsMap,
13001254
cleanup_scope: cleanup::ScopeId)
@@ -1308,9 +1262,8 @@ fn insert_lllocals<'a>(bcx: &'a Block<'a>,
13081262

13091263
for (&ident, &binding_info) in bindings_map.iter() {
13101264
let llval = match binding_info.trmode {
1311-
// By value bindings: use the stack slot that we
1312-
// copied/moved the value into
1313-
TrByValue(lldest) => lldest,
1265+
// By value bindings: load from the ptr into the matched value
1266+
TrByValue => Load(bcx, binding_info.llmatch),
13141267

13151268
// By ref binding: use the ptr into the matched value
13161269
TrByRef => binding_info.llmatch
@@ -1327,9 +1280,7 @@ fn insert_lllocals<'a>(bcx: &'a Block<'a>,
13271280
if bcx.sess().opts.debuginfo == FullDebugInfo {
13281281
debuginfo::create_match_binding_metadata(bcx,
13291282
ident,
1330-
binding_info.id,
1331-
binding_info.span,
1332-
datum);
1283+
binding_info);
13331284
}
13341285
}
13351286
bcx
@@ -1355,11 +1306,8 @@ fn compile_guard<'a, 'b>(
13551306
// scope for any non-ref bindings we create.
13561307
let temp_scope = bcx.fcx.push_custom_cleanup_scope();
13571308

1358-
let mut bcx = bcx;
1359-
bcx = store_non_ref_bindings(bcx, &data.bindings_map,
1360-
Some(cleanup::CustomScope(temp_scope)));
1361-
bcx = insert_lllocals(bcx, &data.bindings_map,
1362-
cleanup::CustomScope(temp_scope));
1309+
let mut bcx = insert_lllocals(bcx, &data.bindings_map,
1310+
cleanup::CustomScope(temp_scope));
13631311

13641312
let val = unpack_datum!(bcx, expr::trans(bcx, guard_expr));
13651313
let val = val.to_llbool(bcx);
@@ -1370,9 +1318,10 @@ fn compile_guard<'a, 'b>(
13701318
bcx.fcx.pop_custom_cleanup_scope(temp_scope);
13711319

13721320
return with_cond(bcx, Not(bcx, val), |bcx| {
1373-
// Guard does not match: free the values we copied,
1374-
// and remove all bindings from the lllocals table
1375-
let bcx = drop_bindings(bcx, data);
1321+
// Guard does not match: remove all bindings from the lllocals table
1322+
for (_, &binding_info) in data.bindings_map.iter() {
1323+
bcx.fcx.lllocals.borrow_mut().remove(&binding_info.id);
1324+
}
13761325
match chk {
13771326
// If the default arm is the only one left, move on to the next
13781327
// condition explicitly rather than (possibly) falling back to
@@ -1386,21 +1335,6 @@ fn compile_guard<'a, 'b>(
13861335
};
13871336
bcx
13881337
});
1389-
1390-
fn drop_bindings<'a>(bcx: &'a Block<'a>, data: &ArmData)
1391-
-> &'a Block<'a> {
1392-
let mut bcx = bcx;
1393-
for (_, &binding_info) in data.bindings_map.iter() {
1394-
match binding_info.trmode {
1395-
TrByValue(llval) => {
1396-
bcx = glue::drop_ty(bcx, llval, binding_info.ty);
1397-
}
1398-
TrByRef => {}
1399-
}
1400-
bcx.fcx.lllocals.borrow_mut().remove(&binding_info.id);
1401-
}
1402-
return bcx;
1403-
}
14041338
}
14051339

14061340
fn compile_submatch<'a, 'b>(
@@ -1836,10 +1770,10 @@ fn create_bindings_map(bcx: &Block, pat: Gc<ast::Pat>) -> BindingsMap {
18361770
// in this case, the final type of the variable will be T,
18371771
// but during matching we need to store a *T as explained
18381772
// above
1839-
llmatch = alloca(bcx, llvariable_ty.ptr_to(), "__llmatch");
1840-
trmode = TrByValue(alloca(bcx,
1841-
llvariable_ty,
1842-
bcx.ident(ident).as_slice()));
1773+
llmatch = alloca(bcx,
1774+
llvariable_ty.ptr_to(),
1775+
bcx.ident(ident).as_slice());
1776+
trmode = TrByValue;
18431777
}
18441778
ast::BindByRef(_) => {
18451779
llmatch = alloca(bcx,
@@ -1925,14 +1859,6 @@ fn trans_match_inner<'a>(scope_cx: &'a Block<'a>,
19251859
for arm_data in arm_datas.iter() {
19261860
let mut bcx = arm_data.bodycx;
19271861

1928-
// If this arm has a guard, then the various by-value bindings have
1929-
// already been copied into their homes. If not, we do it here. This
1930-
// is just to reduce code space. See extensive comment at the start
1931-
// of the file for more details.
1932-
if arm_data.arm.guard.is_none() {
1933-
bcx = store_non_ref_bindings(bcx, &arm_data.bindings_map, None);
1934-
}
1935-
19361862
// insert bindings into the lllocals map and add cleanups
19371863
let cleanup_scope = fcx.push_custom_cleanup_scope();
19381864
bcx = insert_lllocals(bcx, &arm_data.bindings_map,

src/librustc/middle/trans/debuginfo.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,8 @@ use metadata::csearch;
187187
use middle::subst;
188188
use middle::trans::adt;
189189
use middle::trans::common::*;
190-
use middle::trans::datum::{Datum, Lvalue};
191190
use middle::trans::machine;
191+
use middle::trans::_match::{BindingInfo, TrByValue, TrByRef};
192192
use middle::trans::type_of;
193193
use middle::trans::type_::Type;
194194
use middle::trans;
@@ -938,22 +938,36 @@ pub fn create_captured_var_metadata(bcx: &Block,
938938
/// Adds the created metadata nodes directly to the crate's IR.
939939
pub fn create_match_binding_metadata(bcx: &Block,
940940
variable_ident: ast::Ident,
941-
node_id: ast::NodeId,
942-
span: Span,
943-
datum: Datum<Lvalue>) {
941+
binding: BindingInfo) {
944942
if fn_should_be_ignored(bcx.fcx) {
945943
return;
946944
}
947945

948-
let scope_metadata = scope_metadata(bcx.fcx, node_id, span);
946+
let scope_metadata = scope_metadata(bcx.fcx, binding.id, binding.span);
947+
let aops = unsafe {
948+
[llvm::LLVMDIBuilderCreateOpDeref(bcx.ccx().int_type.to_ref())]
949+
};
950+
// Regardless of the actual type (`T`) we're always passed the stack slot (alloca)
951+
// for the binding. For ByRef bindings that's a `T*` but for ByValue bindings we
952+
// actually have `T**`. So to get the actual variable we need to dereference once
953+
// more.
954+
let var_type = match binding.trmode {
955+
TrByValue => IndirectVariable {
956+
alloca: binding.llmatch,
957+
address_operations: aops
958+
},
959+
TrByRef => DirectVariable {
960+
alloca: binding.llmatch
961+
}
962+
};
949963

950964
declare_local(bcx,
951965
variable_ident,
952-
datum.ty,
966+
binding.ty,
953967
scope_metadata,
954-
DirectVariable { alloca: datum.val },
968+
var_type,
955969
LocalVariable,
956-
span);
970+
binding.span);
957971
}
958972

959973
/// Creates debug information for the given function argument.

0 commit comments

Comments
 (0)