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: handle all aggregate kinds in, and always run, the deaggregator. #48052

Merged
merged 7 commits into from
Feb 23, 2018
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
#![feature(underscore_lifetimes)]
#![feature(universal_impl_trait)]
#![feature(trace_macros)]
#![feature(trusted_len)]
#![feature(catch_expr)]
#![feature(test)]

Expand Down
65 changes: 62 additions & 3 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ use std::ascii;
use std::borrow::{Cow};
use std::cell::Ref;
use std::fmt::{self, Debug, Formatter, Write};
use std::{iter, u32};
use std::{iter, mem, u32};
use std::ops::{Index, IndexMut};
use std::rc::Rc;
use std::vec::IntoIter;
use syntax::ast::{self, Name};
use syntax::symbol::InternedString;
use syntax_pos::Span;
use syntax_pos::{Span, DUMMY_SP};

mod cache;
pub mod tcx;
Expand Down Expand Up @@ -984,11 +984,62 @@ impl<'tcx> BasicBlockData<'tcx> {
pub fn retain_statements<F>(&mut self, mut f: F) where F: FnMut(&mut Statement) -> bool {
for s in &mut self.statements {
if !f(s) {
s.kind = StatementKind::Nop;
s.make_nop();
}
}
}

pub fn expand_statements<F, I>(&mut self, mut f: F)
where F: FnMut(&mut Statement<'tcx>) -> Option<I>,
I: iter::TrustedLen<Item = Statement<'tcx>>
{
// Gather all the iterators we'll need to splice in, and their positions.
let mut splices: Vec<(usize, I)> = vec![];
let mut extra_stmts = 0;
for (i, s) in self.statements.iter_mut().enumerate() {
if let Some(mut new_stmts) = f(s) {
if let Some(first) = new_stmts.next() {
// We can already store the first new statement.
*s = first;

// Save the other statements for optimized splicing.
let remaining = new_stmts.size_hint().0;
if remaining > 0 {
splices.push((i + 1 + extra_stmts, new_stmts));
extra_stmts += remaining;
}
} else {
s.make_nop();
}
}
}

// Splice in the new statements, from the end of the block.
// FIXME(eddyb) This could be more efficient with a "gap buffer"
// where a range of elements ("gap") is left uninitialized, with
// splicing adding new elements to the end of that gap and moving
// existing elements from before the gap to the end of the gap.
// For now, this is safe code, emulating a gap but initializing it.
let mut gap = self.statements.len()..self.statements.len()+extra_stmts;
self.statements.resize(gap.end, Statement {
source_info: SourceInfo {
span: DUMMY_SP,
scope: ARGUMENT_VISIBILITY_SCOPE
},
kind: StatementKind::Nop
});
for (splice_start, new_stmts) in splices.into_iter().rev() {
let splice_end = splice_start + new_stmts.size_hint().0;
while gap.end > splice_end {
gap.start -= 1;
gap.end -= 1;
self.statements.swap(gap.start, gap.end);
}
self.statements.splice(splice_start..splice_end, new_stmts);
gap.end = splice_start;
}
}

pub fn visitable(&self, index: usize) -> &dyn MirVisitable<'tcx> {
if index < self.statements.len() {
&self.statements[index]
Expand Down Expand Up @@ -1157,6 +1208,14 @@ impl<'tcx> Statement<'tcx> {
pub fn make_nop(&mut self) {
self.kind = StatementKind::Nop
}

/// Changes a statement to a nop and returns the original statement.
pub fn replace_nop(&mut self) -> Self {
Statement {
source_info: self.source_info,
kind: mem::replace(&mut self.kind, StatementKind::Nop)
}
}
}

#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
Expand Down
170 changes: 72 additions & 98 deletions src/librustc_mir/transform/deaggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,116 +21,90 @@ impl MirPass for Deaggregator {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
source: MirSource,
mir: &mut Mir<'tcx>) {
let node_path = tcx.item_path_str(source.def_id);
debug!("running on: {:?}", node_path);
// we only run when mir_opt_level > 2
if tcx.sess.opts.debugging_opts.mir_opt_level <= 2 {
return;
}

// Don't run on constant MIR, because trans might not be able to
// evaluate the modified MIR.
// FIXME(eddyb) Remove check after miri is merged.
let id = tcx.hir.as_local_node_id(source.def_id).unwrap();
match (tcx.hir.body_owner_kind(id), source.promoted) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I find this match a bit hard to read. Maybe something like this?

let is_constant = source.promoted.is_some() || match tcx.hir.body_owner_kind(id) {
    hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => true,
    hir::BodyOwnerKind::Fn => tcx.is_const_fn(source.def_id),
};
if is_constant {
    return;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's copied from elsewhere, this pattern is used in several passes - and this will hopefully go away ~soon anyway.

(hir::BodyOwnerKind::Fn, None) => {},
_ => return
}
// In fact, we might not want to trigger in other cases.
// Ex: when we could use SROA. See issue #35259
(_, Some(_)) |
(hir::BodyOwnerKind::Const, _) |
(hir::BodyOwnerKind::Static(_), _) => return,

for bb in mir.basic_blocks_mut() {
let mut curr: usize = 0;
while let Some(idx) = get_aggregate_statement_index(curr, &bb.statements) {
// do the replacement
debug!("removing statement {:?}", idx);
let src_info = bb.statements[idx].source_info;
let suffix_stmts = bb.statements.split_off(idx+1);
let orig_stmt = bb.statements.pop().unwrap();
let (lhs, rhs) = match orig_stmt.kind {
StatementKind::Assign(ref lhs, ref rhs) => (lhs, rhs),
_ => span_bug!(src_info.span, "expected assign, not {:?}", orig_stmt),
};
let (agg_kind, operands) = match rhs {
&Rvalue::Aggregate(ref agg_kind, ref operands) => (agg_kind, operands),
_ => span_bug!(src_info.span, "expected aggregate, not {:?}", rhs),
};
let (adt_def, variant, substs) = match **agg_kind {
AggregateKind::Adt(adt_def, variant, substs, None)
=> (adt_def, variant, substs),
_ => span_bug!(src_info.span, "expected struct, not {:?}", rhs),
};
let n = bb.statements.len();
bb.statements.reserve(n + operands.len() + suffix_stmts.len());
for (i, op) in operands.iter().enumerate() {
let ref variant_def = adt_def.variants[variant];
let ty = variant_def.fields[i].ty(tcx, substs);
let rhs = Rvalue::Use(op.clone());
(hir::BodyOwnerKind::Fn, _) => {
if tcx.is_const_fn(source.def_id) {
// Don't run on const functions, as, again, trans might not be able to evaluate
// the optimized IR.
return
}
}
}

let lhs_cast = if adt_def.is_enum() {
Place::Projection(Box::new(PlaceProjection {
base: lhs.clone(),
elem: ProjectionElem::Downcast(adt_def, variant),
}))
let (basic_blocks, local_decls) = mir.basic_blocks_and_local_decls_mut();
let local_decls = &*local_decls;
for bb in basic_blocks {
bb.expand_statements(|stmt| {
// FIXME(eddyb) don't match twice on `stmt.kind` (post-NLL).
if let StatementKind::Assign(_, ref rhs) = stmt.kind {
if let Rvalue::Aggregate(ref kind, _) = *rhs {
// FIXME(#48193) Deaggregate arrays when it's cheaper to do so.
if let AggregateKind::Array(_) = **kind {
return None;
}
} else {
lhs.clone()
};

let lhs_proj = Place::Projection(Box::new(PlaceProjection {
base: lhs_cast,
elem: ProjectionElem::Field(Field::new(i), ty),
}));
let new_statement = Statement {
source_info: src_info,
kind: StatementKind::Assign(lhs_proj, rhs),
};
debug!("inserting: {:?} @ {:?}", new_statement, idx + i);
bb.statements.push(new_statement);
return None;
}
} else {
return None;
}

// if the aggregate was an enum, we need to set the discriminant
if adt_def.is_enum() {
let set_discriminant = Statement {
kind: StatementKind::SetDiscriminant {
place: lhs.clone(),
variant_index: variant,
},
source_info: src_info,
};
bb.statements.push(set_discriminant);
let stmt = stmt.replace_nop();
let source_info = stmt.source_info;
let (mut lhs, kind, operands) = match stmt.kind {
StatementKind::Assign(lhs, Rvalue::Aggregate(kind, operands))
=> (lhs, kind, operands),
_ => bug!()
};

curr = bb.statements.len();
bb.statements.extend(suffix_stmts);
}
}
}
}
let mut set_discriminant = None;
let active_field_index = match *kind {
AggregateKind::Adt(adt_def, variant_index, _, active_field_index) => {
if adt_def.is_enum() {
set_discriminant = Some(Statement {
kind: StatementKind::SetDiscriminant {
place: lhs.clone(),
variant_index,
},
source_info,
});
lhs = lhs.downcast(adt_def, variant_index);
}
active_field_index
}
_ => None
};

fn get_aggregate_statement_index<'a, 'tcx, 'b>(start: usize,
statements: &Vec<Statement<'tcx>>)
-> Option<usize> {
for i in start..statements.len() {
let ref statement = statements[i];
let rhs = match statement.kind {
StatementKind::Assign(_, ref rhs) => rhs,
_ => continue,
};
let (kind, operands) = match rhs {
&Rvalue::Aggregate(ref kind, ref operands) => (kind, operands),
_ => continue,
};
let (adt_def, variant) = match **kind {
AggregateKind::Adt(adt_def, variant, _, None) => (adt_def, variant),
_ => continue,
};
if operands.len() == 0 {
// don't deaggregate ()
continue;
Some(operands.into_iter().enumerate().map(move |(i, op)| {
let lhs_field = if let AggregateKind::Array(_) = *kind {
// FIXME(eddyb) `offset` should be u64.
let offset = i as u32;
assert_eq!(offset as usize, i);
lhs.clone().elem(ProjectionElem::ConstantIndex {
offset,
// FIXME(eddyb) `min_length` doesn't appear to be used.
min_length: offset + 1,
from_end: false
})
} else {
let ty = op.ty(local_decls, tcx);
let field = Field::new(active_field_index.unwrap_or(i));
lhs.clone().field(field, ty)
};
Statement {
source_info,
kind: StatementKind::Assign(lhs_field, Rvalue::Use(op)),
}
}).chain(set_discriminant))
});
}
debug!("getting variant {:?}", variant);
debug!("for adt_def {:?}", adt_def);
return Some(i);
};
None
}
}
6 changes: 5 additions & 1 deletion src/librustc_mir/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,18 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx

// Optimizations begin.
inline::Inline,

// Lowering generator control-flow and variables
// has to happen before we do anything else to them.
generator::StateTransform,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to lower generator as late as possible, in particular we want to run at least SimplifyLocals before the StateTransform. Did you move this up because you triggered the sanity checks in StateTransform?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, those sanity checks don't survive past optimizations.

instcombine::InstCombine,
deaggregator::Deaggregator,
copy_prop::CopyPropagation,
remove_noop_landing_pads::RemoveNoopLandingPads,
simplify::SimplifyCfg::new("final"),
simplify::SimplifyLocals,

generator::StateTransform,
add_call_guards::CriticalCallEdges,
dump_mir::Marker("PreTrans"),
];
Expand Down
17 changes: 13 additions & 4 deletions src/librustc_mir/transform/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use rustc::ty::TyCtxt;
use rustc::mir::*;
use rustc::mir::visit::{MutVisitor, Visitor, PlaceContext};
use rustc::session::config::FullDebugInfo;
use std::borrow::Cow;
use transform::{MirPass, MirSource};

Expand Down Expand Up @@ -281,16 +282,24 @@ pub struct SimplifyLocals;

impl MirPass for SimplifyLocals {
fn run_pass<'a, 'tcx>(&self,
_: TyCtxt<'a, 'tcx, 'tcx>,
tcx: TyCtxt<'a, 'tcx, 'tcx>,
_: MirSource,
mir: &mut Mir<'tcx>) {
let mut marker = DeclMarker { locals: BitVector::new(mir.local_decls.len()) };
marker.visit_mir(mir);
// Return pointer and arguments are always live
marker.locals.insert(0);
for idx in mir.args_iter() {
marker.locals.insert(idx.index());
marker.locals.insert(RETURN_PLACE.index());
for arg in mir.args_iter() {
marker.locals.insert(arg.index());
}

// We may need to keep dead user variables live for debuginfo.
if tcx.sess.opts.debuginfo == FullDebugInfo {
for local in mir.vars_iter() {
marker.locals.insert(local.index());
}
}

let map = make_local_map(&mut mir.local_decls, marker.locals);
// Update references to all vars and tmps now
LocalUpdater { map: map }.visit_mir(mir);
Expand Down
8 changes: 4 additions & 4 deletions src/test/codegen/lifetime_start_end.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ pub fn test() {
// CHECK: [[S_b:%[0-9]+]] = bitcast %"core::option::Option<i32>"** %b to i8*
// CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S_b]])

// CHECK: [[S__5:%[0-9]+]] = bitcast %"core::option::Option<i32>"* %_5 to i8*
// CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S__5]])
// CHECK: [[S__4:%[0-9]+]] = bitcast %"core::option::Option<i32>"* %_4 to i8*
// CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S__4]])

// CHECK: [[E_b:%[0-9]+]] = bitcast %"core::option::Option<i32>"** %b to i8*
// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E_b]])

// CHECK: [[E__5:%[0-9]+]] = bitcast %"core::option::Option<i32>"* %_5 to i8*
// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E__5]])
// CHECK: [[E__4:%[0-9]+]] = bitcast %"core::option::Option<i32>"* %_4 to i8*
// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E__4]])
}

let c = 1;
Expand Down
6 changes: 3 additions & 3 deletions src/test/codegen/match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub enum E {

// CHECK-LABEL: @exhaustive_match
#[no_mangle]
pub fn exhaustive_match(e: E) {
pub fn exhaustive_match(e: E, unit: ()) {
// CHECK: switch{{.*}}, label %[[OTHERWISE:[a-zA-Z0-9_]+]] [
// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[A:[a-zA-Z0-9_]+]]
// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[B:[a-zA-Z0-9_]+]]
Expand All @@ -31,7 +31,7 @@ pub fn exhaustive_match(e: E) {
// CHECK: [[OTHERWISE]]:
// CHECK-NEXT: unreachable
match e {
E::A => (),
E::B => (),
E::A => unit,
E::B => unit,
}
}
2 changes: 1 addition & 1 deletion src/test/incremental/hashes/closure_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub fn change_parameter_pattern() {
}

#[cfg(not(cfail1))]
#[rustc_clean(cfg="cfail2", except="HirBody, MirValidated, MirOptimized, TypeckTables")]
#[rustc_clean(cfg="cfail2", except="HirBody, MirValidated, TypeckTables")]
#[rustc_clean(cfg="cfail3")]
pub fn change_parameter_pattern() {
let _ = |&x: &u32| x;
Expand Down
Loading