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

rustc_mir: allow promotion of promotable temps indexed at runtime. #50603

Merged
merged 5 commits into from
May 19, 2018
Merged
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
162 changes: 104 additions & 58 deletions src/librustc_mir/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@
use rustc::mir::*;
use rustc::mir::visit::{PlaceContext, MutVisitor, Visitor};
use rustc::mir::traversal::ReversePostorder;
use rustc::ty::TyCtxt;
use rustc::ty::{self, TyCtxt};
use syntax_pos::Span;

use rustc_data_structures::indexed_vec::{IndexVec, Idx};

use std::iter;
use std::mem;
use std::usize;
use std::{cmp, iter, mem, usize};

/// State of a temporary during collection and promotion.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -150,9 +148,11 @@ pub fn collect_temps(mir: &Mir, rpo: &mut ReversePostorder) -> IndexVec<Local, T
}

struct Promoter<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
source: &'a mut Mir<'tcx>,
promoted: Mir<'tcx>,
temps: &'a mut IndexVec<Local, TempState>,
extra_statements: &'a mut Vec<(Location, Statement<'tcx>)>,

/// If true, all nested temps are also kept in the
/// source MIR, not moved to the promoted MIR.
Expand Down Expand Up @@ -288,38 +288,90 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
}

fn promote_candidate(mut self, candidate: Candidate) {
let span = self.promoted.span;
let new_operand = Operand::Constant(box Constant {
span,
ty: self.promoted.return_ty(),
literal: Literal::Promoted {
let mut rvalue = {
let promoted = &mut self.promoted;
let literal = Literal::Promoted {
index: Promoted::new(self.source.promoted.len())
}
});
let mut rvalue = match candidate {
Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => {
let ref mut statement = self.source[bb].statements[stmt_idx];
match statement.kind {
StatementKind::Assign(_, ref mut rvalue) => {
mem::replace(rvalue, Rvalue::Use(new_operand))
};
let operand = |ty, span| {
promoted.span = span;
promoted.local_decls[RETURN_PLACE] =
LocalDecl::new_return_place(ty, span);
Operand::Constant(box Constant {
span,
ty,
literal
})
};
let (blocks, local_decls) = self.source.basic_blocks_and_local_decls_mut();
match candidate {
Candidate::Ref(loc) => {
let ref mut statement = blocks[loc.block].statements[loc.statement_index];
match statement.kind {
StatementKind::Assign(_, Rvalue::Ref(r, bk, ref mut place)) => {
// Find the underlying local for this (necessarilly interior) borrow.
// HACK(eddyb) using a recursive function because of mutable borrows.
fn interior_base<'a, 'tcx>(place: &'a mut Place<'tcx>)
-> &'a mut Place<'tcx> {
if let Place::Projection(ref mut proj) = *place {
assert_ne!(proj.elem, ProjectionElem::Deref);
return interior_base(&mut proj.base);
}
place
}
let place = interior_base(place);

let ty = place.ty(local_decls, self.tcx).to_ty(self.tcx);
let ref_ty = self.tcx.mk_ref(r,
ty::TypeAndMut {
ty,
mutbl: bk.to_mutbl_lossy()
}
);
let span = statement.source_info.span;

// Create a temp to hold the promoted reference.
// This is because `*r` requires `r` to be a local,
// otherwise we would use the `promoted` directly.
let mut promoted_ref = LocalDecl::new_temp(ref_ty, span);
promoted_ref.source_info = statement.source_info;
let promoted_ref = local_decls.push(promoted_ref);
assert_eq!(self.temps.push(TempState::Unpromotable), promoted_ref);
self.extra_statements.push((loc, Statement {
source_info: statement.source_info,
kind: StatementKind::Assign(
Place::Local(promoted_ref),
Rvalue::Use(operand(ref_ty, span)),
)
}));
let promoted_place = Place::Local(promoted_ref).deref();

Rvalue::Ref(r, bk, mem::replace(place, promoted_place))
}
_ => bug!()
}
_ => bug!()
}
}
Candidate::Argument { bb, index } => {
match self.source[bb].terminator_mut().kind {
TerminatorKind::Call { ref mut args, .. } => {
Rvalue::Use(mem::replace(&mut args[index], new_operand))
Candidate::Argument { bb, index } => {
let terminator = blocks[bb].terminator_mut();
match terminator.kind {
TerminatorKind::Call { ref mut args, .. } => {
let ty = args[index].ty(local_decls, self.tcx);
let span = terminator.source_info.span;
Rvalue::Use(mem::replace(&mut args[index], operand(ty, span)))
}
_ => bug!()
}
_ => bug!()
}
}
};

assert_eq!(self.new_block(), START_BLOCK);
self.visit_rvalue(&mut rvalue, Location {
block: BasicBlock::new(0),
statement_index: usize::MAX
});

let span = self.promoted.span;
self.assign(RETURN_PLACE, rvalue, span);
self.source.promoted.push(self.promoted);
}
Expand All @@ -343,43 +395,29 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>,
candidates: Vec<Candidate>) {
// Visit candidates in reverse, in case they're nested.
debug!("promote_candidates({:?})", candidates);

let mut extra_statements = vec![];
for candidate in candidates.into_iter().rev() {
let (span, ty) = match candidate {
Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => {
let statement = &mir[bb].statements[stmt_idx];
let dest = match statement.kind {
StatementKind::Assign(ref dest, _) => dest,
_ => {
span_bug!(statement.source_info.span,
"expected assignment to promote");
}
};
if let Place::Local(index) = *dest {
if temps[index] == TempState::PromotedOut {
// Already promoted.
continue;
match candidate {
Candidate::Ref(Location { block, statement_index }) => {
match mir[block].statements[statement_index].kind {
StatementKind::Assign(Place::Local(local), _) => {
if temps[local] == TempState::PromotedOut {
// Already promoted.
continue;
}
}
_ => {}
}
(statement.source_info.span, dest.ty(mir, tcx).to_ty(tcx))
}
Candidate::Argument { bb, index } => {
let terminator = mir[bb].terminator();
let ty = match terminator.kind {
TerminatorKind::Call { ref args, .. } => {
args[index].ty(mir, tcx)
}
_ => {
span_bug!(terminator.source_info.span,
"expected call argument to promote");
}
};
(terminator.source_info.span, ty)
}
};
Candidate::Argument { .. } => {}
}


// Declare return place local
let initial_locals = iter::once(LocalDecl::new_return_place(ty, span))
.collect();
// Declare return place local so that `Mir::new` doesn't complain.
let initial_locals = iter::once(
LocalDecl::new_return_place(tcx.types.never, mir.span)
).collect();

let mut promoter = Promoter {
promoted: Mir::new(
Expand All @@ -393,16 +431,24 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>,
initial_locals,
0,
vec![],
span
mir.span
),
tcx,
source: mir,
temps: &mut temps,
extra_statements: &mut extra_statements,
keep_original: false
};
assert_eq!(promoter.new_block(), START_BLOCK);
promoter.promote_candidate(candidate);
}

// Insert each of `extra_statements` before its indicated location, which
// has to be done in reverse location order, to not invalidate the rest.
extra_statements.sort_by_key(|&(loc, _)| cmp::Reverse(loc));
for (loc, statement) in extra_statements {
mir[loc.block].statements.insert(loc.statement_index, statement);
}

// Eliminate assignments to, and drops of promoted temps.
let promoted = |index: Local| temps[index] == TempState::PromotedOut;
for block in mir.basic_blocks_mut() {
Expand Down
70 changes: 49 additions & 21 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,12 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
}

/// Check if a Local with the current qualifications is promotable.
fn can_promote(&mut self) -> bool {
fn can_promote(&self, qualif: Qualif) -> bool {
// References to statics are allowed, but only in other statics.
if self.mode == Mode::Static || self.mode == Mode::StaticMut {
(self.qualif - Qualif::STATIC_REF).is_empty()
(qualif - Qualif::STATIC_REF).is_empty()
} else {
self.qualif.is_empty()
qualif.is_empty()
}
}

Expand Down Expand Up @@ -679,24 +679,31 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
}

let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx);

// Default to forbidding the borrow and/or its promotion,
// due to the potential for direct or interior mutability,
// and only proceed by setting `forbidden_mut` to `false`.
let mut forbidden_mut = true;

if let BorrowKind::Mut { .. } = kind {
// In theory, any zero-sized value could be borrowed
// mutably without consequences. However, only &mut []
// is allowed right now, and only in functions.
let allow = if self.mode == Mode::StaticMut {
if self.mode == Mode::StaticMut {
// Inside a `static mut`, &mut [...] is also allowed.
match ty.sty {
ty::TyArray(..) | ty::TySlice(_) => true,
_ => false
ty::TyArray(..) | ty::TySlice(_) => forbidden_mut = false,
_ => {}
}
} else if let ty::TyArray(_, len) = ty.sty {
len.unwrap_usize(self.tcx) == 0 &&
self.mode == Mode::Fn
} else {
false
};
// FIXME(eddyb) the `self.mode == Mode::Fn` condition
// seems unnecessary, given that this is merely a ZST.
if len.unwrap_usize(self.tcx) == 0 && self.mode == Mode::Fn {
forbidden_mut = false;
}
}

if !allow {
if forbidden_mut {
self.add(Qualif::NOT_CONST);
if self.mode != Mode::Fn {
let mut err = struct_span_err!(self.tcx.sess, self.span, E0017,
Expand All @@ -722,25 +729,46 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
// it means that our "silent insertion of statics" could change
// initializer values (very bad).
if self.qualif.intersects(Qualif::MUTABLE_INTERIOR) {
// Replace MUTABLE_INTERIOR with NOT_CONST to avoid
// A reference of a MUTABLE_INTERIOR place is instead
// NOT_CONST (see `if forbidden_mut` below), to avoid
// duplicate errors (from reborrowing, for example).
self.qualif = self.qualif - Qualif::MUTABLE_INTERIOR;
self.add(Qualif::NOT_CONST);
if self.mode != Mode::Fn {
span_err!(self.tcx.sess, self.span, E0492,
"cannot borrow a constant which may contain \
interior mutability, create a static instead");
}
} else {
// We allow immutable borrows of frozen data.
forbidden_mut = false;
}
}

// We might have a candidate for promotion.
let candidate = Candidate::Ref(location);
if self.can_promote() {
// We can only promote direct borrows of temps.
if forbidden_mut {
self.add(Qualif::NOT_CONST);
} else {
// We might have a candidate for promotion.
let candidate = Candidate::Ref(location);
// We can only promote interior borrows of promotable temps.
let mut place = place;
while let Place::Projection(ref proj) = *place {
if proj.elem == ProjectionElem::Deref {
break;
}
place = &proj.base;
}
if let Place::Local(local) = *place {
if self.mir.local_kind(local) == LocalKind::Temp {
self.promotion_candidates.push(candidate);
if let Some(qualif) = self.temp_qualif[local] {
// `forbidden_mut` is false, so we can safely ignore
// `MUTABLE_INTERIOR` from the local's qualifications.
// This allows borrowing fields which don't have
// `MUTABLE_INTERIOR`, from a type that does, e.g.:
// `let _: &'static _ = &(Cell::new(1), 2).1;`
if self.can_promote(qualif - Qualif::MUTABLE_INTERIOR) {
self.promotion_candidates.push(candidate);
}
}
}
}
}
Expand Down Expand Up @@ -897,7 +925,7 @@ This does not pose a problem by itself because they can't be accessed directly."
}
let candidate = Candidate::Argument { bb, index: i };
if is_shuffle && i == 2 {
if this.can_promote() {
if this.can_promote(this.qualif) {
this.promotion_candidates.push(candidate);
} else {
span_err!(this.tcx.sess, this.span, E0526,
Expand All @@ -913,7 +941,7 @@ This does not pose a problem by itself because they can't be accessed directly."
if !constant_arguments.contains(&i) {
return
}
if this.can_promote() {
if this.can_promote(this.qualif) {
this.promotion_candidates.push(candidate);
} else {
this.tcx.sess.span_err(this.span,
Expand Down
8 changes: 6 additions & 2 deletions src/test/mir-opt/end_region_destruction_extents_1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,21 @@ unsafe impl<'a, #[may_dangle] 'b> Drop for D1<'a, 'b> {
// let mut _7: &'10s S1;
// let mut _8: &'10s S1;
// let mut _9: S1;
// let mut _10: &'10s S1;
// let mut _11: &'12ds S1;
//
// bb0: {
// StorageLive(_2);
// StorageLive(_3);
// StorageLive(_4);
// StorageLive(_5);
// _5 = promoted[1];
// _11 = promoted[1];
// _5 = &'12ds (*_11);
// _4 = &'12ds (*_5);
// StorageLive(_7);
// StorageLive(_8);
// _8 = promoted[0];
// _10 = promoted[0];
// _8 = &'10s (*_10);
// _7 = &'10s (*_8);
// _3 = D1<'12ds, '10s>::{{constructor}}(move _4, move _7);
// EndRegion('10s);
Expand Down
3 changes: 2 additions & 1 deletion src/test/mir-opt/match_false_edges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ fn main() {
// }
// bb9: { // binding1 and guard
// StorageLive(_5);
// _5 = &((_2 as Some).0: i32);
// _11 = promoted[0];
// _5 = &(((*_11) as Some).0: i32);
// StorageLive(_8);
// _8 = const guard() -> [return: bb10, unwind: bb1];
// }
Expand Down
Loading