about summary refs log tree commit diff
path: root/compiler/rustc_mir_dataflow/src/framework/direction.rs
diff options
context:
space:
mode:
authorNicholas Nethercote <n.nethercote@gmail.com>2024-11-21 10:20:30 +1100
committerNicholas Nethercote <n.nethercote@gmail.com>2024-12-16 09:36:07 +1100
commit4d8316f4d40cc9fb431b9cab5825c32fac43a19a (patch)
treebc8731d40df57e757a045da3c0da38c5b18026be /compiler/rustc_mir_dataflow/src/framework/direction.rs
parent848610fddc127b8fa44c5c40d336bc10ee34e563 (diff)
downloadrust-4d8316f4d40cc9fb431b9cab5825c32fac43a19a.tar.gz
rust-4d8316f4d40cc9fb431b9cab5825c32fac43a19a.zip
Simplify dataflow `SwitchInt` handling.
Current `SwitchInt` handling has complicated control flow.

- The dataflow engine calls `Analysis::apply_switch_int_edge_effects`,
  passing in an "applier" that impls `SwitchIntEdgeEffects`.
- `apply_switch_int_edge_effects` possibly calls `apply` on the applier,
  passing it a closure.
- The `apply` method calls the closure on each `SwitchInt` edge.
- The closure operates on the edge.

I.e. control flow goes from the engine, to the analysis, to the applier
(which came from the engine), to the closure (which came from the
analysis). It took me a while to work this out.

This commit changes to a simpler structure that maintains the important
characteristics.

- The dataflow engine calls `Analysis::get_switch_int_data`.
- `get_switch_int_data` returns an `Option<Self::SwitchIntData>` value.
- If that returned value was `Some`, the dataflow engine calls
  `Analysis::apply_switch_int_edge_effect` on each edge, passing the
  `Self::SwitchIntData` value.
- `Analysis::apply_switch_int_edge_effect` operates on the edge.

I.e. control flow goes from the engine, to the analysis, to the
engine, to the analysis.

Added:
- The `Analysis::SwitchIntData` assoc type and the
  `Analysis::get_switch_int_data` method. Both only need to be
  defined by analyses that look at `SwitchInt` terminators.
- The `MaybePlacesSwitchIntData` struct, which has three fields.

Changes:
- `Analysis::apply_switch_int_edge_effects` becomes
  `Analysis::apply_switch_int_edge_effect`, which is a little simpler
  because it's dealing with a single edge instead of all edges.

Removed:
- The `SwitchIntEdgeEffects` trait, and its two impls:
  `BackwardSwitchIntEdgeEffectsApplier` (which has six fields) and
  `ForwardSwitchIntEdgeEffectsApplier` structs (which has four fields).
- The closure.

The new structure is more concise and simpler.
Diffstat (limited to 'compiler/rustc_mir_dataflow/src/framework/direction.rs')
-rw-r--r--compiler/rustc_mir_dataflow/src/framework/direction.rs130
1 files changed, 34 insertions, 96 deletions
diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs
index 9d943ebe327..45b1023e795 100644
--- a/compiler/rustc_mir_dataflow/src/framework/direction.rs
+++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs
@@ -1,8 +1,6 @@
 use std::ops::RangeInclusive;
 
-use rustc_middle::mir::{
-    self, BasicBlock, CallReturnPlaces, Location, SwitchTargets, TerminatorEdges,
-};
+use rustc_middle::mir::{self, BasicBlock, CallReturnPlaces, Location, TerminatorEdges};
 
 use super::visitor::ResultsVisitor;
 use super::{Analysis, Effect, EffectIndex, Results, SwitchIntTarget};
@@ -115,18 +113,18 @@ impl Direction for Backward {
                 }
 
                 mir::TerminatorKind::SwitchInt { targets: _, ref discr } => {
-                    let mut applier = BackwardSwitchIntEdgeEffectsApplier {
-                        body,
-                        pred,
-                        exit_state,
-                        block,
-                        propagate: &mut propagate,
-                        effects_applied: false,
-                    };
-
-                    analysis.apply_switch_int_edge_effects(pred, discr, &mut applier);
-
-                    if !applier.effects_applied {
+                    if let Some(mut data) = analysis.get_switch_int_data(block, discr) {
+                        let values = &body.basic_blocks.switch_sources()[&(block, pred)];
+                        let targets =
+                            values.iter().map(|&value| SwitchIntTarget { value, target: block });
+
+                        let mut tmp = None;
+                        for target in targets {
+                            let tmp = opt_clone_from_or_clone(&mut tmp, exit_state);
+                            analysis.apply_switch_int_edge_effect(&mut data, tmp, target);
+                            propagate(pred, tmp);
+                        }
+                    } else {
                         propagate(pred, exit_state)
                     }
                 }
@@ -245,37 +243,6 @@ impl Direction for Backward {
     }
 }
 
-struct BackwardSwitchIntEdgeEffectsApplier<'mir, 'tcx, D, F> {
-    body: &'mir mir::Body<'tcx>,
-    pred: BasicBlock,
-    exit_state: &'mir mut D,
-    block: BasicBlock,
-    propagate: &'mir mut F,
-    effects_applied: bool,
-}
-
-impl<D, F> super::SwitchIntEdgeEffects<D> for BackwardSwitchIntEdgeEffectsApplier<'_, '_, D, F>
-where
-    D: Clone,
-    F: FnMut(BasicBlock, &D),
-{
-    fn apply(&mut self, mut apply_edge_effect: impl FnMut(&mut D, SwitchIntTarget)) {
-        assert!(!self.effects_applied);
-
-        let values = &self.body.basic_blocks.switch_sources()[&(self.block, self.pred)];
-        let targets = values.iter().map(|&value| SwitchIntTarget { value, target: self.block });
-
-        let mut tmp = None;
-        for target in targets {
-            let tmp = opt_clone_from_or_clone(&mut tmp, self.exit_state);
-            apply_edge_effect(tmp, target);
-            (self.propagate)(self.pred, tmp);
-        }
-
-        self.effects_applied = true;
-    }
-}
-
 /// Dataflow that runs from the entry of a block (the first statement), to its exit (terminator).
 pub struct Forward;
 
@@ -324,23 +291,27 @@ impl Direction for Forward {
                 }
             }
             TerminatorEdges::SwitchInt { targets, discr } => {
-                let mut applier = ForwardSwitchIntEdgeEffectsApplier {
-                    exit_state,
-                    targets,
-                    propagate,
-                    effects_applied: false,
-                };
-
-                analysis.apply_switch_int_edge_effects(block, discr, &mut applier);
-
-                let ForwardSwitchIntEdgeEffectsApplier {
-                    exit_state,
-                    mut propagate,
-                    effects_applied,
-                    ..
-                } = applier;
-
-                if !effects_applied {
+                if let Some(mut data) = analysis.get_switch_int_data(block, discr) {
+                    let mut tmp = None;
+                    for (value, target) in targets.iter() {
+                        let tmp = opt_clone_from_or_clone(&mut tmp, exit_state);
+                        analysis.apply_switch_int_edge_effect(&mut data, tmp, SwitchIntTarget {
+                            value: Some(value),
+                            target,
+                        });
+                        propagate(target, tmp);
+                    }
+
+                    // Once we get to the final, "otherwise" branch, there is no need to preserve
+                    // `exit_state`, so pass it directly to `apply_switch_int_edge_effect` to save
+                    // a clone of the dataflow state.
+                    let otherwise = targets.otherwise();
+                    analysis.apply_switch_int_edge_effect(&mut data, exit_state, SwitchIntTarget {
+                        value: None,
+                        target: otherwise,
+                    });
+                    propagate(otherwise, exit_state);
+                } else {
                     for target in targets.all_targets() {
                         propagate(*target, exit_state);
                     }
@@ -455,39 +426,6 @@ impl Direction for Forward {
     }
 }
 
-struct ForwardSwitchIntEdgeEffectsApplier<'mir, D, F> {
-    exit_state: &'mir mut D,
-    targets: &'mir SwitchTargets,
-    propagate: F,
-
-    effects_applied: bool,
-}
-
-impl<D, F> super::SwitchIntEdgeEffects<D> for ForwardSwitchIntEdgeEffectsApplier<'_, D, F>
-where
-    D: Clone,
-    F: FnMut(BasicBlock, &D),
-{
-    fn apply(&mut self, mut apply_edge_effect: impl FnMut(&mut D, SwitchIntTarget)) {
-        assert!(!self.effects_applied);
-
-        let mut tmp = None;
-        for (value, target) in self.targets.iter() {
-            let tmp = opt_clone_from_or_clone(&mut tmp, self.exit_state);
-            apply_edge_effect(tmp, SwitchIntTarget { value: Some(value), target });
-            (self.propagate)(target, tmp);
-        }
-
-        // Once we get to the final, "otherwise" branch, there is no need to preserve `exit_state`,
-        // so pass it directly to `apply_edge_effect` to save a clone of the dataflow state.
-        let otherwise = self.targets.otherwise();
-        apply_edge_effect(self.exit_state, SwitchIntTarget { value: None, target: otherwise });
-        (self.propagate)(otherwise, self.exit_state);
-
-        self.effects_applied = true;
-    }
-}
-
 /// An analogue of `Option::get_or_insert_with` that stores a clone of `val` into `opt`, but uses
 /// the more efficient `clone_from` if `opt` was `Some`.
 ///