From ee7413b94cd0f953518ad73f37cac28feb6e8d52 Mon Sep 17 00:00:00 2001 From: "Samuel E. Moelius III" Date: Sat, 19 Mar 2022 12:04:59 -0400 Subject: [PATCH 1/3] Implement `apply_switch_int_edge_effects` for backward analyses --- .../src/framework/direction.rs | 60 +++++++++++++++++-- .../rustc_mir_dataflow/src/framework/mod.rs | 2 - compiler/rustc_mir_dataflow/src/lib.rs | 2 +- 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs index 102e74397724a..1ad2722d91b85 100644 --- a/compiler/rustc_mir_dataflow/src/framework/direction.rs +++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs @@ -248,6 +248,7 @@ impl Direction for Backward { ); propagate(pred, &tmp); } + mir::TerminatorKind::InlineAsm { destination: Some(dest), ref operands, .. } if dest == bb => { @@ -266,6 +267,25 @@ impl Direction for Backward { propagate(pred, &tmp); } + mir::TerminatorKind::SwitchInt { ref targets, ref discr, switch_ty: _ } => { + let mut applier = BackwardSwitchIntEdgeEffectsApplier { + pred, + exit_state, + targets, + bb, + propagate: &mut propagate, + effects_applied: false, + }; + + analysis.apply_switch_int_edge_effects(pred, discr, &mut applier); + + let BackwardSwitchIntEdgeEffectsApplier { effects_applied, .. } = applier; + + if !effects_applied { + propagate(pred, exit_state) + } + } + // Ignore dead unwinds. mir::TerminatorKind::Call { cleanup: Some(unwind), .. } | mir::TerminatorKind::Assert { cleanup: Some(unwind), .. } @@ -286,6 +306,33 @@ impl Direction for Backward { } } +struct BackwardSwitchIntEdgeEffectsApplier<'a, D, F> { + pred: BasicBlock, + exit_state: &'a mut D, + targets: &'a SwitchTargets, + bb: BasicBlock, + propagate: &'a mut F, + + effects_applied: bool, +} + +impl super::SwitchIntEdgeEffects 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 value = + self.targets.iter().find_map(|(value, target)| (target == self.bb).then_some(value)); + apply_edge_effect(self.exit_state, SwitchIntTarget { value, target: self.bb }); + (self.propagate)(self.pred, self.exit_state); + + self.effects_applied = true; + } +} + /// Dataflow that runs from the entry of a block (the first statement), to its exit (terminator). pub struct Forward; @@ -528,7 +575,7 @@ impl Direction for Forward { } SwitchInt { ref targets, ref discr, switch_ty: _ } => { - let mut applier = SwitchIntEdgeEffectApplier { + let mut applier = ForwardSwitchIntEdgeEffectsApplier { exit_state, targets, propagate, @@ -537,8 +584,11 @@ impl Direction for Forward { analysis.apply_switch_int_edge_effects(bb, discr, &mut applier); - let SwitchIntEdgeEffectApplier { - exit_state, mut propagate, effects_applied, .. + let ForwardSwitchIntEdgeEffectsApplier { + exit_state, + mut propagate, + effects_applied, + .. } = applier; if !effects_applied { @@ -551,7 +601,7 @@ impl Direction for Forward { } } -struct SwitchIntEdgeEffectApplier<'a, D, F> { +struct ForwardSwitchIntEdgeEffectsApplier<'a, D, F> { exit_state: &'a mut D, targets: &'a SwitchTargets, propagate: F, @@ -559,7 +609,7 @@ struct SwitchIntEdgeEffectApplier<'a, D, F> { effects_applied: bool, } -impl super::SwitchIntEdgeEffects for SwitchIntEdgeEffectApplier<'_, D, F> +impl super::SwitchIntEdgeEffects for ForwardSwitchIntEdgeEffectsApplier<'_, D, F> where D: Clone, F: FnMut(BasicBlock, &D), diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index c51dd06de251d..67c16e6c0849d 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -234,8 +234,6 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// about a given `SwitchInt` terminator for each one of its edges—and more efficient—the /// engine doesn't need to clone the exit state for a block unless /// `SwitchIntEdgeEffects::apply` is actually called. - /// - /// FIXME: This class of effects is not supported for backward dataflow analyses. fn apply_switch_int_edge_effects( &self, _block: BasicBlock, diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/compiler/rustc_mir_dataflow/src/lib.rs index 6c2d1b85646b1..c221b35867082 100644 --- a/compiler/rustc_mir_dataflow/src/lib.rs +++ b/compiler/rustc_mir_dataflow/src/lib.rs @@ -28,7 +28,7 @@ pub use self::drop_flag_effects::{ pub use self::framework::{ fmt, graphviz, lattice, visit_results, Analysis, AnalysisDomain, Backward, CallReturnPlaces, Direction, Engine, Forward, GenKill, GenKillAnalysis, JoinSemiLattice, Results, ResultsCursor, - ResultsRefCursor, ResultsVisitable, ResultsVisitor, + ResultsRefCursor, ResultsVisitable, ResultsVisitor, SwitchIntEdgeEffects, }; use self::move_paths::MoveData; From 37ebd47ddb1398c7b0f19aeb888417fbcf2d5992 Mon Sep 17 00:00:00 2001 From: "Samuel E. Moelius III" Date: Fri, 25 Mar 2022 21:11:49 -0400 Subject: [PATCH 2/3] Address review comments * Add lazily computed `switch_sources` data structure * Don't assume a target has only one associated value --- compiler/rustc_middle/src/mir/mod.rs | 13 +++ .../rustc_middle/src/mir/switch_sources.rs | 82 +++++++++++++++++++ .../src/framework/direction.rs | 18 ++-- 3 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 compiler/rustc_middle/src/mir/switch_sources.rs diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index a5468b3f4f202..455387901b61d 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -45,6 +45,7 @@ use std::{iter, mem, option}; use self::graph_cyclic_cache::GraphIsCyclicCache; use self::predecessors::{PredecessorCache, Predecessors}; pub use self::query::*; +use self::switch_sources::{SwitchSourceCache, SwitchSources}; pub mod coverage; mod generic_graph; @@ -58,6 +59,7 @@ mod predecessors; pub mod pretty; mod query; pub mod spanview; +mod switch_sources; pub mod tcx; pub mod terminator; pub use terminator::*; @@ -284,6 +286,7 @@ pub struct Body<'tcx> { pub is_polymorphic: bool, predecessor_cache: PredecessorCache, + switch_source_cache: SwitchSourceCache, is_cyclic: GraphIsCyclicCache, pub tainted_by_errors: Option, @@ -332,6 +335,7 @@ impl<'tcx> Body<'tcx> { required_consts: Vec::new(), is_polymorphic: false, predecessor_cache: PredecessorCache::new(), + switch_source_cache: SwitchSourceCache::new(), is_cyclic: GraphIsCyclicCache::new(), tainted_by_errors, }; @@ -360,6 +364,7 @@ impl<'tcx> Body<'tcx> { var_debug_info: Vec::new(), is_polymorphic: false, predecessor_cache: PredecessorCache::new(), + switch_source_cache: SwitchSourceCache::new(), is_cyclic: GraphIsCyclicCache::new(), tainted_by_errors: None, }; @@ -380,6 +385,7 @@ impl<'tcx> Body<'tcx> { // FIXME: Use a finer-grained API for this, so only transformations that alter terminators // invalidate the caches. self.predecessor_cache.invalidate(); + self.switch_source_cache.invalidate(); self.is_cyclic.invalidate(); &mut self.basic_blocks } @@ -389,6 +395,7 @@ impl<'tcx> Body<'tcx> { &mut self, ) -> (&mut IndexVec>, &mut LocalDecls<'tcx>) { self.predecessor_cache.invalidate(); + self.switch_source_cache.invalidate(); self.is_cyclic.invalidate(); (&mut self.basic_blocks, &mut self.local_decls) } @@ -402,6 +409,7 @@ impl<'tcx> Body<'tcx> { &mut Vec>, ) { self.predecessor_cache.invalidate(); + self.switch_source_cache.invalidate(); self.is_cyclic.invalidate(); (&mut self.basic_blocks, &mut self.local_decls, &mut self.var_debug_info) } @@ -529,6 +537,11 @@ impl<'tcx> Body<'tcx> { self.predecessor_cache.compute(&self.basic_blocks) } + #[inline] + pub fn switch_sources(&self) -> &SwitchSources { + self.switch_source_cache.compute(&self.basic_blocks) + } + #[inline] pub fn dominators(&self) -> Dominators { dominators(self) diff --git a/compiler/rustc_middle/src/mir/switch_sources.rs b/compiler/rustc_middle/src/mir/switch_sources.rs new file mode 100644 index 0000000000000..7f62b4d0dbab9 --- /dev/null +++ b/compiler/rustc_middle/src/mir/switch_sources.rs @@ -0,0 +1,82 @@ +//! Lazily compute the inverse of each `SwitchInt`'s switch targets. Modeled after +//! `Predecessors`/`PredecessorCache`. + +use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_data_structures::sync::OnceCell; +use rustc_index::vec::IndexVec; +use rustc_serialize as serialize; +use smallvec::SmallVec; + +use crate::mir::{BasicBlock, BasicBlockData, Terminator, TerminatorKind}; + +pub type SwitchSources = IndexVec; 1]>>>; + +#[derive(Clone, Debug)] +pub(super) struct SwitchSourceCache { + cache: OnceCell, +} + +impl SwitchSourceCache { + #[inline] + pub(super) fn new() -> Self { + SwitchSourceCache { cache: OnceCell::new() } + } + + /// Invalidates the switch source cache. + #[inline] + pub(super) fn invalidate(&mut self) { + self.cache = OnceCell::new(); + } + + /// Returns the switch sources for this MIR. + #[inline] + pub(super) fn compute( + &self, + basic_blocks: &IndexVec>, + ) -> &SwitchSources { + self.cache.get_or_init(|| { + let mut switch_sources = IndexVec::from_elem( + IndexVec::from_elem(SmallVec::new(), basic_blocks), + basic_blocks, + ); + for (bb, data) in basic_blocks.iter_enumerated() { + if let Some(Terminator { + kind: TerminatorKind::SwitchInt { targets, .. }, .. + }) = &data.terminator + { + for (value, target) in targets.iter() { + switch_sources[target][bb].push(Some(value)); + } + switch_sources[targets.otherwise()][bb].push(None); + } + } + + switch_sources + }) + } +} + +impl serialize::Encodable for SwitchSourceCache { + #[inline] + fn encode(&self, s: &mut S) -> Result<(), S::Error> { + s.emit_unit() + } +} + +impl serialize::Decodable for SwitchSourceCache { + #[inline] + fn decode(_: &mut D) -> Self { + Self::new() + } +} + +impl HashStable for SwitchSourceCache { + #[inline] + fn hash_stable(&self, _: &mut CTX, _: &mut StableHasher) { + // do nothing + } +} + +TrivialTypeFoldableAndLiftImpls! { + SwitchSourceCache, +} diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs index 1ad2722d91b85..d7f531f7de7dd 100644 --- a/compiler/rustc_mir_dataflow/src/framework/direction.rs +++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs @@ -267,11 +267,11 @@ impl Direction for Backward { propagate(pred, &tmp); } - mir::TerminatorKind::SwitchInt { ref targets, ref discr, switch_ty: _ } => { + mir::TerminatorKind::SwitchInt { targets: _, ref discr, switch_ty: _ } => { let mut applier = BackwardSwitchIntEdgeEffectsApplier { pred, exit_state, - targets, + values: &body.switch_sources()[bb][pred], bb, propagate: &mut propagate, effects_applied: false, @@ -309,7 +309,7 @@ impl Direction for Backward { struct BackwardSwitchIntEdgeEffectsApplier<'a, D, F> { pred: BasicBlock, exit_state: &'a mut D, - targets: &'a SwitchTargets, + values: &'a [Option], bb: BasicBlock, propagate: &'a mut F, @@ -324,10 +324,14 @@ where fn apply(&mut self, mut apply_edge_effect: impl FnMut(&mut D, SwitchIntTarget)) { assert!(!self.effects_applied); - let value = - self.targets.iter().find_map(|(value, target)| (target == self.bb).then_some(value)); - apply_edge_effect(self.exit_state, SwitchIntTarget { value, target: self.bb }); - (self.propagate)(self.pred, self.exit_state); + let targets = self.values.iter().map(|&value| SwitchIntTarget { value, target: self.bb }); + + 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; } From 241ec5b3b35900cf5a9becf7da3e3ea49448d8fb Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 27 Mar 2022 10:58:55 -0700 Subject: [PATCH 3/3] Nit --- compiler/rustc_mir_dataflow/src/framework/direction.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs index d7f531f7de7dd..93118dfeb7737 100644 --- a/compiler/rustc_mir_dataflow/src/framework/direction.rs +++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs @@ -279,9 +279,7 @@ impl Direction for Backward { analysis.apply_switch_int_edge_effects(pred, discr, &mut applier); - let BackwardSwitchIntEdgeEffectsApplier { effects_applied, .. } = applier; - - if !effects_applied { + if !applier.effects_applied { propagate(pred, exit_state) } }