From 37e1d2975e1002f0718552554055647392e46f0d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 20 Oct 2018 16:18:17 -0400 Subject: [PATCH 1/4] Don't rerun Mir passes when inlining When inlining a function using the Mir inliner, we shouldn't rerun the various Mir passes on it because the Mir has already been lowered and that wil break various early Mir passes. The issue in #50411 is that we've inlined a function with promotions whose Mir has already been lowered. The promotions are then copied into the local function and we begin to run passes on their lowered Mir which causes the ICE. Fixes #50411 --- src/librustc/mir/mod.rs | 29 +++++++++++++++++++++++++++++ src/librustc_mir/transform/mod.rs | 25 ++++++++++++++++++++----- src/test/ui/issues/issue-50411.rs | 11 +++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/issues/issue-50411.rs diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 34fc81a495e24..797836f166173 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -69,6 +69,17 @@ impl<'tcx> HasLocalDecls<'tcx> for Mir<'tcx> { } } +/// The various "big phases" that MIR goes through. +/// +/// Warning: ordering of variants is significant +#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum MirPhase { + Build, + Const, + Validated, + Optimized, +} + /// Lowered representation of a single function. #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct Mir<'tcx> { @@ -76,6 +87,13 @@ pub struct Mir<'tcx> { /// that indexes into this vector. basic_blocks: IndexVec>, + /// Records how far through the "desugaring and optimization" process this particular + /// MIR has traversed. This is particularly useful when inlining, since in that context + /// we instantiate the promoted constants and add them to our promoted vector -- but those + /// promoted items have already been optimized, whereas ours have not. This field allows + /// us to see the difference and forego optimization on the inlined promoted items. + pub phase: MirPhase, + /// List of source scopes; these are referenced by statements /// and used for debuginfo. Indexed by a `SourceScope`. pub source_scopes: IndexVec, @@ -151,6 +169,7 @@ impl<'tcx> Mir<'tcx> { ); Mir { + phase: MirPhase::Build, basic_blocks, source_scopes, source_scope_local_data, @@ -368,6 +387,7 @@ pub enum Safety { } impl_stable_hash_for!(struct Mir<'tcx> { + phase, basic_blocks, source_scopes, source_scope_local_data, @@ -616,6 +636,13 @@ impl_stable_hash_for!(enum self::ImplicitSelfKind { None }); +impl_stable_hash_for!(enum self::MirPhase { + Build, + Const, + Validated, + Optimized, +}); + mod binding_form_impl { use ich::StableHashingContext; use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult}; @@ -2777,6 +2804,7 @@ pub enum ClosureOutlivesSubject<'tcx> { CloneTypeFoldableAndLiftImpls! { BlockTailInfo, + MirPhase, Mutability, SourceInfo, UpvarDecl, @@ -2789,6 +2817,7 @@ CloneTypeFoldableAndLiftImpls! { BraceStructTypeFoldableImpl! { impl<'tcx> TypeFoldable<'tcx> for Mir<'tcx> { + phase, basic_blocks, source_scopes, source_scope_local_data, diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index d18836999dccf..61e150ea12a22 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -11,7 +11,7 @@ use borrow_check::nll::type_check; use build; use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; -use rustc::mir::{Mir, Promoted}; +use rustc::mir::{Mir, MirPhase, Promoted}; use rustc::ty::TyCtxt; use rustc::ty::query::Providers; use rustc::ty::steal::Steal; @@ -155,9 +155,22 @@ pub trait MirPass { mir: &mut Mir<'tcx>); } -pub macro run_passes($tcx:ident, $mir:ident, $def_id:ident, $suite_index:expr; $($pass:expr,)*) {{ +pub macro run_passes( + $tcx:ident, + $mir:ident, + $def_id:ident, + $suite_index:expr, + $mir_phase:expr; + $($pass:expr,)* +) {{ let suite_index: usize = $suite_index; let run_passes = |mir: &mut _, promoted| { + let mir: &mut Mir<'_> = mir; + + if mir.phase >= $mir_phase { + return; + } + let source = MirSource { def_id: $def_id, promoted @@ -175,6 +188,8 @@ pub macro run_passes($tcx:ident, $mir:ident, $def_id:ident, $suite_index:expr; $ index += 1; }; $(run_pass(&$pass);)* + + mir.phase = $mir_phase; }; run_passes(&mut $mir, None); @@ -192,7 +207,7 @@ fn mir_const<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx Stea let _ = tcx.unsafety_check_result(def_id); let mut mir = tcx.mir_built(def_id).steal(); - run_passes![tcx, mir, def_id, 0; + run_passes![tcx, mir, def_id, 0, MirPhase::Const; // Remove all `EndRegion` statements that are not involved in borrows. cleanup_post_borrowck::CleanEndRegions, @@ -214,7 +229,7 @@ fn mir_validated<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx } let mut mir = tcx.mir_const(def_id).steal(); - run_passes![tcx, mir, def_id, 1; + run_passes![tcx, mir, def_id, 1, MirPhase::Validated; // What we need to run borrowck etc. qualify_consts::QualifyAndPromoteConstants, simplify::SimplifyCfg::new("qualify-consts"), @@ -232,7 +247,7 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx } let mut mir = tcx.mir_validated(def_id).steal(); - run_passes![tcx, mir, def_id, 2; + run_passes![tcx, mir, def_id, 2, MirPhase::Optimized; // Remove all things not needed by analysis no_landing_pads::NoLandingPads, simplify_branches::SimplifyBranches::new("initial"), diff --git a/src/test/ui/issues/issue-50411.rs b/src/test/ui/issues/issue-50411.rs new file mode 100644 index 0000000000000..1ba47d3b932ef --- /dev/null +++ b/src/test/ui/issues/issue-50411.rs @@ -0,0 +1,11 @@ +// Regression test for #50411: the MIR inliner was causing problems +// here because it would inline promoted code (which had already had +// elaborate-drops invoked on it) and then try to elaboate drops a +// second time. Uncool. + +// compile-flags:-Zmir-opt-level=3 +// compile-pass + +fn main() { + let _ = (0 .. 1).filter(|_| [1].iter().all(|_| true)).count(); +} From 895a4b2d45e6d5c274bcd561391b2cc6a73dd2c5 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sun, 21 Oct 2018 10:47:01 -0400 Subject: [PATCH 2/4] Remove the `suite_index` parameter of the `run_passes!()` macro This can be obtained via the `$mir_phase` value. --- src/librustc/mir/mod.rs | 12 ++++++++++++ src/librustc_mir/transform/mod.rs | 12 ++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 797836f166173..e53def65886de 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -80,6 +80,18 @@ pub enum MirPhase { Optimized, } +impl MirPhase { + /// Gets the index of the current MirPhase within the set of all MirPhases. + pub fn phase_index(&self) -> usize { + match self { + MirPhase::Build => 0, + MirPhase::Const => 1, + MirPhase::Validated => 2, + MirPhase::Optimized => 3, + } + } +} + /// Lowered representation of a single function. #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct Mir<'tcx> { diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 61e150ea12a22..ff85d780a4ce3 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -159,11 +159,11 @@ pub macro run_passes( $tcx:ident, $mir:ident, $def_id:ident, - $suite_index:expr, $mir_phase:expr; $($pass:expr,)* ) {{ - let suite_index: usize = $suite_index; + let phase_index = $mir_phase.phase_index(); + let run_passes = |mir: &mut _, promoted| { let mir: &mut Mir<'_> = mir; @@ -178,7 +178,7 @@ pub macro run_passes( let mut index = 0; let mut run_pass = |pass: &dyn MirPass| { let run_hooks = |mir: &_, index, is_after| { - dump_mir::on_mir_pass($tcx, &format_args!("{:03}-{:03}", suite_index, index), + dump_mir::on_mir_pass($tcx, &format_args!("{:03}-{:03}", phase_index, index), &pass.name(), source, mir, is_after); }; run_hooks(mir, index, false); @@ -207,7 +207,7 @@ fn mir_const<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx Stea let _ = tcx.unsafety_check_result(def_id); let mut mir = tcx.mir_built(def_id).steal(); - run_passes![tcx, mir, def_id, 0, MirPhase::Const; + run_passes![tcx, mir, def_id, MirPhase::Const; // Remove all `EndRegion` statements that are not involved in borrows. cleanup_post_borrowck::CleanEndRegions, @@ -229,7 +229,7 @@ fn mir_validated<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx } let mut mir = tcx.mir_const(def_id).steal(); - run_passes![tcx, mir, def_id, 1, MirPhase::Validated; + run_passes![tcx, mir, def_id, MirPhase::Validated; // What we need to run borrowck etc. qualify_consts::QualifyAndPromoteConstants, simplify::SimplifyCfg::new("qualify-consts"), @@ -247,7 +247,7 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx } let mut mir = tcx.mir_validated(def_id).steal(); - run_passes![tcx, mir, def_id, 2, MirPhase::Optimized; + run_passes![tcx, mir, def_id, MirPhase::Optimized; // Remove all things not needed by analysis no_landing_pads::NoLandingPads, simplify_branches::SimplifyBranches::new("initial"), From c535147f291fee0c79553eabcd520156707cd0d4 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 22 Oct 2018 22:41:21 -0400 Subject: [PATCH 3/4] Replace the `run_passes!` macro with a regular function As suggested in the feedback for #55244. When I replaced the macro with a function, rustc started complaining that there were two unused functions so I also removed those. --- src/librustc_mir/dataflow/impls/borrows.rs | 1 - src/librustc_mir/dataflow/mod.rs | 14 --- src/librustc_mir/transform/mod.rs | 134 ++++++++++----------- 3 files changed, 67 insertions(+), 82 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index f7043487c51a6..cfccb950e8276 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -184,7 +184,6 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { } crate fn borrows(&self) -> &IndexVec> { &self.borrow_set.borrows } - pub fn scope_tree(&self) -> &Lrc { &self.scope_tree } pub fn location(&self, idx: BorrowIndex) -> &Location { &self.borrow_set.borrows[idx].reserve_location diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index da4bd780eb4fa..c19145636e6da 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -724,20 +724,6 @@ impl<'a, 'tcx, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation } } } - - pub fn new_from_sets(mir: &'a Mir<'tcx>, - dead_unwinds: &'a BitSet, - sets: AllSets, - denotation: D) -> Self { - DataflowAnalysis { - mir, - dead_unwinds, - flow_state: DataflowState { - sets: sets, - operator: denotation, - } - } - } } impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index ff85d780a4ce3..28b7d517da6c9 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -155,68 +155,68 @@ pub trait MirPass { mir: &mut Mir<'tcx>); } -pub macro run_passes( - $tcx:ident, - $mir:ident, - $def_id:ident, - $mir_phase:expr; - $($pass:expr,)* -) {{ - let phase_index = $mir_phase.phase_index(); - - let run_passes = |mir: &mut _, promoted| { - let mir: &mut Mir<'_> = mir; - - if mir.phase >= $mir_phase { +pub fn run_passes( + tcx: TyCtxt<'a, 'tcx, 'tcx>, + mir: &mut Mir<'tcx>, + def_id: DefId, + mir_phase: MirPhase, + passes: &[&dyn MirPass]) { + let phase_index = mir_phase.phase_index(); + + let run_passes = |mir: &mut Mir<'tcx>, promoted| { + if mir.phase >= mir_phase { return; } let source = MirSource { - def_id: $def_id, - promoted + def_id, + promoted, }; let mut index = 0; let mut run_pass = |pass: &dyn MirPass| { let run_hooks = |mir: &_, index, is_after| { - dump_mir::on_mir_pass($tcx, &format_args!("{:03}-{:03}", phase_index, index), + dump_mir::on_mir_pass(tcx, &format_args!("{:03}-{:03}", phase_index, index), &pass.name(), source, mir, is_after); }; run_hooks(mir, index, false); - pass.run_pass($tcx, source, mir); + pass.run_pass(tcx, source, mir); run_hooks(mir, index, true); index += 1; }; - $(run_pass(&$pass);)* - mir.phase = $mir_phase; + for pass in passes { + run_pass(*pass); + } + + mir.phase = mir_phase; }; - run_passes(&mut $mir, None); + run_passes(mir, None); - for (index, promoted_mir) in $mir.promoted.iter_enumerated_mut() { + for (index, promoted_mir) in mir.promoted.iter_enumerated_mut() { run_passes(promoted_mir, Some(index)); - // Let's make sure we don't miss any nested instances - assert!(promoted_mir.promoted.is_empty()); + //Let's make sure we don't miss any nested instances + assert!(promoted_mir.promoted.is_empty()) } -}} +} fn mir_const<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx Steal> { // Unsafety check uses the raw mir, so make sure it is run let _ = tcx.unsafety_check_result(def_id); let mut mir = tcx.mir_built(def_id).steal(); - run_passes![tcx, mir, def_id, MirPhase::Const; + run_passes(tcx, &mut mir, def_id, MirPhase::Const, &[ // Remove all `EndRegion` statements that are not involved in borrows. - cleanup_post_borrowck::CleanEndRegions, + &cleanup_post_borrowck::CleanEndRegions, // What we need to do constant evaluation. - simplify::SimplifyCfg::new("initial"), - type_check::TypeckMir, - rustc_peek::SanityCheck, - uniform_array_move_out::UniformArrayMoveOut, - ]; + &simplify::SimplifyCfg::new("initial"), + &type_check::TypeckMir, + &rustc_peek::SanityCheck, + &uniform_array_move_out::UniformArrayMoveOut, + ]); tcx.alloc_steal_mir(mir) } @@ -229,11 +229,11 @@ fn mir_validated<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx } let mut mir = tcx.mir_const(def_id).steal(); - run_passes![tcx, mir, def_id, MirPhase::Validated; + run_passes(tcx, &mut mir, def_id, MirPhase::Validated, &[ // What we need to run borrowck etc. - qualify_consts::QualifyAndPromoteConstants, - simplify::SimplifyCfg::new("qualify-consts"), - ]; + &qualify_consts::QualifyAndPromoteConstants, + &simplify::SimplifyCfg::new("qualify-consts"), + ]); tcx.alloc_steal_mir(mir) } @@ -247,59 +247,59 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx } let mut mir = tcx.mir_validated(def_id).steal(); - run_passes![tcx, mir, def_id, MirPhase::Optimized; + run_passes(tcx, &mut mir, def_id, MirPhase::Optimized, &[ // Remove all things not needed by analysis - no_landing_pads::NoLandingPads, - simplify_branches::SimplifyBranches::new("initial"), - remove_noop_landing_pads::RemoveNoopLandingPads, + &no_landing_pads::NoLandingPads, + &simplify_branches::SimplifyBranches::new("initial"), + &remove_noop_landing_pads::RemoveNoopLandingPads, // Remove all `AscribeUserType` statements. - cleanup_post_borrowck::CleanAscribeUserType, + &cleanup_post_borrowck::CleanAscribeUserType, // Remove all `FakeRead` statements and the borrows that are only // used for checking matches - cleanup_post_borrowck::CleanFakeReadsAndBorrows, - simplify::SimplifyCfg::new("early-opt"), + &cleanup_post_borrowck::CleanFakeReadsAndBorrows, + &simplify::SimplifyCfg::new("early-opt"), // These next passes must be executed together - add_call_guards::CriticalCallEdges, - elaborate_drops::ElaborateDrops, - no_landing_pads::NoLandingPads, + &add_call_guards::CriticalCallEdges, + &elaborate_drops::ElaborateDrops, + &no_landing_pads::NoLandingPads, // AddValidation needs to run after ElaborateDrops and before EraseRegions, and it needs // an AllCallEdges pass right before it. - add_call_guards::AllCallEdges, - add_validation::AddValidation, + &add_call_guards::AllCallEdges, + &add_validation::AddValidation, // AddMovesForPackedDrops needs to run after drop // elaboration. - add_moves_for_packed_drops::AddMovesForPackedDrops, + &add_moves_for_packed_drops::AddMovesForPackedDrops, - simplify::SimplifyCfg::new("elaborate-drops"), + &simplify::SimplifyCfg::new("elaborate-drops"), // No lifetime analysis based on borrowing can be done from here on out. // From here on out, regions are gone. - erase_regions::EraseRegions, + &erase_regions::EraseRegions, - lower_128bit::Lower128Bit, + &lower_128bit::Lower128Bit, // Optimizations begin. - uniform_array_move_out::RestoreSubsliceArrayMoveOut, - inline::Inline, + &uniform_array_move_out::RestoreSubsliceArrayMoveOut, + &inline::Inline, // Lowering generator control-flow and variables // has to happen before we do anything else to them. - generator::StateTransform, - - instcombine::InstCombine, - const_prop::ConstProp, - simplify_branches::SimplifyBranches::new("after-const-prop"), - deaggregator::Deaggregator, - copy_prop::CopyPropagation, - remove_noop_landing_pads::RemoveNoopLandingPads, - simplify::SimplifyCfg::new("final"), - simplify::SimplifyLocals, - - add_call_guards::CriticalCallEdges, - dump_mir::Marker("PreCodegen"), - ]; + &generator::StateTransform, + + &instcombine::InstCombine, + &const_prop::ConstProp, + &simplify_branches::SimplifyBranches::new("after-const-prop"), + &deaggregator::Deaggregator, + ©_prop::CopyPropagation, + &remove_noop_landing_pads::RemoveNoopLandingPads, + &simplify::SimplifyCfg::new("final"), + &simplify::SimplifyLocals, + + &add_call_guards::CriticalCallEdges, + &dump_mir::Marker("PreCodegen"), + ]); tcx.alloc_mir(mir) } From 4655866a11ddb345c84781cf4e292e99e26ee9fe Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 25 Oct 2018 08:35:53 -0400 Subject: [PATCH 4/4] Fix CR feedback --- src/librustc/mir/mod.rs | 15 +++++---------- src/librustc_mir/transform/mod.rs | 3 ++- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index e53def65886de..f9da40b583899 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -74,21 +74,16 @@ impl<'tcx> HasLocalDecls<'tcx> for Mir<'tcx> { /// Warning: ordering of variants is significant #[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum MirPhase { - Build, - Const, - Validated, - Optimized, + Build = 0, + Const = 1, + Validated = 2, + Optimized = 3, } impl MirPhase { /// Gets the index of the current MirPhase within the set of all MirPhases. pub fn phase_index(&self) -> usize { - match self { - MirPhase::Build => 0, - MirPhase::Const => 1, - MirPhase::Validated => 2, - MirPhase::Optimized => 3, - } + *self as usize } } diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 28b7d517da6c9..46c73c27fe10d 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -160,7 +160,8 @@ pub fn run_passes( mir: &mut Mir<'tcx>, def_id: DefId, mir_phase: MirPhase, - passes: &[&dyn MirPass]) { + passes: &[&dyn MirPass], +) { let phase_index = mir_phase.phase_index(); let run_passes = |mir: &mut Mir<'tcx>, promoted| {