From 3922dd72fe98ecc8866d448fe1e637971a46341a Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 7 Feb 2018 14:36:03 +0200 Subject: [PATCH 1/7] rustc_mir: use the "idiomatic" optimization gating in the deaggregator. --- src/librustc_mir/transform/deaggregator.rs | 27 +++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/transform/deaggregator.rs b/src/librustc_mir/transform/deaggregator.rs index eccb0d231b89d..ea27d789248b0 100644 --- a/src/librustc_mir/transform/deaggregator.rs +++ b/src/librustc_mir/transform/deaggregator.rs @@ -21,23 +21,28 @@ 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 + } + } + } + + // We only run when the MIR optimization level is > 2. + if tcx.sess.opts.debugging_opts.mir_opt_level <= 2 { + 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; From b88180f74c39cb57dc7426f9ad3be813f356ecee Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 7 Feb 2018 15:27:00 +0200 Subject: [PATCH 2/7] rustc_mir: handle all aggregate kinds in the deaggregator. --- src/librustc_mir/transform/deaggregator.rs | 149 ++++++++---------- src/test/mir-opt/copy_propagation_arg.rs | 2 - .../mir-opt/deaggregator_test_multiple.rs | 3 +- 3 files changed, 70 insertions(+), 84 deletions(-) diff --git a/src/librustc_mir/transform/deaggregator.rs b/src/librustc_mir/transform/deaggregator.rs index ea27d789248b0..179c196f26f56 100644 --- a/src/librustc_mir/transform/deaggregator.rs +++ b/src/librustc_mir/transform/deaggregator.rs @@ -44,98 +44,85 @@ impl MirPass for Deaggregator { 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 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>) - -> Option { - 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 -} diff --git a/src/test/mir-opt/copy_propagation_arg.rs b/src/test/mir-opt/copy_propagation_arg.rs index 35bb231df5adf..9e4c06b7d7b23 100644 --- a/src/test/mir-opt/copy_propagation_arg.rs +++ b/src/test/mir-opt/copy_propagation_arg.rs @@ -78,7 +78,6 @@ fn main() { // bb1: { // StorageDead(_3); // _1 = const 5u8; -// _0 = (); // return; // } // END rustc.bar.CopyPropagation.before.mir @@ -100,7 +99,6 @@ fn main() { // _2 = _1; // _1 = move _2; // StorageDead(_2); -// _0 = (); // return; // } // END rustc.baz.CopyPropagation.before.mir diff --git a/src/test/mir-opt/deaggregator_test_multiple.rs b/src/test/mir-opt/deaggregator_test_multiple.rs index 3a9a458fd464d..5127ed5885371 100644 --- a/src/test/mir-opt/deaggregator_test_multiple.rs +++ b/src/test/mir-opt/deaggregator_test_multiple.rs @@ -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; // } From e598bdfaa017e3bf786c19587ea917c6a8aa984e Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 7 Feb 2018 17:28:07 +0200 Subject: [PATCH 3/7] rustc_mir: do not remove dead user variables if debuginfo needs them. --- src/librustc_mir/transform/simplify.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/transform/simplify.rs b/src/librustc_mir/transform/simplify.rs index e7675b4ceaf29..2c6ed1f19b7fd 100644 --- a/src/librustc_mir/transform/simplify.rs +++ b/src/librustc_mir/transform/simplify.rs @@ -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}; @@ -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); From 6e5dacbd5e8aab43cdc4c2f1d4ec66fe0b8d4bed Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 7 Feb 2018 17:28:32 +0200 Subject: [PATCH 4/7] rustc_mir: always run the deaggregator. --- src/librustc_mir/transform/deaggregator.rs | 5 ----- src/librustc_mir/transform/mod.rs | 6 +++++- src/test/codegen/lifetime_start_end.rs | 8 ++++---- src/test/codegen/match.rs | 6 +++--- src/test/incremental/hashes/closure_expressions.rs | 2 +- src/test/incremental/issue-38222.rs | 2 +- src/test/ui/print_type_sizes/generics.rs | 2 +- 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/transform/deaggregator.rs b/src/librustc_mir/transform/deaggregator.rs index 179c196f26f56..44c35960abaf8 100644 --- a/src/librustc_mir/transform/deaggregator.rs +++ b/src/librustc_mir/transform/deaggregator.rs @@ -39,11 +39,6 @@ impl MirPass for Deaggregator { } } - // We only run when the MIR optimization level is > 2. - if tcx.sess.opts.debugging_opts.mir_opt_level <= 2 { - return; - } - let can_deaggregate = |statement: &Statement| { if let StatementKind::Assign(_, ref rhs) = statement.kind { if let Rvalue::Aggregate(..) = *rhs { diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 7ed250e94c52f..b16c1436a1cbe 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -258,6 +258,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, @@ -265,7 +270,6 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx simplify::SimplifyCfg::new("final"), simplify::SimplifyLocals, - generator::StateTransform, add_call_guards::CriticalCallEdges, dump_mir::Marker("PreTrans"), ]; diff --git a/src/test/codegen/lifetime_start_end.rs b/src/test/codegen/lifetime_start_end.rs index 1f900a3770eb3..62aa93398ac42 100644 --- a/src/test/codegen/lifetime_start_end.rs +++ b/src/test/codegen/lifetime_start_end.rs @@ -28,14 +28,14 @@ pub fn test() { // CHECK: [[S_b:%[0-9]+]] = bitcast %"core::option::Option"** %b to i8* // CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S_b]]) -// CHECK: [[S__5:%[0-9]+]] = bitcast %"core::option::Option"* %_5 to i8* -// CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S__5]]) +// CHECK: [[S__4:%[0-9]+]] = bitcast %"core::option::Option"* %_4 to i8* +// CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S__4]]) // CHECK: [[E_b:%[0-9]+]] = bitcast %"core::option::Option"** %b to i8* // CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E_b]]) -// CHECK: [[E__5:%[0-9]+]] = bitcast %"core::option::Option"* %_5 to i8* -// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E__5]]) +// CHECK: [[E__4:%[0-9]+]] = bitcast %"core::option::Option"* %_4 to i8* +// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E__4]]) } let c = 1; diff --git a/src/test/codegen/match.rs b/src/test/codegen/match.rs index 660b6346c57f1..c9d0427dd0ad4 100644 --- a/src/test/codegen/match.rs +++ b/src/test/codegen/match.rs @@ -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_]+]] @@ -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, } } diff --git a/src/test/incremental/hashes/closure_expressions.rs b/src/test/incremental/hashes/closure_expressions.rs index d8a87da5918a9..73418f4886070 100644 --- a/src/test/incremental/hashes/closure_expressions.rs +++ b/src/test/incremental/hashes/closure_expressions.rs @@ -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; diff --git a/src/test/incremental/issue-38222.rs b/src/test/incremental/issue-38222.rs index 7bb8af74eeb7e..f890672aa8f58 100644 --- a/src/test/incremental/issue-38222.rs +++ b/src/test/incremental/issue-38222.rs @@ -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. diff --git a/src/test/ui/print_type_sizes/generics.rs b/src/test/ui/print_type_sizes/generics.rs index d0e5bd1d92abf..21fdbb3f5a1cc 100644 --- a/src/test/ui/print_type_sizes/generics.rs +++ b/src/test/ui/print_type_sizes/generics.rs @@ -72,7 +72,7 @@ pub fn f1(x: T) { fn start(_: isize, _: *const *const u8) -> isize { let _b: Pair = Pair::new(0, 0); let _s: Pair = Pair::new(SevenBytes::new(), SevenBytes::new()); - let _z: ZeroSized = ZeroSized; + let ref _z: ZeroSized = ZeroSized; f1::(SevenBytes::new()); 0 } From 6195ad8654d713678460aca29b6f986655b988f1 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 7 Feb 2018 21:27:37 +0200 Subject: [PATCH 5/7] test: use the right amount of CGUs in sepcomp-cci-copies to ensure deterministic splitting. --- src/test/run-make/sepcomp-cci-copies/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/run-make/sepcomp-cci-copies/Makefile b/src/test/run-make/sepcomp-cci-copies/Makefile index ccd4e1b0e715a..36f913ff3faac 100644 --- a/src/test/run-make/sepcomp-cci-copies/Makefile +++ b/src/test/run-make/sepcomp-cci-copies/Makefile @@ -2,9 +2,11 @@ # Check that cross-crate inlined items are inlined in all compilation units # that refer to them, and not in any other compilation units. +# Note that we have to pass `-C codegen-units=6` because up to two CGUs may be +# created for each source module (see `rustc_mir::monomorphize::partitioning`). all: $(RUSTC) cci_lib.rs - $(RUSTC) foo.rs --emit=llvm-ir -C codegen-units=3 \ + $(RUSTC) foo.rs --emit=llvm-ir -C codegen-units=6 \ -Z inline-in-all-cgus [ "$$(cat "$(TMPDIR)"/foo.*.ll | grep -c define\ .*cci_fn)" -eq "2" ] From d773d95880b0866ce2bee4ab68ee6fa363235f84 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 14 Feb 2018 16:10:25 +0200 Subject: [PATCH 6/7] rustc_mir: don't run the deaggregator on arrays for now. --- src/librustc_mir/transform/deaggregator.rs | 6 +++++- src/test/mir-opt/deaggregator_test_multiple.rs | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/transform/deaggregator.rs b/src/librustc_mir/transform/deaggregator.rs index 44c35960abaf8..8ffcc4025f113 100644 --- a/src/librustc_mir/transform/deaggregator.rs +++ b/src/librustc_mir/transform/deaggregator.rs @@ -41,7 +41,11 @@ impl MirPass for Deaggregator { let can_deaggregate = |statement: &Statement| { if let StatementKind::Assign(_, ref rhs) = statement.kind { - if let Rvalue::Aggregate(..) = *rhs { + if let Rvalue::Aggregate(ref kind, _) = *rhs { + // FIXME(#48193) Deaggregate arrays when it's cheaper to do so. + if let AggregateKind::Array(_) = **kind { + return false; + } return true; } } diff --git a/src/test/mir-opt/deaggregator_test_multiple.rs b/src/test/mir-opt/deaggregator_test_multiple.rs index 5127ed5885371..3a9a458fd464d 100644 --- a/src/test/mir-opt/deaggregator_test_multiple.rs +++ b/src/test/mir-opt/deaggregator_test_multiple.rs @@ -52,8 +52,7 @@ fn main() { // ((_4 as A).0: i32) = move _5; // discriminant(_4) = 0; // ... -// _0[0 of 1] = move _2; -// _0[1 of 2] = move _4; +// _0 = [move _2, move _4]; // ... // return; // } From c9fcedeb4c30ef9362453c93ee2dc6655c7ed31a Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 16 Feb 2018 19:20:18 +0200 Subject: [PATCH 7/7] rustc_mir: optimize the deaggregator's expansion of statements. --- src/librustc/lib.rs | 1 + src/librustc/mir/mod.rs | 65 +++++++++++++++++++- src/librustc_mir/transform/deaggregator.rs | 69 ++++++++-------------- src/test/mir-opt/copy_propagation_arg.rs | 26 ++++---- 4 files changed, 104 insertions(+), 57 deletions(-) diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index a7a2619505931..2700ef28d671a 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -69,6 +69,7 @@ #![feature(underscore_lifetimes)] #![feature(universal_impl_trait)] #![feature(trace_macros)] +#![feature(trusted_len)] #![feature(catch_expr)] #![feature(test)] diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index b88dea871ce67..475946468fa35 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -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; @@ -984,11 +984,62 @@ impl<'tcx> BasicBlockData<'tcx> { pub fn retain_statements(&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(&mut self, mut f: F) + where F: FnMut(&mut Statement<'tcx>) -> Option, + I: iter::TrustedLen> + { + // 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] @@ -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)] diff --git a/src/librustc_mir/transform/deaggregator.rs b/src/librustc_mir/transform/deaggregator.rs index 8ffcc4025f113..503354ebc4ffd 100644 --- a/src/librustc_mir/transform/deaggregator.rs +++ b/src/librustc_mir/transform/deaggregator.rs @@ -39,32 +39,27 @@ impl MirPass for Deaggregator { } } - let can_deaggregate = |statement: &Statement| { - if let StatementKind::Assign(_, ref rhs) = statement.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 false; - } - return true; - } - } - - false - }; - let (basic_blocks, local_decls) = mir.basic_blocks_and_local_decls_mut(); + let local_decls = &*local_decls; for bb in basic_blocks { - let mut start = 0; - while let Some(i) = bb.statements[start..].iter().position(&can_deaggregate) { - let i = start + i; + 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 { + return None; + } + } else { + return None; + } - // 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 source_info = orig_stmt.source_info; - let (mut lhs, kind, operands) = match orig_stmt.kind { + 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!() @@ -88,17 +83,11 @@ impl MirPass for Deaggregator { _ => None }; - let new_total_count = bb.statements.len() + - operands.len() + - (set_discriminant.is_some() as usize) + - suffix_stmts.len(); - bb.statements.reserve(new_total_count); - - for (j, op) in operands.into_iter().enumerate() { + 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 = j as u32; - assert_eq!(offset as usize, j); + 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. @@ -107,21 +96,15 @@ impl MirPass for Deaggregator { }) } else { let ty = op.ty(local_decls, tcx); - let field = Field::new(active_field_index.unwrap_or(j)); + let field = Field::new(active_field_index.unwrap_or(i)); lhs.clone().field(field, ty) }; - bb.statements.push(Statement { + Statement { source_info, kind: StatementKind::Assign(lhs_field, Rvalue::Use(op)), - }); - } - - // If the aggregate was an enum, we need to set the discriminant. - bb.statements.extend(set_discriminant); - - start = bb.statements.len(); - bb.statements.extend(suffix_stmts); - } + } + }).chain(set_discriminant)) + }); } } } diff --git a/src/test/mir-opt/copy_propagation_arg.rs b/src/test/mir-opt/copy_propagation_arg.rs index 9e4c06b7d7b23..feadec6bbf76e 100644 --- a/src/test/mir-opt/copy_propagation_arg.rs +++ b/src/test/mir-opt/copy_propagation_arg.rs @@ -78,6 +78,7 @@ fn main() { // bb1: { // StorageDead(_3); // _1 = const 5u8; +// ... // return; // } // END rustc.bar.CopyPropagation.before.mir @@ -91,6 +92,7 @@ fn main() { // ... // _1 = const 5u8; // ... +// return; // } // END rustc.bar.CopyPropagation.after.mir // START rustc.baz.CopyPropagation.before.mir @@ -99,6 +101,7 @@ fn main() { // _2 = _1; // _1 = move _2; // StorageDead(_2); +// ... // return; // } // END rustc.baz.CopyPropagation.before.mir @@ -108,21 +111,22 @@ fn main() { // _2 = _1; // _1 = move _2; // ... +// return; // } // END rustc.baz.CopyPropagation.after.mir // START rustc.arg_src.CopyPropagation.before.mir // bb0: { -// ... -// _3 = _1; -// _2 = move _3; -// ... -// _1 = const 123i32; -// ... -// _4 = _2; -// _0 = move _4; -// ... -// return; -// } +// ... +// _3 = _1; +// _2 = move _3; +// ... +// _1 = const 123i32; +// ... +// _4 = _2; +// _0 = move _4; +// ... +// return; +// } // END rustc.arg_src.CopyPropagation.before.mir // START rustc.arg_src.CopyPropagation.after.mir // bb0: {