Rollup merge of rust-lang#55244 - wesleywiser:issue-50411, r=nikomatsakis

Don't rerun MIR passes when inlining

Fixes rust-lang#50411

r? @nikomatsakis

I updated your commit message with additional details. Let me know if any of that is incorrect. I also added the appropriate `compile-flags` directive to the test.

Thanks for you help on this!

cc @RalfJung related to your PR rust-lang#55086

Author: kennytm

src/librustc/mir/mod.rs

@@ -69,13 +69,38 @@ 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 = 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 {
+        *self as usize
+    }
+}
+
 /// Lowered representation of a single function.
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
 pub struct Mir<'tcx> {
     /// List of basic blocks. References to basic block use a newtyped index type `BasicBlock`
     /// that indexes into this vector.
     basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,
 
+    /// 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<SourceScope, SourceScopeData>,
@@ -151,6 +176,7 @@ impl<'tcx> Mir<'tcx> {
         );
 
         Mir {
+            phase: MirPhase::Build,
             basic_blocks,
             source_scopes,
             source_scope_local_data,
@@ -368,6 +394,7 @@ pub enum Safety {
 }
 
 impl_stable_hash_for!(struct Mir<'tcx> {
+    phase,
     basic_blocks,
     source_scopes,
     source_scope_local_data,
@@ -616,6 +643,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};
@@ -2905,6 +2939,7 @@ pub enum ClosureOutlivesSubject<'tcx> {
 
 CloneTypeFoldableAndLiftImpls! {
     BlockTailInfo,
+    MirPhase,
     Mutability,
     SourceInfo,
     UpvarDecl,
@@ -2917,6 +2952,7 @@ CloneTypeFoldableAndLiftImpls! {
 
 BraceStructTypeFoldableImpl! {
     impl<'tcx> TypeFoldable<'tcx> for Mir<'tcx> {
+        phase,
         basic_blocks,
         source_scopes,
         source_scope_local_data,

src/librustc_mir/dataflow/impls/borrows.rs

@@ -184,7 +184,6 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
     }
 
     crate fn borrows(&self) -> &IndexVec<BorrowIndex, BorrowData<'tcx>> { &self.borrow_set.borrows }
-    pub fn scope_tree(&self) -> &Lrc<region::ScopeTree> { &self.scope_tree }
 
     pub fn location(&self, idx: BorrowIndex) -> &Location {
         &self.borrow_set.borrows[idx].reserve_location

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<mir::BasicBlock>,
-                         sets: AllSets<D::Idx>,
-                         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

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,53 +155,69 @@ pub trait MirPass {
                           mir: &mut Mir<'tcx>);
 }
 
-pub macro run_passes($tcx:ident, $mir:ident, $def_id:ident, $suite_index:expr; $($pass:expr,)*) {{
-    let suite_index: usize = $suite_index;
-    let run_passes = |mir: &mut _, promoted| {
+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}", 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);
-            pass.run_pass($tcx, source, mir);
+            pass.run_pass(tcx, source, mir);
             run_hooks(mir, index, true);
 
             index += 1;
         };
-        $(run_pass(&$pass);)*
+
+        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<Mir<'tcx>> {
     // 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, 0;
+    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)
 }
 
@@ -214,11 +230,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, 1;
+    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)
 }
 
@@ -232,59 +248,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, 2;
+    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,
+        &copy_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)
 }

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();
+}