Skip to content

Commit

Permalink
Auto merge of #42023 - nikomatsakis:issue-36799-ostn15_phf, r=arielb1
Browse files Browse the repository at this point in the history
introduce local-scope to prevent `StorageLive`/`StorageDead` in statics

In investigating #36799, I found that we were creating storage-live/storage-dead instructions in statics/constants, where they are not needed. This arose due to the fix for local scopes. This PR tries to fix that (and adds a test -- I'm curious if there is a way to make that test more targeted, though).

r? @arielb1
  • Loading branch information
bors committed May 23, 2017
2 parents 9fa25a7 + 8a4e593 commit 5b13bff
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/librustc_mir/build/expr/as_operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
-> BlockAnd<Operand<'tcx>>
where M: Mirror<'tcx, Output = Expr<'tcx>>
{
let topmost_scope = self.topmost_scope(); // FIXME(#6393)
self.as_operand(block, Some(topmost_scope), expr)
let local_scope = self.local_scope();
self.as_operand(block, local_scope, expr)
}

/// Compile `expr` into a value that can be used as an operand.
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
-> BlockAnd<Rvalue<'tcx>>
where M: Mirror<'tcx, Output = Expr<'tcx>>
{
let topmost_scope = self.topmost_scope(); // FIXME(#6393)
self.as_rvalue(block, Some(topmost_scope), expr)
let local_scope = self.local_scope();
self.as_rvalue(block, local_scope, expr)
}

/// Compile `expr`, yielding an rvalue.
Expand All @@ -51,7 +51,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
scope: Option<CodeExtent>,
expr: Expr<'tcx>)
-> BlockAnd<Rvalue<'tcx>> {
debug!("expr_as_rvalue(block={:?}, expr={:?})", block, expr);
debug!("expr_as_rvalue(block={:?}, scope={:?}, expr={:?})", block, scope, expr);

let this = self;
let expr_span = expr.span;
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
temp_lifetime: Option<CodeExtent>,
expr: Expr<'tcx>)
-> BlockAnd<Lvalue<'tcx>> {
debug!("expr_as_temp(block={:?}, expr={:?})", block, expr);
debug!("expr_as_temp(block={:?}, temp_lifetime={:?}, expr={:?})",
block, temp_lifetime, expr);
let this = self;

if let ExprKind::Scope { extent, value } = expr.kind {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_mir/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,9 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>,
mir
}

pub fn construct_const<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>,
body_id: hir::BodyId)
-> Mir<'tcx> {
fn construct_const<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>,
body_id: hir::BodyId)
-> Mir<'tcx> {
let tcx = hir.tcx();
let ast_expr = &tcx.hir.body(body_id).value;
let ty = hir.tables().expr_ty_adjusted(ast_expr);
Expand All @@ -415,7 +415,7 @@ pub fn construct_const<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>,
builder.finish(vec![], ty)
}

pub fn construct_error<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>,
fn construct_error<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>,
body_id: hir::BodyId)
-> Mir<'tcx> {
let span = hir.tcx().hir.span(hir.tcx().hir.body_owner(body_id));
Expand Down
36 changes: 36 additions & 0 deletions src/librustc_mir/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ use rustc::middle::const_val::ConstVal;
use rustc::ty::subst::{Kind, Subst};
use rustc::ty::{Ty, TyCtxt};
use rustc::mir::*;
use rustc::mir::transform::MirSource;
use syntax_pos::Span;
use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::fx::FxHashMap;
Expand Down Expand Up @@ -428,6 +429,41 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
self.scopes.last().expect("topmost_scope: no scopes present").extent
}

/// Returns the scope that we should use as the lifetime of an
/// operand. Basically, an operand must live until it is consumed.
/// This is similar to, but not quite the same as, the temporary
/// scope (which can be larger or smaller).
///
/// Consider:
///
/// let x = foo(bar(X, Y));
///
/// We wish to pop the storage for X and Y after `bar()` is
/// called, not after the whole `let` is completed.
///
/// As another example, if the second argument diverges:
///
/// foo(Box::new(2), panic!())
///
/// We would allocate the box but then free it on the unwinding
/// path; we would also emit a free on the 'success' path from
/// panic, but that will turn out to be removed as dead-code.
///
/// When building statics/constants, returns `None` since
/// intermediate values do not have to be dropped in that case.
pub fn local_scope(&self) -> Option<CodeExtent> {
match self.hir.src {
MirSource::Const(_) |
MirSource::Static(..) =>
// No need to free storage in this context.
None,
MirSource::Fn(_) =>
Some(self.topmost_scope()),
MirSource::Promoted(..) =>
bug!(),
}
}

// Scheduling drops
// ================
/// Indicates that `lvalue` should be dropped on exit from
Expand Down
8 changes: 7 additions & 1 deletion src/librustc_mir/hair/cx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,14 @@ pub struct Cx<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
tcx: TyCtxt<'a, 'gcx, 'tcx>,
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
pub region_maps: Rc<RegionMaps>,

/// This is `Constness::Const` if we are compiling a `static`,
/// `const`, or the body of a `const fn`.
constness: hir::Constness,

/// What are we compiling?
pub src: MirSource,

/// True if this constant/function needs overflow checks.
check_overflow: bool,
}
Expand Down Expand Up @@ -74,7 +80,7 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
// Constants and const fn's always need overflow checks.
check_overflow |= constness == hir::Constness::Const;

Cx { tcx, infcx, region_maps, constness, check_overflow }
Cx { tcx, infcx, region_maps, constness, src, check_overflow }
}
}

Expand Down
100 changes: 100 additions & 0 deletions src/test/mir-opt/storage_live_dead_in_statics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// Copyright 2017 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.

// Check that when we compile the static `XXX` into MIR, we do not
// generate `StorageStart` or `StorageEnd` statements.

// ignore-tidy-linelength

static XXX: &'static Foo = &Foo {
tup: "hi",
data: &[
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
(0, 1), (0, 2), (0, 3),
]
};

#[derive(Debug)]
struct Foo {
tup: &'static str,
data: &'static [(u32, u32)]
}

fn main() {
println!("{:?}", XXX);
}

// END RUST SOURCE
// START rustc.node4.mir_map.0.mir
// bb0: {
// _7 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:29:9: 29:15
// _8 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:29:17: 29:23
// _9 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:29:25: 29:31
// _10 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:30:9: 30:15
// _11 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:30:17: 30:23
// _12 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:30:25: 30:31
// _13 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:31:9: 31:15
// _14 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:31:17: 31:23
// _15 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:31:25: 31:31
// _16 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:32:9: 32:15
// _17 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:32:17: 32:23
// _18 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:32:25: 32:31
// _19 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:33:9: 33:15
// _20 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:33:17: 33:23
// _21 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:33:25: 33:31
// _22 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:34:9: 34:15
// _23 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:34:17: 34:23
// _24 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:34:25: 34:31
// _25 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:35:9: 35:15
// _26 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:35:17: 35:23
// _27 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:35:25: 35:31
// _28 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:36:9: 36:15
// _29 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:36:17: 36:23
// _30 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:36:25: 36:31
// _31 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:37:9: 37:15
// _32 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:37:17: 37:23
// _33 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:37:25: 37:31
// _34 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:38:9: 38:15
// _35 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:38:17: 38:23
// _36 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:38:25: 38:31
// _37 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:39:9: 39:15
// _38 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:39:17: 39:23
// _39 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:39:25: 39:31
// _40 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:40:9: 40:15
// _41 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:40:17: 40:23
// _42 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:40:25: 40:31
// _43 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:41:9: 41:15
// _44 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:41:17: 41:23
// _45 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:41:25: 41:31
// _46 = (const 0u32, const 1u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:42:9: 42:15
// _47 = (const 0u32, const 2u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:42:17: 42:23
// _48 = (const 0u32, const 3u32); // scope 0 at src/test/mir-opt/basic_assignment.rs:42:25: 42:31
// _6 = [_7, _8, _9, _10, _11, _12, _13, _14, _15, _16, _17, _18, _19, _20, _21, _22, _23, _24, _25, _26, _27, _28, _29, _30, _31, _32, _33, _34, _35, _36, _37, _38, _39, _40, _41, _42, _43, _44, _45, _46, _47, _48]; // scope 0 at src/test/mir-opt/basic_assignment.rs:28:12: 43:6
// _5 = &_6; // scope 0 at src/test/mir-opt/basic_assignment.rs:28:11: 43:6
// _4 = &(*_5); // scope 0 at src/test/mir-opt/basic_assignment.rs:28:11: 43:6
// _3 = _4 as &'static [(u32, u32)] (Unsize); // scope 0 at src/test/mir-opt/basic_assignment.rs:28:11: 43:6
// _2 = Foo { tup: const "hi", data: _3 }; // scope 0 at src/test/mir-opt/basic_assignment.rs:26:29: 44:2
// _1 = &_2; // scope 0 at src/test/mir-opt/basic_assignment.rs:26:28: 44:2
// _0 = &(*_1); // scope 0 at src/test/mir-opt/basic_assignment.rs:26:28: 44:2
// return; // scope 0 at src/test/mir-opt/basic_assignment.rs:26:1: 44:3
// }
// END rustc.node4.mir_map.0.mir

0 comments on commit 5b13bff

Please sign in to comment.