Skip to content

Commit

Permalink
Auto merge of #48052 - eddyb:deggregate, r=<try>
Browse files Browse the repository at this point in the history
 rustc_mir: handle all aggregate kinds in, and always run, the deaggregator.

This helps with removing`Rvalue::Aggregate` from the MIR, and with enabling more optimizations.
r? @nikomatsakis
  • Loading branch information
bors committed Feb 7, 2018
2 parents 4f93357 + 5cc4b94 commit 7d831d0
Show file tree
Hide file tree
Showing 10 changed files with 109 additions and 109 deletions.
171 changes: 79 additions & 92 deletions src/librustc_mir/transform/deaggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,116 +21,103 @@ 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) {
(hir::BodyOwnerKind::Fn, None) => {},
_ => return
(_, Some(_)) |
(hir::BodyOwnerKind::Const, _) |
(hir::BodyOwnerKind::Static(_), _) => return,

(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
}
}
}
// In fact, we might not want to trigger in other cases.
// Ex: when we could use SROA. See issue #35259

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 can_deaggregate = |statement: &Statement| {
if let StatementKind::Assign(_, ref rhs) = statement.kind {
if let Rvalue::Aggregate(..) = *rhs {
return true;
}
}

false
};

let (basic_blocks, local_decls) = mir.basic_blocks_and_local_decls_mut();
for bb in basic_blocks {
let mut start = 0;
while let Some(i) = bb.statements[start..].iter().position(&can_deaggregate) {
let i = start + i;

// FIXME(eddyb) this is probably more expensive than it should be.
// Ideally we'd move the block's statements all at once.
let suffix_stmts = bb.statements.split_off(i + 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 source_info = orig_stmt.source_info;
let (mut lhs, kind, operands) = match orig_stmt.kind {
StatementKind::Assign(lhs, Rvalue::Aggregate(kind, operands))
=> (lhs, kind, operands),
_ => bug!()
};
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 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
};
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());

let lhs_cast = if adt_def.is_enum() {
Place::Projection(Box::new(PlaceProjection {
base: lhs.clone(),
elem: ProjectionElem::Downcast(adt_def, variant),
}))
} else {
lhs.clone()
};
let new_total_count = bb.statements.len() +
operands.len() +
(set_discriminant.is_some() as usize) +
suffix_stmts.len();
bb.statements.reserve(new_total_count);

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),
for (j, op) in operands.into_iter().enumerate() {
let lhs_field = if let AggregateKind::Array(_) = *kind {
// FIXME(eddyb) `offset` should be u64.
let offset = j as u32;
assert_eq!(offset as usize, j);
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(j));
lhs.clone().field(field, ty)
};
debug!("inserting: {:?} @ {:?}", new_statement, idx + i);
bb.statements.push(new_statement);
bb.statements.push(Statement {
source_info,
kind: StatementKind::Assign(lhs_field, Rvalue::Use(op)),
});
}

// 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);
};
// If the aggregate was an enum, we need to set the discriminant.
bb.statements.extend(set_discriminant);

curr = bb.statements.len();
start = bb.statements.len();
bb.statements.extend(suffix_stmts);
}
}
}
}

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;
}
debug!("getting variant {:?}", variant);
debug!("for adt_def {:?}", adt_def);
return Some(i);
};
None
}
5 changes: 5 additions & 0 deletions src/librustc_mir/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,11 @@ 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,

instcombine::InstCombine,
deaggregator::Deaggregator,
copy_prop::CopyPropagation,
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
2 changes: 1 addition & 1 deletion src/test/incremental/issue-38222.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#![feature(rustc_attrs)]


#![rustc_partition_translated(module="issue_38222-mod1", cfg="rpass2")]
#![rustc_partition_reused(module="issue_38222-mod1", cfg="rpass2")]

// If trans had added a dependency edge to the Krate dep-node, nothing would
// be re-used, so checking that this module was re-used is sufficient.
Expand Down
2 changes: 0 additions & 2 deletions src/test/mir-opt/copy_propagation_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ fn main() {
// bb1: {
// StorageDead(_3);
// _1 = const 5u8;
// _0 = ();
// return;
// }
// END rustc.bar.CopyPropagation.before.mir
Expand All @@ -100,7 +99,6 @@ fn main() {
// _2 = _1;
// _1 = move _2;
// StorageDead(_2);
// _0 = ();
// return;
// }
// END rustc.baz.CopyPropagation.before.mir
Expand Down
3 changes: 2 additions & 1 deletion src/test/mir-opt/deaggregator_test_multiple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ fn main() {
// ((_4 as A).0: i32) = move _5;
// discriminant(_4) = 0;
// ...
// _0 = [move _2, move _4];
// _0[0 of 1] = move _2;
// _0[1 of 2] = move _4;
// ...
// return;
// }
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/print_type_sizes/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub fn f1<T:Copy>(x: T) {
fn start(_: isize, _: *const *const u8) -> isize {
let _b: Pair<u8> = Pair::new(0, 0);
let _s: Pair<SevenBytes> = Pair::new(SevenBytes::new(), SevenBytes::new());
let _z: ZeroSized = ZeroSized;
let ref _z: ZeroSized = ZeroSized;
f1::<SevenBytes>(SevenBytes::new());
0
}

0 comments on commit 7d831d0

Please sign in to comment.