Skip to content

Commit

Permalink
Auto merge of rust-lang#78679 - oli-obk:temp_lifetime, r=eddyb
Browse files Browse the repository at this point in the history
Also generate `StorageDead` in constants

r? `@eddyb`

None of this special casing is actually necessary since we started promoting within constants and statics.

We may want to keep some of it around out of perf reasons, but it's not required for user visible behaviour

somewhat related: rust-lang#68622
  • Loading branch information
bors committed Dec 9, 2020
2 parents c0bfe34 + 84fe7cf commit cc03ee6
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 57 deletions.
22 changes: 6 additions & 16 deletions compiler/rustc_mir/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::mem;

use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_hir::{self as hir, def::DefKind, def_id::DefId, definitions::DefPathData};
use rustc_hir::{self as hir, def_id::DefId, definitions::DefPathData};
use rustc_index::vec::IndexVec;
use rustc_macros::HashStable;
use rustc_middle::ich::StableHashingContext;
Expand Down Expand Up @@ -700,21 +700,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let mut locals = IndexVec::from_elem(dummy, &body.local_decls);

// Now mark those locals as dead that we do not want to initialize
match self.tcx.def_kind(instance.def_id()) {
// statics and constants don't have `Storage*` statements, no need to look for them
//
// FIXME: The above is likely untrue. See
// <https://github.com/rust-lang/rust/pull/70004#issuecomment-602022110>. Is it
// okay to ignore `StorageDead`/`StorageLive` annotations during CTFE?
DefKind::Static | DefKind::Const | DefKind::AssocConst => {}
_ => {
// Mark locals that use `Storage*` annotations as dead on function entry.
let always_live = AlwaysLiveLocals::new(self.body());
for local in locals.indices() {
if !always_live.contains(local) {
locals[local].value = LocalValue::Dead;
}
}
// Mark locals that use `Storage*` annotations as dead on function entry.
let always_live = AlwaysLiveLocals::new(self.body());
for local in locals.indices() {
if !always_live.contains(local) {
locals[local].value = LocalValue::Dead;
}
}
// done
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/as_operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
M: Mirror<'tcx, Output = Expr<'tcx>>,
{
let local_scope = self.local_scope();
self.as_operand(block, local_scope, expr)
self.as_operand(block, Some(local_scope), expr)
}

/// Returns an operand suitable for use until the end of the current scope expression and
Expand Down Expand Up @@ -79,7 +79,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
M: Mirror<'tcx, Output = Expr<'tcx>>,
{
let local_scope = self.local_scope();
self.as_call_operand(block, local_scope, expr)
self.as_call_operand(block, Some(local_scope), expr)
}

/// Compile `expr` into a value that can be used as an operand.
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
M: Mirror<'tcx, Output = Expr<'tcx>>,
{
let local_scope = self.local_scope();
self.as_rvalue(block, local_scope, expr)
self.as_rvalue(block, Some(local_scope), expr)
}

/// Compile `expr`, yielding an rvalue.
Expand Down Expand Up @@ -445,9 +445,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Rvalue::Ref(this.hir.tcx().lifetimes.re_erased, borrow_kind, arg_place),
);

// In constants, temp_lifetime is None. We should not need to drop
// anything because no values with a destructor can be created in
// a constant at this time, even if the type may need dropping.
// See the comment in `expr_as_temp` and on the `rvalue_scopes` field for why
// this can be `None`.
if let Some(temp_lifetime) = temp_lifetime {
this.schedule_drop_storage_and_value(upvar_span, temp_lifetime, temp);
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

// (#66975) Source could be a const of type `!`, so has to
// exist in the generated MIR.
unpack!(block = this.as_temp(block, this.local_scope(), source, Mutability::Mut,));
unpack!(block = this.as_temp(block, Some(this.local_scope()), source, Mutability::Mut,));

// This is an optimization. If the expression was a call then we already have an
// unreachable block. Don't bother to terminate it and create a new one.
Expand Down Expand Up @@ -300,7 +300,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// (evaluating them in order given by user)
let fields_map: FxHashMap<_, _> = fields
.into_iter()
.map(|f| (f.name, unpack!(block = this.as_operand(block, scope, f.expr))))
.map(|f| (f.name, unpack!(block = this.as_operand(block, Some(scope), f.expr))))
.collect();

let field_names = this.hir.all_fields(adt_def, variant_index);
Expand Down Expand Up @@ -468,7 +468,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

ExprKind::Yield { value } => {
let scope = this.local_scope();
let value = unpack!(block = this.as_operand(block, scope, value));
let value = unpack!(block = this.as_operand(block, Some(scope), value));
let resume = this.cfg.start_new_block();
this.record_operands_moved(slice::from_ref(&value));
this.cfg.terminate(
Expand Down
40 changes: 8 additions & 32 deletions compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ use crate::build::matches::{ArmHasGuard, Candidate};
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder, CFG};
use crate::thir::{Arm, Expr, ExprRef, LintLevel};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir as hir;
use rustc_index::vec::IndexVec;
use rustc_middle::middle::region;
use rustc_middle::mir::*;
Expand Down Expand Up @@ -740,18 +739,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// 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.
crate fn local_scope(&self) -> Option<region::Scope> {
match self.hir.body_owner_kind {
hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) =>
// No need to free storage in this context.
{
None
}
hir::BodyOwnerKind::Closure | hir::BodyOwnerKind::Fn => Some(self.scopes.topmost()),
}
crate fn local_scope(&self) -> region::Scope {
self.scopes.topmost()
}

// Scheduling drops
Expand Down Expand Up @@ -938,23 +927,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// not the `DROP(_X)` itself, but the (spurious) unwind pathways
/// that it creates. See #64391 for an example.
crate fn record_operands_moved(&mut self, operands: &[Operand<'tcx>]) {
let scope = match self.local_scope() {
None => {
// if there is no local scope, operands won't be dropped anyway
return;
}
let local_scope = self.local_scope();
let scope = self.scopes.scopes.last_mut().unwrap();

Some(local_scope) => {
let top_scope = self.scopes.scopes.last_mut().unwrap();
assert!(
top_scope.region_scope == local_scope,
"local scope ({:?}) is not the topmost scope!",
local_scope
);

top_scope
}
};
assert_eq!(
scope.region_scope, local_scope,
"local scope is not the topmost scope!",
);

// look for moves of a local variable, like `MOVE(_X)`
let locals_moved = operands
Expand Down Expand Up @@ -993,9 +972,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
match cond {
// Don't try to drop a constant
Operand::Constant(_) => (),
// If constants and statics, we don't generate StorageLive for this
// temporary, so don't try to generate StorageDead for it either.
_ if self.local_scope().is_none() => (),
Operand::Copy(place) | Operand::Move(place) => {
if let Some(cond_temp) = place.as_local() {
// Manually drop the condition on both branches.
Expand Down
3 changes: 3 additions & 0 deletions src/test/mir-opt/const-promotion-extern-static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ static mut BAR: *const &i32 = [&Y].as_ptr();
// EMIT_MIR const_promotion_extern_static.FOO-promoted[0].ConstProp.after.mir
static mut FOO: *const &i32 = [unsafe { &X }].as_ptr();

// EMIT_MIR const_promotion_extern_static.BOP.mir_map.0.mir
static BOP: &i32 = &13;

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
+ // + literal: Const { ty: &[&i32; 1], val: Unevaluated(WithOptConstParam { did: DefId(0:6 ~ const_promotion_extern_static[317d]::BAR), const_param_did: None }, [], Some(promoted[0])) }
+ _2 = &(*_6); // scope 0 at $DIR/const-promotion-extern-static.rs:9:31: 9:35
_1 = move _2 as &[&i32] (Pointer(Unsize)); // scope 0 at $DIR/const-promotion-extern-static.rs:9:31: 9:35
- StorageDead(_4); // scope 0 at $DIR/const-promotion-extern-static.rs:9:34: 9:35
StorageDead(_2); // scope 0 at $DIR/const-promotion-extern-static.rs:9:34: 9:35
_0 = core::slice::<impl [&i32]>::as_ptr(move _1) -> [return: bb1, unwind: bb2]; // scope 0 at $DIR/const-promotion-extern-static.rs:9:31: 9:44
// mir::Constant
// + span: $DIR/const-promotion-extern-static.rs:9:36: 9:42
Expand All @@ -42,6 +44,7 @@
bb1: {
- StorageDead(_5); // scope 0 at $DIR/const-promotion-extern-static.rs:9:43: 9:44
- StorageDead(_3); // scope 0 at $DIR/const-promotion-extern-static.rs:9:43: 9:44
StorageDead(_1); // scope 0 at $DIR/const-promotion-extern-static.rs:9:43: 9:44
return; // scope 0 at $DIR/const-promotion-extern-static.rs:9:1: 9:45
}

Expand Down
17 changes: 17 additions & 0 deletions src/test/mir-opt/const_promotion_extern_static.BOP.mir_map.0.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// MIR for `BOP` 0 mir_map

static BOP: &i32 = {
let mut _0: &i32; // return place in scope 0 at $DIR/const-promotion-extern-static.rs:16:13: 16:17
let _1: &i32; // in scope 0 at $DIR/const-promotion-extern-static.rs:16:20: 16:23
let _2: i32; // in scope 0 at $DIR/const-promotion-extern-static.rs:16:21: 16:23

bb0: {
StorageLive(_1); // scope 0 at $DIR/const-promotion-extern-static.rs:16:20: 16:23
StorageLive(_2); // scope 0 at $DIR/const-promotion-extern-static.rs:16:21: 16:23
_2 = const 13_i32; // scope 0 at $DIR/const-promotion-extern-static.rs:16:21: 16:23
_1 = &_2; // scope 0 at $DIR/const-promotion-extern-static.rs:16:20: 16:23
_0 = &(*_1); // scope 0 at $DIR/const-promotion-extern-static.rs:16:20: 16:23
StorageDead(_1); // scope 0 at $DIR/const-promotion-extern-static.rs:16:22: 16:23
return; // scope 0 at $DIR/const-promotion-extern-static.rs:16:1: 16:24
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
+ // + literal: Const { ty: &[&i32; 1], val: Unevaluated(WithOptConstParam { did: DefId(0:7 ~ const_promotion_extern_static[317d]::FOO), const_param_did: None }, [], Some(promoted[0])) }
+ _2 = &(*_6); // scope 0 at $DIR/const-promotion-extern-static.rs:13:31: 13:46
_1 = move _2 as &[&i32] (Pointer(Unsize)); // scope 0 at $DIR/const-promotion-extern-static.rs:13:31: 13:46
- StorageDead(_4); // scope 0 at $DIR/const-promotion-extern-static.rs:13:45: 13:46
StorageDead(_2); // scope 0 at $DIR/const-promotion-extern-static.rs:13:45: 13:46
_0 = core::slice::<impl [&i32]>::as_ptr(move _1) -> [return: bb1, unwind: bb2]; // scope 0 at $DIR/const-promotion-extern-static.rs:13:31: 13:55
// mir::Constant
// + span: $DIR/const-promotion-extern-static.rs:13:47: 13:53
Expand All @@ -44,6 +46,7 @@
bb1: {
- StorageDead(_5); // scope 0 at $DIR/const-promotion-extern-static.rs:13:54: 13:55
- StorageDead(_3); // scope 0 at $DIR/const-promotion-extern-static.rs:13:54: 13:55
StorageDead(_1); // scope 0 at $DIR/const-promotion-extern-static.rs:13:54: 13:55
return; // scope 0 at $DIR/const-promotion-extern-static.rs:13:1: 13:56
}

Expand Down
44 changes: 44 additions & 0 deletions src/test/mir-opt/storage_live_dead_in_statics.XXX.mir_map.0.mir
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,60 @@ static XXX: &Foo = {
StorageLive(_48); // scope 0 at $DIR/storage_live_dead_in_statics.rs:21:25: 21:31
_48 = (const 0_u32, const 3_u32); // scope 0 at $DIR/storage_live_dead_in_statics.rs:21:25: 21:31
_6 = [move _7, move _8, move _9, move _10, move _11, move _12, move _13, move _14, move _15, move _16, move _17, move _18, move _19, move _20, move _21, move _22, move _23, move _24, move _25, move _26, move _27, move _28, move _29, move _30, move _31, move _32, move _33, move _34, move _35, move _36, move _37, move _38, move _39, move _40, move _41, move _42, move _43, move _44, move _45, move _46, move _47, move _48]; // scope 0 at $DIR/storage_live_dead_in_statics.rs:7:12: 22:6
StorageDead(_48); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_47); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_46); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_45); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_44); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_43); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_42); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_41); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_40); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_39); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_38); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_37); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_36); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_35); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_34); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_33); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_32); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_31); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_30); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_29); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_28); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_27); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_26); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_25); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_24); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_23); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_22); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_21); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_20); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_19); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_18); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_17); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_16); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_15); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_14); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_13); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_12); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_11); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_10); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_9); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_8); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
StorageDead(_7); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
_5 = &_6; // scope 0 at $DIR/storage_live_dead_in_statics.rs:7:11: 22:6
_4 = &(*_5); // scope 0 at $DIR/storage_live_dead_in_statics.rs:7:11: 22:6
_3 = move _4 as &[(u32, u32)] (Pointer(Unsize)); // scope 0 at $DIR/storage_live_dead_in_statics.rs:7:11: 22:6
StorageDead(_4); // scope 0 at $DIR/storage_live_dead_in_statics.rs:22:5: 22:6
_2 = Foo { tup: const "hi", data: move _3 }; // scope 0 at $DIR/storage_live_dead_in_statics.rs:5:29: 23:2
// ty::Const
// + ty: &str
// + val: Value(Slice { data: Allocation { bytes: [104, 105], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [3], len: Size { raw: 2 } }, size: Size { raw: 2 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 2 })
// mir::Constant
// + span: $DIR/storage_live_dead_in_statics.rs:6:10: 6:14
// + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [104, 105], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [3], len: Size { raw: 2 } }, size: Size { raw: 2 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 2 }) }
StorageDead(_3); // scope 0 at $DIR/storage_live_dead_in_statics.rs:23:1: 23:2
_1 = &_2; // scope 0 at $DIR/storage_live_dead_in_statics.rs:5:28: 23:2
_0 = &(*_1); // scope 0 at $DIR/storage_live_dead_in_statics.rs:5:28: 23:2
StorageDead(_5); // scope 0 at $DIR/storage_live_dead_in_statics.rs:23:1: 23:2
Expand Down

0 comments on commit cc03ee6

Please sign in to comment.