Skip to content

Commit 009a649

Browse files
committed
Auto merge of #32980 - Aatch:better-mir-building, r=nagisa
Various improvements to MIR and LLVM IR Construction Primarily affects the MIR construction, which indirectly improves LLVM IR generation, but some LLVM IR changes have been made too. * Handle "statement expressions" more intelligently. These are expressions that always evaluate to `()`. Previously a temporary would be generated as a destination to translate into, which is unnecessary. This affects assignment, augmented assignment, `return`, `break` and `continue`. * Avoid inserting drops for non-drop types in more places. Scheduled drops were already skipped for types that we knew wouldn't need dropping at construction time. However manually-inserted drops like those for `x` in `x = y;` were still generated. `build_drop` now takes a type parameter like its `schedule_drop` counterpart and checks to see if the type needs dropping. * Avoid generating an extra temporary for an assignment where the types involved don't need dropping. Previously an expression like `a = b + 1;` would result in a temporary for `b + 1`. This is so the RHS can be evaluated, then the LHS evaluated and dropped and have everything work correctly. However, this isn't necessary if the `LHS` doesn't need a drop, as we can just overwrite the existing value. * Improves lvalue analysis to allow treating an `Rvalue::Use` as an operand in certain conditions. The reason for it never being an operand is so it can be zeroed/drop-filled, but this is only true for types that need dropping. The first two changes result in significantly fewer MIR blocks being generated, as previously almost every statement would end up generating a new block due to the drop of the `()` temporary being generated.
2 parents cf3970a + 5bda576 commit 009a649

18 files changed

+298
-182
lines changed

src/librustc_mir/build/block.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
4444
StmtKind::Expr { scope, expr } => {
4545
unpack!(block = this.in_scope(scope, block, |this, _| {
4646
let expr = this.hir.mirror(expr);
47-
let expr_span = expr.span;
48-
let temp = this.temp(expr.ty.clone());
49-
unpack!(block = this.into(&temp, block, expr));
50-
unpack!(block = this.build_drop(block, expr_span, temp));
51-
block.unit()
47+
this.stmt_expr(block, expr)
5248
}));
5349
}
5450
StmtKind::Let { remainder_scope, init_scope, pattern, initializer } => {

src/librustc_mir/build/expr/as_rvalue.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,11 @@ impl<'a,'tcx> Builder<'a,'tcx> {
189189
block.and(Rvalue::Aggregate(AggregateKind::Adt(adt_def, variant_index, substs),
190190
fields))
191191
}
192+
ExprKind::Assign { .. } |
193+
ExprKind::AssignOp { .. } => {
194+
block = unpack!(this.stmt_expr(block, expr));
195+
block.and(this.unit_rvalue())
196+
}
192197
ExprKind::Literal { .. } |
193198
ExprKind::Block { .. } |
194199
ExprKind::Match { .. } |
@@ -201,8 +206,6 @@ impl<'a,'tcx> Builder<'a,'tcx> {
201206
ExprKind::Index { .. } |
202207
ExprKind::VarRef { .. } |
203208
ExprKind::SelfRef |
204-
ExprKind::Assign { .. } |
205-
ExprKind::AssignOp { .. } |
206209
ExprKind::Break { .. } |
207210
ExprKind::Continue { .. } |
208211
ExprKind::Return { .. } |

src/librustc_mir/build/expr/into.rs

+9-78
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,9 @@
1212
1313
use build::{BlockAnd, BlockAndExtension, Builder};
1414
use build::expr::category::{Category, RvalueFunc};
15-
use build::scope::LoopScope;
1615
use hair::*;
17-
use rustc::middle::region::CodeExtent;
1816
use rustc::ty;
1917
use rustc::mir::repr::*;
20-
use syntax::codemap::Span;
2118

2219
impl<'a,'tcx> Builder<'a,'tcx> {
2320
/// Compile `expr`, storing the result into `destination`, which
@@ -207,65 +204,6 @@ impl<'a,'tcx> Builder<'a,'tcx> {
207204
}
208205
exit_block.unit()
209206
}
210-
ExprKind::Assign { lhs, rhs } => {
211-
// Note: we evaluate assignments right-to-left. This
212-
// is better for borrowck interaction with overloaded
213-
// operators like x[j] = x[i].
214-
let lhs = this.hir.mirror(lhs);
215-
let lhs_span = lhs.span;
216-
let rhs = unpack!(block = this.as_operand(block, rhs));
217-
let lhs = unpack!(block = this.as_lvalue(block, lhs));
218-
unpack!(block = this.build_drop(block, lhs_span, lhs.clone()));
219-
this.cfg.push_assign(block, scope_id, expr_span, &lhs, Rvalue::Use(rhs));
220-
block.unit()
221-
}
222-
ExprKind::AssignOp { op, lhs, rhs } => {
223-
// FIXME(#28160) there is an interesting semantics
224-
// question raised here -- should we "freeze" the
225-
// value of the lhs here? I'm inclined to think not,
226-
// since it seems closer to the semantics of the
227-
// overloaded version, which takes `&mut self`. This
228-
// only affects weird things like `x += {x += 1; x}`
229-
// -- is that equal to `x + (x + 1)` or `2*(x+1)`?
230-
231-
// As above, RTL.
232-
let rhs = unpack!(block = this.as_operand(block, rhs));
233-
let lhs = unpack!(block = this.as_lvalue(block, lhs));
234-
235-
// we don't have to drop prior contents or anything
236-
// because AssignOp is only legal for Copy types
237-
// (overloaded ops should be desugared into a call).
238-
this.cfg.push_assign(block, scope_id, expr_span, &lhs,
239-
Rvalue::BinaryOp(op,
240-
Operand::Consume(lhs.clone()),
241-
rhs));
242-
243-
block.unit()
244-
}
245-
ExprKind::Continue { label } => {
246-
this.break_or_continue(expr_span, label, block,
247-
|loop_scope| loop_scope.continue_block)
248-
}
249-
ExprKind::Break { label } => {
250-
this.break_or_continue(expr_span, label, block, |loop_scope| {
251-
loop_scope.might_break = true;
252-
loop_scope.break_block
253-
})
254-
}
255-
ExprKind::Return { value } => {
256-
block = match value {
257-
Some(value) => unpack!(this.into(&Lvalue::ReturnPointer, block, value)),
258-
None => {
259-
this.cfg.push_assign_unit(block, scope_id,
260-
expr_span, &Lvalue::ReturnPointer);
261-
block
262-
}
263-
};
264-
let extent = this.extent_of_return_scope();
265-
let return_block = this.return_block();
266-
this.exit_scope(expr_span, extent, block, return_block);
267-
this.cfg.start_new_block().unit()
268-
}
269207
ExprKind::Call { ty, fun, args } => {
270208
let diverges = match ty.sty {
271209
ty::TyFnDef(_, _, ref f) | ty::TyFnPtr(ref f) => {
@@ -294,6 +232,15 @@ impl<'a,'tcx> Builder<'a,'tcx> {
294232
success.unit()
295233
}
296234

235+
// These cases don't actually need a destination
236+
ExprKind::Assign { .. } |
237+
ExprKind::AssignOp { .. } |
238+
ExprKind::Continue { .. } |
239+
ExprKind::Break { .. } |
240+
ExprKind::Return {.. } => {
241+
this.stmt_expr(block, expr)
242+
}
243+
297244
// these are the cases that are more naturally handled by some other mode
298245
ExprKind::Unary { .. } |
299246
ExprKind::Binary { .. } |
@@ -327,20 +274,4 @@ impl<'a,'tcx> Builder<'a,'tcx> {
327274
}
328275
}
329276
}
330-
331-
fn break_or_continue<F>(&mut self,
332-
span: Span,
333-
label: Option<CodeExtent>,
334-
block: BasicBlock,
335-
exit_selector: F)
336-
-> BlockAnd<()>
337-
where F: FnOnce(&mut LoopScope) -> BasicBlock
338-
{
339-
let (exit_block, extent) = {
340-
let loop_scope = self.find_loop_scope(span, label);
341-
(exit_selector(loop_scope), loop_scope.extent)
342-
};
343-
self.exit_scope(span, extent, block, exit_block);
344-
self.cfg.start_new_block().unit()
345-
}
346277
}

src/librustc_mir/build/expr/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,4 @@ mod as_operand;
7777
mod as_temp;
7878
mod category;
7979
mod into;
80+
mod stmt;

src/librustc_mir/build/expr/stmt.rs

+135
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use build::{BlockAnd, BlockAndExtension, Builder};
12+
use build::scope::LoopScope;
13+
use hair::*;
14+
use rustc::middle::region::CodeExtent;
15+
use rustc::mir::repr::*;
16+
use syntax::codemap::Span;
17+
18+
impl<'a,'tcx> Builder<'a,'tcx> {
19+
20+
pub fn stmt_expr(&mut self, mut block: BasicBlock, expr: Expr<'tcx>) -> BlockAnd<()> {
21+
let this = self;
22+
let expr_span = expr.span;
23+
let scope_id = this.innermost_scope_id();
24+
// Handle a number of expressions that don't need a destination at all. This
25+
// avoids needing a mountain of temporary `()` variables.
26+
match expr.kind {
27+
ExprKind::Scope { extent, value } => {
28+
let value = this.hir.mirror(value);
29+
this.in_scope(extent, block, |this, _| this.stmt_expr(block, value))
30+
}
31+
ExprKind::Assign { lhs, rhs } => {
32+
let lhs = this.hir.mirror(lhs);
33+
let rhs = this.hir.mirror(rhs);
34+
let scope_id = this.innermost_scope_id();
35+
let lhs_span = lhs.span;
36+
37+
let lhs_ty = lhs.ty;
38+
let rhs_ty = rhs.ty;
39+
40+
let lhs_needs_drop = this.hir.needs_drop(lhs_ty);
41+
let rhs_needs_drop = this.hir.needs_drop(rhs_ty);
42+
43+
// Note: we evaluate assignments right-to-left. This
44+
// is better for borrowck interaction with overloaded
45+
// operators like x[j] = x[i].
46+
47+
// Generate better code for things that don't need to be
48+
// dropped.
49+
let rhs = if lhs_needs_drop || rhs_needs_drop {
50+
let op = unpack!(block = this.as_operand(block, rhs));
51+
Rvalue::Use(op)
52+
} else {
53+
unpack!(block = this.as_rvalue(block, rhs))
54+
};
55+
56+
let lhs = unpack!(block = this.as_lvalue(block, lhs));
57+
unpack!(block = this.build_drop(block, lhs_span, lhs.clone(), lhs_ty));
58+
this.cfg.push_assign(block, scope_id, expr_span, &lhs, rhs);
59+
block.unit()
60+
}
61+
ExprKind::AssignOp { op, lhs, rhs } => {
62+
// FIXME(#28160) there is an interesting semantics
63+
// question raised here -- should we "freeze" the
64+
// value of the lhs here? I'm inclined to think not,
65+
// since it seems closer to the semantics of the
66+
// overloaded version, which takes `&mut self`. This
67+
// only affects weird things like `x += {x += 1; x}`
68+
// -- is that equal to `x + (x + 1)` or `2*(x+1)`?
69+
70+
// As above, RTL.
71+
let rhs = unpack!(block = this.as_operand(block, rhs));
72+
let lhs = unpack!(block = this.as_lvalue(block, lhs));
73+
74+
// we don't have to drop prior contents or anything
75+
// because AssignOp is only legal for Copy types
76+
// (overloaded ops should be desugared into a call).
77+
this.cfg.push_assign(block, scope_id, expr_span, &lhs,
78+
Rvalue::BinaryOp(op,
79+
Operand::Consume(lhs.clone()),
80+
rhs));
81+
82+
block.unit()
83+
}
84+
ExprKind::Continue { label } => {
85+
this.break_or_continue(expr_span, label, block,
86+
|loop_scope| loop_scope.continue_block)
87+
}
88+
ExprKind::Break { label } => {
89+
this.break_or_continue(expr_span, label, block, |loop_scope| {
90+
loop_scope.might_break = true;
91+
loop_scope.break_block
92+
})
93+
}
94+
ExprKind::Return { value } => {
95+
block = match value {
96+
Some(value) => unpack!(this.into(&Lvalue::ReturnPointer, block, value)),
97+
None => {
98+
this.cfg.push_assign_unit(block, scope_id,
99+
expr_span, &Lvalue::ReturnPointer);
100+
block
101+
}
102+
};
103+
let extent = this.extent_of_return_scope();
104+
let return_block = this.return_block();
105+
this.exit_scope(expr_span, extent, block, return_block);
106+
this.cfg.start_new_block().unit()
107+
}
108+
_ => {
109+
let expr_span = expr.span;
110+
let expr_ty = expr.ty;
111+
let temp = this.temp(expr.ty.clone());
112+
unpack!(block = this.into(&temp, block, expr));
113+
unpack!(block = this.build_drop(block, expr_span, temp, expr_ty));
114+
block.unit()
115+
}
116+
}
117+
}
118+
119+
fn break_or_continue<F>(&mut self,
120+
span: Span,
121+
label: Option<CodeExtent>,
122+
block: BasicBlock,
123+
exit_selector: F)
124+
-> BlockAnd<()>
125+
where F: FnOnce(&mut LoopScope) -> BasicBlock
126+
{
127+
let (exit_block, extent) = {
128+
let loop_scope = self.find_loop_scope(span, label);
129+
(exit_selector(loop_scope), loop_scope.extent)
130+
};
131+
self.exit_scope(span, extent, block, exit_block);
132+
self.cfg.start_new_block().unit()
133+
}
134+
135+
}

src/librustc_mir/build/misc.rs

+4
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ impl<'a,'tcx> Builder<'a,'tcx> {
4646
Operand::Constant(constant)
4747
}
4848

49+
pub fn unit_rvalue(&mut self) -> Rvalue<'tcx> {
50+
Rvalue::Aggregate(AggregateKind::Tuple, vec![])
51+
}
52+
4953
pub fn push_usize(&mut self,
5054
block: BasicBlock,
5155
scope_id: ScopeId,

src/librustc_mir/build/scope.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -497,8 +497,11 @@ impl<'a,'tcx> Builder<'a,'tcx> {
497497
pub fn build_drop(&mut self,
498498
block: BasicBlock,
499499
span: Span,
500-
value: Lvalue<'tcx>)
501-
-> BlockAnd<()> {
500+
value: Lvalue<'tcx>,
501+
ty: Ty<'tcx>) -> BlockAnd<()> {
502+
if !self.hir.needs_drop(ty) {
503+
return block.unit();
504+
}
502505
let scope_id = self.innermost_scope_id();
503506
let next_target = self.cfg.start_new_block();
504507
let diverge_target = self.diverge_cleanup();

src/librustc_trans/builder.rs

+31-24
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
165165
args: &[ValueRef],
166166
then: BasicBlockRef,
167167
catch: BasicBlockRef,
168-
bundle: Option<&OperandBundleDef>)
169-
-> ValueRef {
168+
bundle: Option<&OperandBundleDef>) -> ValueRef {
170169
self.count_insn("invoke");
171170

172171
debug!("Invoke {:?} with args ({})",
@@ -176,6 +175,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
176175
.collect::<Vec<String>>()
177176
.join(", "));
178177

178+
check_call("invoke", llfn, args);
179+
179180
let bundle = bundle.as_ref().map(|b| b.raw()).unwrap_or(0 as *mut _);
180181

181182
unsafe {
@@ -856,28 +857,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
856857
.collect::<Vec<String>>()
857858
.join(", "));
858859

859-
let mut fn_ty = val_ty(llfn);
860-
// Strip off pointers
861-
while fn_ty.kind() == llvm::TypeKind::Pointer {
862-
fn_ty = fn_ty.element_type();
863-
}
864-
865-
assert!(fn_ty.kind() == llvm::TypeKind::Function,
866-
"builder::call not passed a function");
867-
868-
let param_tys = fn_ty.func_params();
869-
870-
let iter = param_tys.into_iter()
871-
.zip(args.iter().map(|&v| val_ty(v)));
872-
for (i, (expected_ty, actual_ty)) in iter.enumerate() {
873-
if expected_ty != actual_ty {
874-
bug!("Type mismatch in function call of {:?}. \
875-
Expected {:?} for param {}, got {:?}",
876-
Value(llfn),
877-
expected_ty, i, actual_ty);
878-
879-
}
880-
}
860+
check_call("call", llfn, args);
881861

882862
let bundle = bundle.as_ref().map(|b| b.raw()).unwrap_or(0 as *mut _);
883863

@@ -1121,3 +1101,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
11211101
}
11221102
}
11231103
}
1104+
1105+
fn check_call(typ: &str, llfn: ValueRef, args: &[ValueRef]) {
1106+
if cfg!(debug_assertions) {
1107+
let mut fn_ty = val_ty(llfn);
1108+
// Strip off pointers
1109+
while fn_ty.kind() == llvm::TypeKind::Pointer {
1110+
fn_ty = fn_ty.element_type();
1111+
}
1112+
1113+
assert!(fn_ty.kind() == llvm::TypeKind::Function,
1114+
"builder::{} not passed a function", typ);
1115+
1116+
let param_tys = fn_ty.func_params();
1117+
1118+
let iter = param_tys.into_iter()
1119+
.zip(args.iter().map(|&v| val_ty(v)));
1120+
for (i, (expected_ty, actual_ty)) in iter.enumerate() {
1121+
if expected_ty != actual_ty {
1122+
bug!("Type mismatch in function call of {:?}. \
1123+
Expected {:?} for param {}, got {:?}",
1124+
Value(llfn),
1125+
expected_ty, i, actual_ty);
1126+
1127+
}
1128+
}
1129+
}
1130+
}

0 commit comments

Comments
 (0)