Auto merge of #131481 - nnethercote:rm-GenKillSet, r=cjgillot

Remove `GenKillAnalysis`

There are two kinds of dataflow analysis in the compiler: `Analysis`, which is the basic kind, and `GenKillAnalysis`, which is a more specialized kind for gen/kill analyses that is intended as an optimization. However, it turns out that `GenKillAnalysis` is actually a  pessimization! It's faster (and much simpler) to do all the gen/kill analyses via `Analysis`. This lets us remove `GenKillAnalysis`, and `GenKillSet`, and a few other things, and also merge `AnalysisDomain` into `Analysis`. The PR removes 500 lines of code and improves performance.

r? `@tmiasko`

Author: bors

compiler/rustc_borrowck/src/dataflow.rs

@@ -1,13 +1,11 @@
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::graph;
 use rustc_index::bit_set::BitSet;
-use rustc_middle::mir::{
-    self, BasicBlock, Body, CallReturnPlaces, Location, Place, TerminatorEdges,
-};
+use rustc_middle::mir::{self, BasicBlock, Body, Location, Place, TerminatorEdges};
 use rustc_middle::ty::{RegionVid, TyCtxt};
 use rustc_mir_dataflow::fmt::DebugWithContext;
 use rustc_mir_dataflow::impls::{EverInitializedPlaces, MaybeUninitializedPlaces};
-use rustc_mir_dataflow::{Analysis, AnalysisDomain, Forward, GenKill, Results, ResultsVisitable};
+use rustc_mir_dataflow::{Analysis, Forward, GenKill, Results, ResultsVisitable};
 use tracing::debug;
 
 use crate::{BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext, places_conflict};
@@ -22,9 +20,9 @@ pub(crate) struct BorrowckResults<'a, 'tcx> {
 /// The transient state of the dataflow analyses used by the borrow checker.
 #[derive(Debug)]
 pub(crate) struct BorrowckDomain<'a, 'tcx> {
-    pub(crate) borrows: <Borrows<'a, 'tcx> as AnalysisDomain<'tcx>>::Domain,
-    pub(crate) uninits: <MaybeUninitializedPlaces<'a, 'tcx> as AnalysisDomain<'tcx>>::Domain,
-    pub(crate) ever_inits: <EverInitializedPlaces<'a, 'tcx> as AnalysisDomain<'tcx>>::Domain,
+    pub(crate) borrows: <Borrows<'a, 'tcx> as Analysis<'tcx>>::Domain,
+    pub(crate) uninits: <MaybeUninitializedPlaces<'a, 'tcx> as Analysis<'tcx>>::Domain,
+    pub(crate) ever_inits: <EverInitializedPlaces<'a, 'tcx> as Analysis<'tcx>>::Domain,
 }
 
 impl<'a, 'tcx> ResultsVisitable<'tcx> for BorrowckResults<'a, 'tcx> {
@@ -427,7 +425,7 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
     /// That means they went out of a nonlexical scope
     fn kill_loans_out_of_scope_at_location(
         &self,
-        trans: &mut impl GenKill<BorrowIndex>,
+        trans: &mut <Self as Analysis<'tcx>>::Domain,
         location: Location,
     ) {
         // NOTE: The state associated with a given `location`
@@ -447,7 +445,11 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
     }
 
     /// Kill any borrows that conflict with `place`.
-    fn kill_borrows_on_place(&self, trans: &mut impl GenKill<BorrowIndex>, place: Place<'tcx>) {
+    fn kill_borrows_on_place(
+        &self,
+        trans: &mut <Self as Analysis<'tcx>>::Domain,
+        place: Place<'tcx>,
+    ) {
         debug!("kill_borrows_on_place: place={:?}", place);
 
         let other_borrows_of_local = self
@@ -486,7 +488,14 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
     }
 }
 
-impl<'tcx> rustc_mir_dataflow::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> {
+/// Forward dataflow computation of the set of borrows that are in scope at a particular location.
+/// - we gen the introduced loans
+/// - we kill loans on locals going out of (regular) scope
+/// - we kill the loans going out of their region's NLL scope: in NLL terms, the frontier where a
+///   region stops containing the CFG points reachable from the issuing location.
+/// - we also kill loans of conflicting places when overwriting a shared path: e.g. borrows of
+///   `a.b.c` when `a` is overwritten.
+impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> {
     type Domain = BitSet<BorrowIndex>;
 
     const NAME: &'static str = "borrows";
@@ -500,34 +509,19 @@ impl<'tcx> rustc_mir_dataflow::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> {
         // no borrows of code region_scopes have been taken prior to
         // function execution, so this method has no effect.
     }
-}
-
-/// Forward dataflow computation of the set of borrows that are in scope at a particular location.
-/// - we gen the introduced loans
-/// - we kill loans on locals going out of (regular) scope
-/// - we kill the loans going out of their region's NLL scope: in NLL terms, the frontier where a
-///   region stops containing the CFG points reachable from the issuing location.
-/// - we also kill loans of conflicting places when overwriting a shared path: e.g. borrows of
-///   `a.b.c` when `a` is overwritten.
-impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
-    type Idx = BorrowIndex;
-
-    fn domain_size(&self, _: &mir::Body<'tcx>) -> usize {
-        self.borrow_set.len()
-    }
 
-    fn before_statement_effect(
+    fn apply_before_statement_effect(
         &mut self,
-        trans: &mut impl GenKill<Self::Idx>,
+        trans: &mut Self::Domain,
         _statement: &mir::Statement<'tcx>,
         location: Location,
     ) {
         self.kill_loans_out_of_scope_at_location(trans, location);
     }
 
-    fn statement_effect(
+    fn apply_statement_effect(
         &mut self,
-        trans: &mut impl GenKill<Self::Idx>,
+        trans: &mut Self::Domain,
         stmt: &mir::Statement<'tcx>,
         location: Location,
     ) {
@@ -573,7 +567,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
         }
     }
 
-    fn before_terminator_effect(
+    fn apply_before_terminator_effect(
         &mut self,
         trans: &mut Self::Domain,
         _terminator: &mir::Terminator<'tcx>,
@@ -582,7 +576,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
         self.kill_loans_out_of_scope_at_location(trans, location);
     }
 
-    fn terminator_effect<'mir>(
+    fn apply_terminator_effect<'mir>(
         &mut self,
         trans: &mut Self::Domain,
         terminator: &'mir mir::Terminator<'tcx>,
@@ -599,14 +593,6 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
         }
         terminator.edges()
     }
-
-    fn call_return_effect(
-        &mut self,
-        _trans: &mut Self::Domain,
-        _block: mir::BasicBlock,
-        _return_places: CallReturnPlaces<'_, 'tcx>,
-    ) {
-    }
 }
 
 impl<C> DebugWithContext<C> for BorrowIndex {}

compiler/rustc_const_eval/src/check_consts/resolver.rs

@@ -11,7 +11,7 @@ use rustc_middle::mir::{
     self, BasicBlock, CallReturnPlaces, Local, Location, Statement, StatementKind, TerminatorEdges,
 };
 use rustc_mir_dataflow::fmt::DebugWithContext;
-use rustc_mir_dataflow::{Analysis, AnalysisDomain, JoinSemiLattice};
+use rustc_mir_dataflow::{Analysis, JoinSemiLattice};
 
 use super::{ConstCx, Qualif, qualifs};
 
@@ -310,7 +310,7 @@ impl JoinSemiLattice for State {
     }
 }
 
-impl<'tcx, Q> AnalysisDomain<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q>
+impl<'tcx, Q> Analysis<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q>
 where
     Q: Qualif,
 {
@@ -328,12 +328,7 @@ where
     fn initialize_start_block(&self, _body: &mir::Body<'tcx>, state: &mut Self::Domain) {
         self.transfer_function(state).initialize_state();
     }
-}
 
-impl<'tcx, Q> Analysis<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q>
-where
-    Q: Qualif,
-{
     fn apply_statement_effect(
         &mut self,
         state: &mut Self::Domain,

compiler/rustc_middle/src/mir/basic_blocks.rs

@@ -26,7 +26,6 @@ type SwitchSources = FxHashMap<(BasicBlock, BasicBlock), SmallVec<[Option<u128>;
 struct Cache {
     predecessors: OnceLock<Predecessors>,
     switch_sources: OnceLock<SwitchSources>,
-    is_cyclic: OnceLock<bool>,
     reverse_postorder: OnceLock<Vec<BasicBlock>>,
     dominators: OnceLock<Dominators<BasicBlock>>,
 }
@@ -37,12 +36,6 @@ impl<'tcx> BasicBlocks<'tcx> {
         BasicBlocks { basic_blocks, cache: Cache::default() }
     }
 
-    /// Returns true if control-flow graph contains a cycle reachable from the `START_BLOCK`.
-    #[inline]
-    pub fn is_cfg_cyclic(&self) -> bool {
-        *self.cache.is_cyclic.get_or_init(|| graph::is_cyclic(self))
-    }
-
     pub fn dominators(&self) -> &Dominators<BasicBlock> {
         self.cache.dominators.get_or_init(|| dominators(self))
     }

compiler/rustc_mir_dataflow/src/framework/cursor.rs

@@ -7,7 +7,6 @@ use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::{self, BasicBlock, Location};
 
 use super::{Analysis, Direction, Effect, EffectIndex, Results};
-use crate::framework::BitSetExt;
 
 /// Allows random access inspection of the results of a dataflow analysis.
 ///
@@ -221,16 +220,6 @@ where
     }
 }
 
-impl<'mir, 'tcx, A> ResultsCursor<'mir, 'tcx, A>
-where
-    A: crate::GenKillAnalysis<'tcx>,
-    A::Domain: BitSetExt<A::Idx>,
-{
-    pub fn contains(&self, elem: A::Idx) -> bool {
-        self.get().contains(elem)
-    }
-}
-
 #[derive(Clone, Copy, Debug)]
 struct CursorPosition {
     block: BasicBlock,

compiler/rustc_mir_dataflow/src/framework/direction.rs

@@ -5,7 +5,7 @@ use rustc_middle::mir::{
 };
 
 use super::visitor::{ResultsVisitable, ResultsVisitor};
-use super::{Analysis, Effect, EffectIndex, GenKillAnalysis, GenKillSet, SwitchIntTarget};
+use super::{Analysis, Effect, EffectIndex, SwitchIntTarget};
 
 pub trait Direction {
     const IS_FORWARD: bool;
@@ -29,19 +29,10 @@ pub trait Direction {
         state: &mut A::Domain,
         block: BasicBlock,
         block_data: &'mir mir::BasicBlockData<'tcx>,
-        statement_effect: Option<&dyn Fn(BasicBlock, &mut A::Domain)>,
     ) -> TerminatorEdges<'mir, 'tcx>
     where
         A: Analysis<'tcx>;
 
-    fn gen_kill_statement_effects_in_block<'tcx, A>(
-        analysis: &mut A,
-        trans: &mut GenKillSet<A::Idx>,
-        block: BasicBlock,
-        block_data: &mir::BasicBlockData<'tcx>,
-    ) where
-        A: GenKillAnalysis<'tcx>;
-
     fn visit_results_in_block<'mir, 'tcx, D, R>(
         state: &mut D,
         block: BasicBlock,
@@ -73,7 +64,6 @@ impl Direction for Backward {
         state: &mut A::Domain,
         block: BasicBlock,
         block_data: &'mir mir::BasicBlockData<'tcx>,
-        statement_effect: Option<&dyn Fn(BasicBlock, &mut A::Domain)>,
     ) -> TerminatorEdges<'mir, 'tcx>
     where
         A: Analysis<'tcx>,
@@ -82,31 +72,12 @@ impl Direction for Backward {
         let location = Location { block, statement_index: block_data.statements.len() };
         analysis.apply_before_terminator_effect(state, terminator, location);
         let edges = analysis.apply_terminator_effect(state, terminator, location);
-        if let Some(statement_effect) = statement_effect {
-            statement_effect(block, state)
-        } else {
-            for (statement_index, statement) in block_data.statements.iter().enumerate().rev() {
-                let location = Location { block, statement_index };
-                analysis.apply_before_statement_effect(state, statement, location);
-                analysis.apply_statement_effect(state, statement, location);
-            }
-        }
-        edges
-    }
-
-    fn gen_kill_statement_effects_in_block<'tcx, A>(
-        analysis: &mut A,
-        trans: &mut GenKillSet<A::Idx>,
-        block: BasicBlock,
-        block_data: &mir::BasicBlockData<'tcx>,
-    ) where
-        A: GenKillAnalysis<'tcx>,
-    {
         for (statement_index, statement) in block_data.statements.iter().enumerate().rev() {
             let location = Location { block, statement_index };
-            analysis.before_statement_effect(trans, statement, location);
-            analysis.statement_effect(trans, statement, location);
+            analysis.apply_before_statement_effect(state, statement, location);
+            analysis.apply_statement_effect(state, statement, location);
         }
+        edges
     }
 
     fn apply_effects_in_range<'tcx, A>(
@@ -330,42 +301,21 @@ impl Direction for Forward {
         state: &mut A::Domain,
         block: BasicBlock,
         block_data: &'mir mir::BasicBlockData<'tcx>,
-        statement_effect: Option<&dyn Fn(BasicBlock, &mut A::Domain)>,
     ) -> TerminatorEdges<'mir, 'tcx>
     where
         A: Analysis<'tcx>,
     {
-        if let Some(statement_effect) = statement_effect {
-            statement_effect(block, state)
-        } else {
-            for (statement_index, statement) in block_data.statements.iter().enumerate() {
-                let location = Location { block, statement_index };
-                analysis.apply_before_statement_effect(state, statement, location);
-                analysis.apply_statement_effect(state, statement, location);
-            }
+        for (statement_index, statement) in block_data.statements.iter().enumerate() {
+            let location = Location { block, statement_index };
+            analysis.apply_before_statement_effect(state, statement, location);
+            analysis.apply_statement_effect(state, statement, location);
         }
-
         let terminator = block_data.terminator();
         let location = Location { block, statement_index: block_data.statements.len() };
         analysis.apply_before_terminator_effect(state, terminator, location);
         analysis.apply_terminator_effect(state, terminator, location)
     }
 
-    fn gen_kill_statement_effects_in_block<'tcx, A>(
-        analysis: &mut A,
-        trans: &mut GenKillSet<A::Idx>,
-        block: BasicBlock,
-        block_data: &mir::BasicBlockData<'tcx>,
-    ) where
-        A: GenKillAnalysis<'tcx>,
-    {
-        for (statement_index, statement) in block_data.statements.iter().enumerate() {
-            let location = Location { block, statement_index };
-            analysis.before_statement_effect(trans, statement, location);
-            analysis.statement_effect(trans, statement, location);
-        }
-    }
-
     fn apply_effects_in_range<'tcx, A>(
         analysis: &mut A,
         state: &mut A::Domain,