OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #25645 - luqmana:lnr, r=eddyb

Browse Source 
This micro-optimization actually led to generating broken IR in certain cases.

Fixes #18845.
Fixes #25497.
This commit is contained in:
bors 2015-05-20 13:58:10 +00:00
commit cec980a1a7
4 changed files with 60 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/librustc_trans/trans/datum.rs
View File

@ -74,17 +74,6 @@
//! `&foo()` or `match foo() { ref x => ... }`, where the user is
//! implicitly requesting a temporary.
//!
//! Somewhat surprisingly, not all lvalue expressions yield lvalue datums
//! when trans'd. Ultimately the reason for this is to micro-optimize
//! the resulting LLVM. For example, consider the following code:
//!
//! fn foo() -> Box<int> { ... }
//! let x = *foo();
//!
//! The expression `*foo()` is an lvalue, but if you invoke `expr::trans`,
//! it will return an rvalue datum. See `deref_once` in expr.rs for
//! more details.
//!
//! ### Rvalues in detail
//!
//! Rvalues datums are values with no cleanup scheduled. One must be

57
src/librustc_trans/trans/expr.rs
View File

@ -2248,16 +2248,20 @@ fn deref_once<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
let r = match datum.ty.sty {
ty::ty_uniq(content_ty) => {
// Make sure we have an lvalue datum here to get the
// proper cleanups scheduled
let datum = unpack_datum!(
bcx, datum.to_lvalue_datum(bcx, "deref", expr.id));
if type_is_sized(bcx.tcx(), content_ty) {
deref_owned_pointer(bcx, expr, datum, content_ty)
let ptr = load_ty(bcx, datum.val, datum.ty);
DatumBlock::new(bcx, Datum::new(ptr, content_ty, LvalueExpr))
} else {
// A fat pointer and a DST lvalue have the same representation
// just different types. Since there is no temporary for `*e`
// here (because it is unsized), we cannot emulate the sized
// object code path for running drop glue and free. Instead,
// we schedule cleanup for `e`, turning it into an lvalue.
let datum = unpack_datum!(
bcx, datum.to_lvalue_datum(bcx, "deref", expr.id));
let datum = Datum::new(datum.val, content_ty, LvalueExpr);
DatumBlock::new(bcx, datum)
@ -2294,53 +2298,6 @@ fn deref_once<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
expr.id, method_call, r.datum.to_string(ccx));
return r;
/// We microoptimize derefs of owned pointers a bit here. Basically, the idea is to make the
/// deref of an rvalue result in an rvalue. This helps to avoid intermediate stack slots in the
/// resulting LLVM. The idea here is that, if the `Box<T>` pointer is an rvalue, then we can
/// schedule a *shallow* free of the `Box<T>` pointer, and then return a ByRef rvalue into the
/// pointer. Because the free is shallow, it is legit to return an rvalue, because we know that
/// the contents are not yet scheduled to be freed. The language rules ensure that the contents
/// will be used (or moved) before the free occurs.
fn deref_owned_pointer<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
expr: &ast::Expr,
datum: Datum<'tcx, Expr>,
content_ty: Ty<'tcx>)
-> DatumBlock<'blk, 'tcx, Expr> {
match datum.kind {
RvalueExpr(Rvalue { mode: ByRef }) => {
let scope = cleanup::temporary_scope(bcx.tcx(), expr.id);
let ptr = Load(bcx, datum.val);
if !type_is_zero_size(bcx.ccx(), content_ty) {
bcx.fcx.schedule_free_value(scope, ptr, cleanup::HeapExchange, content_ty);
}
}
RvalueExpr(Rvalue { mode: ByValue }) => {
let scope = cleanup::temporary_scope(bcx.tcx(), expr.id);
if !type_is_zero_size(bcx.ccx(), content_ty) {
bcx.fcx.schedule_free_value(scope, datum.val, cleanup::HeapExchange,
content_ty);
}
}
LvalueExpr => { }
}
// If we had an rvalue in, we produce an rvalue out.
let (llptr, kind) = match datum.kind {
LvalueExpr => {
(Load(bcx, datum.val), LvalueExpr)
}
RvalueExpr(Rvalue { mode: ByRef }) => {
(Load(bcx, datum.val), RvalueExpr(Rvalue::new(ByRef)))
}
RvalueExpr(Rvalue { mode: ByValue }) => {
(datum.val, RvalueExpr(Rvalue::new(ByRef)))
}
};
let datum = Datum { ty: content_ty, val: llptr, kind: kind };
DatumBlock { bcx: bcx, datum: datum }
}
}
#[derive(Debug)]

25
src/test/run-pass/issue-18845.rs Normal file
View File

@ -0,0 +1,25 @@
// Copyright 2015 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.
// This used to generate invalid IR in that even if we took the
// `false` branch we'd still try to free the Box from the other
// arm. This was due to treating `*Box::new(9)` as an rvalue datum
// instead of as an lvalue.
fn test(foo: bool) -> u8 {
match foo {
true => *Box::new(9),
false => 0
}
}
fn main() {
assert_eq!(9, test(true));
}

28
src/test/run-pass/issue-25497.rs Normal file
View File

@ -0,0 +1,28 @@
// Copyright 2015 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.
#[derive(Clone, Debug, PartialEq)]
enum Expression {
Dummy,
Add(Box<Expression>),
}
use Expression::*;
fn simplify(exp: Expression) -> Expression {
match exp {
Add(n) => *n.clone(),
_ => Dummy
}
}
fn main() {
assert_eq!(simplify(Add(Box::new(Dummy))), Dummy);
}