Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better StorageLive / StorageDead placement for constants. #44252

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/librustc/ty/maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use ty::fast_reject::SimplifiedType;
use util::nodemap::{DefIdSet, NodeSet};
use util::common::{profq_msg, ProfileQueriesMsg};

use rustc_data_structures::indexed_set::IdxSetBuf;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::fx::FxHashMap;
use std::cell::{RefCell, RefMut, Cell};
Expand Down Expand Up @@ -1019,7 +1020,7 @@ define_maps! { <'tcx>
/// Maps DefId's that have an associated Mir to the result
/// of the MIR qualify_consts pass. The actual meaning of
/// the value isn't known except to the pass itself.
[] fn mir_const_qualif: MirConstQualif(DefId) -> u8,
[] fn mir_const_qualif: MirConstQualif(DefId) -> (u8, Rc<IdxSetBuf<mir::Local>>),

/// Fetch the MIR for a given def-id up till the point where it is
/// ready for const evaluation.
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use syntax::ext::base::SyntaxExtension;
use syntax::parse::filemap_to_stream;
use syntax::symbol::Symbol;
use syntax_pos::{Span, NO_EXPANSION};
use rustc_data_structures::indexed_set::IdxSetBuf;
use rustc::hir::svh::Svh;
use rustc::hir;

Expand Down Expand Up @@ -106,7 +107,9 @@ provide! { <'tcx> tcx, def_id, cdata,
mir
}
generator_sig => { cdata.generator_sig(def_id.index, tcx) }
mir_const_qualif => { cdata.mir_const_qualif(def_id.index) }
mir_const_qualif => {
(cdata.mir_const_qualif(def_id.index), Rc::new(IdxSetBuf::new_empty(0)))
}
typeck_tables_of => { cdata.item_body_tables(def_id.index, tcx) }
closure_kind => { cdata.closure_kind(def_id.index) }
fn_sig => { cdata.fn_sig(def_id.index, tcx) }
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
let kind = match impl_item.kind {
ty::AssociatedKind::Const => {
EntryKind::AssociatedConst(container,
self.tcx.at(ast_item.span).mir_const_qualif(def_id))
self.tcx.at(ast_item.span).mir_const_qualif(def_id).0)
}
ty::AssociatedKind::Method => {
let fn_data = if let hir::ImplItemKind::Method(ref sig, body) = ast_item.node {
Expand Down Expand Up @@ -912,7 +912,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
hir::ItemStatic(_, hir::MutMutable, _) => EntryKind::MutStatic,
hir::ItemStatic(_, hir::MutImmutable, _) => EntryKind::ImmStatic,
hir::ItemConst(..) => {
EntryKind::Const(tcx.at(item.span).mir_const_qualif(def_id))
EntryKind::Const(tcx.at(item.span).mir_const_qualif(def_id).0)
}
hir::ItemFn(_, _, constness, .., body) => {
let data = FnData {
Expand Down Expand Up @@ -1256,7 +1256,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
let body = tcx.hir.body_owned_by(id);

Entry {
kind: EntryKind::Const(tcx.mir_const_qualif(def_id)),
kind: EntryKind::Const(tcx.mir_const_qualif(def_id).0),
visibility: self.lazy(&ty::Visibility::Public),
span: self.lazy(&tcx.def_span(def_id)),
attributes: LazySeq::empty(),
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_mir/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
ExprKind::Box { value } => {
let value = this.hir.mirror(value);
let result = this.temp(expr.ty, expr_span);
this.cfg.push(block, Statement {
source_info,
kind: StatementKind::StorageLive(result.clone())
});
if let Some(scope) = scope {
// schedule a shallow free of that memory, lest we unwind:
this.cfg.push(block, Statement {
source_info,
kind: StatementKind::StorageLive(result.clone())
});
this.schedule_drop(expr_span, scope, &result, value.ty);
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let expr_ty = expr.ty.clone();
let temp = this.temp(expr_ty.clone(), expr_span);

if !expr_ty.is_never() && temp_lifetime.is_some() {
if !expr_ty.is_never() {
this.cfg.push(block, Statement {
source_info,
kind: StatementKind::StorageLive(temp.clone())
Expand Down
105 changes: 80 additions & 25 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//! diagnostics as to why a constant rvalue wasn't promoted.

use rustc_data_structures::bitvec::BitVector;
use rustc_data_structures::indexed_set::IdxSetBuf;
use rustc_data_structures::indexed_vec::{IndexVec, Idx};
use rustc::hir;
use rustc::hir::map as hir_map;
Expand All @@ -33,6 +34,7 @@ use syntax::feature_gate::UnstableFeatures;
use syntax_pos::{Span, DUMMY_SP};

use std::fmt;
use std::rc::Rc;
use std::usize;

use super::promote_consts::{self, Candidate, TempState};
Expand Down Expand Up @@ -127,7 +129,6 @@ struct Qualifier<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {

impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
def_id: DefId,
mir: &'a Mir<'tcx>,
mode: Mode)
Expand All @@ -142,7 +143,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
mir,
rpo,
tcx,
param_env,
param_env: tcx.param_env(def_id),
temp_qualif: IndexVec::from_elem(None, &mir.local_decls),
return_qualif: None,
qualif: Qualif::empty(),
Expand Down Expand Up @@ -368,7 +369,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
}

/// Qualify a whole const, static initializer or const fn.
fn qualify_const(&mut self) -> Qualif {
fn qualify_const(&mut self) -> (Qualif, Rc<IdxSetBuf<Local>>) {
debug!("qualifying {} {:?}", self.mode, self.def_id);

let mir = self.mir;
Expand All @@ -390,7 +391,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {

// Non-terminating calls cannot produce any value.
TerminatorKind::Call { destination: None, .. } => {
return Qualif::empty();
break;
}

TerminatorKind::SwitchInt {..} |
Expand Down Expand Up @@ -472,7 +473,25 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
}
}
}
self.qualif

// Collect all the temps we need to promote.
let mut promoted_temps = IdxSetBuf::new_empty(self.temp_promotion_state.len());

for candidate in &self.promotion_candidates {
match *candidate {
Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => {
match self.mir[bb].statements[stmt_idx].kind {
StatementKind::Assign(_, Rvalue::Ref(_, _, Lvalue::Local(index))) => {
promoted_temps.add(&index);
}
_ => {}
}
}
Candidate::ShuffleIndices(_) => {}
}
}

(self.qualif, Rc::new(promoted_temps))
}
}

Expand Down Expand Up @@ -516,6 +535,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
span_err!(self.tcx.sess, self.span, E0625,
"thread-local statics cannot be \
accessed at compile-time");
self.add(Qualif::NOT_CONST);
return;
}
}
Expand Down Expand Up @@ -598,7 +618,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
if self.tcx.trait_of_item(def_id).is_some() {
self.add_type(constant.ty);
} else {
let bits = self.tcx.at(constant.span).mir_const_qualif(def_id);
let (bits, _) = self.tcx.at(constant.span).mir_const_qualif(def_id);

let qualif = Qualif::from_bits(bits).expect("invalid mir_const_qualif");
self.add(qualif);
Expand Down Expand Up @@ -702,13 +722,11 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {

// We might have a candidate for promotion.
let candidate = Candidate::Ref(location);
if self.mode == Mode::Fn || self.mode == Mode::ConstFn {
if !self.qualif.intersects(Qualif::NEVER_PROMOTE) {
// We can only promote direct borrows of temps.
if let Lvalue::Local(local) = *lvalue {
if self.mir.local_kind(local) == LocalKind::Temp {
self.promotion_candidates.push(candidate);
}
if !self.qualif.intersects(Qualif::NEVER_PROMOTE) {
// We can only promote direct borrows of temps.
if let Lvalue::Local(local) = *lvalue {
if self.mir.local_kind(local) == LocalKind::Temp {
self.promotion_candidates.push(candidate);
}
}
}
Expand Down Expand Up @@ -999,21 +1017,21 @@ pub fn provide(providers: &mut Providers) {

fn mir_const_qualif<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> u8 {
-> (u8, Rc<IdxSetBuf<Local>>) {
// NB: This `borrow()` is guaranteed to be valid (i.e., the value
// cannot yet be stolen), because `mir_validated()`, which steals
// from `mir_const(), forces this query to execute before
// performing the steal.
let mir = &tcx.mir_const(def_id).borrow();

if mir.return_ty.references_error() {
return Qualif::NOT_CONST.bits();
tcx.sess.delay_span_bug(mir.span, "mir_const_qualif: Mir had errors");
return (Qualif::NOT_CONST.bits(), Rc::new(IdxSetBuf::new_empty(0)));
}

let param_env = tcx.param_env(def_id);

let mut qualifier = Qualifier::new(tcx, param_env, def_id, mir, Mode::Const);
qualifier.qualify_const().bits()
let mut qualifier = Qualifier::new(tcx, def_id, mir, Mode::Const);
let (qualif, promoted_temps) = qualifier.qualify_const();
(qualif.bits(), promoted_temps)
}

pub struct QualifyAndPromoteConstants;
Expand All @@ -1023,8 +1041,15 @@ impl MirPass for QualifyAndPromoteConstants {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
src: MirSource,
mir: &mut Mir<'tcx>) {
// There's not really any point in promoting errorful MIR.
if mir.return_ty.references_error() {
tcx.sess.delay_span_bug(mir.span, "QualifyAndPromoteConstants: Mir had errors");
return;
}

let id = src.item_id();
let def_id = tcx.hir.local_def_id(id);
let mut const_promoted_temps = None;
let mode = match src {
MirSource::Fn(_) => {
if tcx.is_const_fn(def_id) {
Expand All @@ -1033,20 +1058,21 @@ impl MirPass for QualifyAndPromoteConstants {
Mode::Fn
}
}
MirSource::Const(_) => {
const_promoted_temps = Some(tcx.mir_const_qualif(def_id).1);
Mode::Const
}
MirSource::Static(_, hir::MutImmutable) => Mode::Static,
MirSource::Static(_, hir::MutMutable) => Mode::StaticMut,
MirSource::GeneratorDrop(_) |
MirSource::Const(_) |
MirSource::Promoted(..) => return
};
let param_env = tcx.param_env(def_id);

if mode == Mode::Fn || mode == Mode::ConstFn {
// This is ugly because Qualifier holds onto mir,
// which can't be mutated until its scope ends.
let (temps, candidates) = {
let mut qualifier = Qualifier::new(tcx, param_env,
def_id, mir, mode);
let mut qualifier = Qualifier::new(tcx, def_id, mir, mode);
if mode == Mode::ConstFn {
// Enforce a constant-like CFG for `const fn`.
qualifier.qualify_const();
Expand All @@ -1062,8 +1088,37 @@ impl MirPass for QualifyAndPromoteConstants {
// Do the actual promotion, now that we know what's viable.
promote_consts::promote_candidates(mir, tcx, temps, candidates);
} else {
let mut qualifier = Qualifier::new(tcx, param_env, def_id, mir, mode);
qualifier.qualify_const();
let promoted_temps = if mode == Mode::Const {
// Already computed by `mir_const_qualif`.
const_promoted_temps.unwrap()
} else {
Qualifier::new(tcx, def_id, mir, mode).qualify_const().1
};

// In `const` and `static` everything without `StorageDead`
// is `'static`, we don't have to create promoted MIR fragments,
// just remove `Drop` and `StorageDead` on "promoted" locals.
for block in mir.basic_blocks_mut() {
block.statements.retain(|statement| {
match statement.kind {
StatementKind::StorageDead(Lvalue::Local(index)) => {
!promoted_temps.contains(&index)
}
_ => true
}
});
let terminator = block.terminator_mut();
match terminator.kind {
TerminatorKind::Drop { location: Lvalue::Local(index), target, .. } => {
if promoted_temps.contains(&index) {
terminator.kind = TerminatorKind::Goto {
target,
};
}
}
_ => {}
}
}
}

// Statics must be Sync.
Expand Down