From 71138e99337e791eb73d73d8a2cf8aaef29960b1 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 24 Apr 2023 17:10:39 +0000 Subject: [PATCH 1/8] Make HasTop and HasBottom consts. --- .../src/framework/lattice.rs | 20 ++++------- .../rustc_mir_dataflow/src/value_analysis.rs | 34 +++++++++---------- .../src/dataflow_const_prop.rs | 4 +-- 3 files changed, 24 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/lattice.rs b/compiler/rustc_mir_dataflow/src/framework/lattice.rs index af44a4329bf3d..3952f44ad489d 100644 --- a/compiler/rustc_mir_dataflow/src/framework/lattice.rs +++ b/compiler/rustc_mir_dataflow/src/framework/lattice.rs @@ -75,12 +75,12 @@ pub trait MeetSemiLattice: Eq { /// A set that has a "bottom" element, which is less than or equal to any other element. pub trait HasBottom { - fn bottom() -> Self; + const BOTTOM: Self; } /// A set that has a "top" element, which is greater than or equal to any other element. pub trait HasTop { - fn top() -> Self; + const TOP: Self; } /// A `bool` is a "two-point" lattice with `true` as the top element and `false` as the bottom: @@ -113,15 +113,11 @@ impl MeetSemiLattice for bool { } impl HasBottom for bool { - fn bottom() -> Self { - false - } + const BOTTOM: Self = false; } impl HasTop for bool { - fn top() -> Self { - true - } + const TOP: Self = true; } /// A tuple (or list) of lattices is itself a lattice whose least upper bound is the concatenation @@ -274,13 +270,9 @@ impl MeetSemiLattice for FlatSet { } impl HasBottom for FlatSet { - fn bottom() -> Self { - Self::Bottom - } + const BOTTOM: Self = Self::Bottom; } impl HasTop for FlatSet { - fn top() -> Self { - Self::Top - } + const TOP: Self = Self::Top; } diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index fdff1ec788de5..e830b8c715777 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -74,11 +74,11 @@ pub trait ValueAnalysis<'tcx> { StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => { // StorageLive leaves the local in an uninitialized state. // StorageDead makes it UB to access the local afterwards. - state.flood_with(Place::from(*local).as_ref(), self.map(), Self::Value::bottom()); + state.flood_with(Place::from(*local).as_ref(), self.map(), Self::Value::BOTTOM); } StatementKind::Deinit(box place) => { // Deinit makes the place uninitialized. - state.flood_with(place.as_ref(), self.map(), Self::Value::bottom()); + state.flood_with(place.as_ref(), self.map(), Self::Value::BOTTOM); } StatementKind::Retag(..) => { // We don't track references. @@ -154,7 +154,7 @@ pub trait ValueAnalysis<'tcx> { Rvalue::CopyForDeref(place) => self.handle_operand(&Operand::Copy(*place), state), Rvalue::Ref(..) | Rvalue::AddressOf(..) => { // We don't track such places. - ValueOrPlace::top() + ValueOrPlace::TOP } Rvalue::Repeat(..) | Rvalue::ThreadLocalRef(..) @@ -168,7 +168,7 @@ pub trait ValueAnalysis<'tcx> { | Rvalue::Aggregate(..) | Rvalue::ShallowInitBox(..) => { // No modification is possible through these r-values. - ValueOrPlace::top() + ValueOrPlace::TOP } } } @@ -196,7 +196,7 @@ pub trait ValueAnalysis<'tcx> { self.map() .find(place.as_ref()) .map(ValueOrPlace::Place) - .unwrap_or(ValueOrPlace::top()) + .unwrap_or(ValueOrPlace::TOP) } } } @@ -214,7 +214,7 @@ pub trait ValueAnalysis<'tcx> { _constant: &Constant<'tcx>, _state: &mut State, ) -> Self::Value { - Self::Value::top() + Self::Value::TOP } /// The effect of a successful function call return should not be @@ -229,7 +229,7 @@ pub trait ValueAnalysis<'tcx> { // Effect is applied by `handle_call_return`. } TerminatorKind::Drop { place, .. } => { - state.flood_with(place.as_ref(), self.map(), Self::Value::bottom()); + state.flood_with(place.as_ref(), self.map(), Self::Value::BOTTOM); } TerminatorKind::Yield { .. } => { // They would have an effect, but are not allowed in this phase. @@ -307,7 +307,7 @@ impl<'tcx, T: ValueAnalysis<'tcx>> AnalysisDomain<'tcx> for ValueAnalysisWrapper fn initialize_start_block(&self, body: &Body<'tcx>, state: &mut Self::Domain) { // The initial state maps all tracked places of argument projections to ⊤ and the rest to ⊥. assert!(matches!(state.0, StateData::Unreachable)); - let values = IndexVec::from_elem_n(T::Value::bottom(), self.0.map().value_count); + let values = IndexVec::from_elem_n(T::Value::BOTTOM, self.0.map().value_count); *state = State(StateData::Reachable(values)); for arg in body.args_iter() { state.flood(PlaceRef { local: arg, projection: &[] }, self.0.map()); @@ -437,7 +437,7 @@ impl State { } pub fn flood_all(&mut self) { - self.flood_all_with(V::top()) + self.flood_all_with(V::TOP) } pub fn flood_all_with(&mut self, value: V) { @@ -455,7 +455,7 @@ impl State { } pub fn flood(&mut self, place: PlaceRef<'_>, map: &Map) { - self.flood_with(place, map, V::top()) + self.flood_with(place, map, V::TOP) } pub fn flood_discr_with(&mut self, place: PlaceRef<'_>, map: &Map, value: V) { @@ -468,7 +468,7 @@ impl State { } pub fn flood_discr(&mut self, place: PlaceRef<'_>, map: &Map) { - self.flood_discr_with(place, map, V::top()) + self.flood_discr_with(place, map, V::TOP) } /// Low-level method that assigns to a place. @@ -538,14 +538,14 @@ impl State { /// Retrieve the value stored for a place, or ⊤ if it is not tracked. pub fn get(&self, place: PlaceRef<'_>, map: &Map) -> V { - map.find(place).map(|place| self.get_idx(place, map)).unwrap_or(V::top()) + map.find(place).map(|place| self.get_idx(place, map)).unwrap_or(V::TOP) } /// Retrieve the value stored for a place, or ⊤ if it is not tracked. pub fn get_discr(&self, place: PlaceRef<'_>, map: &Map) -> V { match map.find_discr(place) { Some(place) => self.get_idx(place, map), - None => V::top(), + None => V::TOP, } } @@ -553,11 +553,11 @@ impl State { pub fn get_idx(&self, place: PlaceIndex, map: &Map) -> V { match &self.0 { StateData::Reachable(values) => { - map.places[place].value_index.map(|v| values[v].clone()).unwrap_or(V::top()) + map.places[place].value_index.map(|v| values[v].clone()).unwrap_or(V::TOP) } StateData::Unreachable => { // Because this is unreachable, we can return any value we want. - V::bottom() + V::BOTTOM } } } @@ -909,9 +909,7 @@ pub enum ValueOrPlace { } impl ValueOrPlace { - pub fn top() -> Self { - ValueOrPlace::Value(V::top()) - } + pub const TOP: Self = ValueOrPlace::Value(V::TOP); } /// The set of projection elements that can be used by a tracked place. diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 254b704f9fc84..5ef1bd00a6f9a 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -208,8 +208,8 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { _ => unreachable!(), } .map(|result| ValueOrPlace::Value(self.wrap_immediate(result, *ty))) - .unwrap_or(ValueOrPlace::top()), - _ => ValueOrPlace::top(), + .unwrap_or(ValueOrPlace::TOP), + _ => ValueOrPlace::TOP, }, Rvalue::BinaryOp(op, box (left, right)) => { // Overflows must be ignored here. From 7c3d55150dc09494fda56814ff1bd529d72b6afb Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 22 Apr 2023 13:06:28 +0000 Subject: [PATCH 2/8] Create tracked places breadth first. --- .../rustc_mir_dataflow/src/value_analysis.rs | 114 ++++++++---------- 1 file changed, 49 insertions(+), 65 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index e830b8c715777..7d8fc5ffeec85 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -32,6 +32,7 @@ //! Because of that, we can assume that the only way to change the value behind a tracked place is //! by direct assignment. +use std::collections::VecDeque; use std::fmt::{Debug, Formatter}; use rustc_data_structures::fx::FxHashMap; @@ -608,7 +609,7 @@ impl Map { pub fn from_filter<'tcx>( tcx: TyCtxt<'tcx>, body: &Body<'tcx>, - filter: impl FnMut(Ty<'tcx>) -> bool, + filter: impl Fn(Ty<'tcx>) -> bool, place_limit: Option, ) -> Self { let mut map = Self::new(); @@ -623,51 +624,67 @@ impl Map { &mut self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>, - mut filter: impl FnMut(Ty<'tcx>) -> bool, + filter: impl Fn(Ty<'tcx>) -> bool, exclude: BitSet, place_limit: Option, ) { // We use this vector as stack, pushing and popping projections. - let mut projection = Vec::new(); + let mut worklist = VecDeque::with_capacity(place_limit.unwrap_or(body.local_decls.len())); + self.locals = IndexVec::from_elem(None, &body.local_decls); for (local, decl) in body.local_decls.iter_enumerated() { - if !exclude.contains(local) { - self.register_with_filter_rec( - tcx, - local, - &mut projection, - decl.ty, - &mut filter, - place_limit, - ); + if exclude.contains(local) { + continue; } + + // Create a place for the local. + debug_assert!(self.locals[local].is_none()); + let place = self.places.push(PlaceInfo::new(None)); + self.locals[local] = Some(place); + + // And push the eventual children places to the worklist. + self.register_children(tcx, place, decl.ty, &filter, &mut worklist); + } + + // `place.elem1.elem2` with type `ty`. + while let Some((mut place, elem1, elem2, ty)) = worklist.pop_front() { + if let Some(place_limit) = place_limit && self.value_count >= place_limit { + break + } + + // Create a place for this projection. + for elem in [elem1, Some(elem2)].into_iter().flatten() { + place = *self.projections.entry((place, elem)).or_insert_with(|| { + // Prepend new child to the linked list. + let next = self.places.push(PlaceInfo::new(Some(elem))); + self.places[next].next_sibling = self.places[place].first_child; + self.places[place].first_child = Some(next); + next + }); + } + + // And push the eventual children places to the worklist. + self.register_children(tcx, place, ty, &filter, &mut worklist); } } /// Potentially register the (local, projection) place and its fields, recursively. /// /// Invariant: The projection must only contain trackable elements. - fn register_with_filter_rec<'tcx>( + fn register_children<'tcx>( &mut self, tcx: TyCtxt<'tcx>, - local: Local, - projection: &mut Vec>, + place: PlaceIndex, ty: Ty<'tcx>, - filter: &mut impl FnMut(Ty<'tcx>) -> bool, - place_limit: Option, + filter: &impl Fn(Ty<'tcx>) -> bool, + worklist: &mut VecDeque<(PlaceIndex, Option, TrackElem, Ty<'tcx>)>, ) { - if let Some(place_limit) = place_limit && self.value_count >= place_limit { - return - } - - // We know that the projection only contains trackable elements. - let place = self.make_place(local, projection).unwrap(); - // Allocate a value slot if it doesn't have one, and the user requested one. if self.places[place].value_index.is_none() && filter(ty) { self.places[place].value_index = Some(self.value_count.into()); self.value_count += 1; } + // For enums, directly create the `Discriminant`, as that's their main use. if ty.is_enum() { let discr_ty = ty.discriminant_ty(tcx); if filter(discr_ty) { @@ -692,48 +709,15 @@ impl Map { // Recurse with all fields of this place. iter_fields(ty, tcx, ty::ParamEnv::reveal_all(), |variant, field, ty| { - if let Some(variant) = variant { - projection.push(PlaceElem::Downcast(None, variant)); - let _ = self.make_place(local, projection); - projection.push(PlaceElem::Field(field, ty)); - self.register_with_filter_rec(tcx, local, projection, ty, filter, place_limit); - projection.pop(); - projection.pop(); - return; - } - projection.push(PlaceElem::Field(field, ty)); - self.register_with_filter_rec(tcx, local, projection, ty, filter, place_limit); - projection.pop(); + worklist.push_back(( + place, + variant.map(TrackElem::Variant), + TrackElem::Field(field), + ty, + )) }); } - /// Tries to add the place to the map, without allocating a value slot. - /// - /// Can fail if the projection contains non-trackable elements. - fn make_place<'tcx>( - &mut self, - local: Local, - projection: &[PlaceElem<'tcx>], - ) -> Result { - // Get the base index of the local. - let mut index = - *self.locals.get_or_insert_with(local, || self.places.push(PlaceInfo::new(None))); - - // Apply the projection. - for &elem in projection { - let elem = elem.try_into()?; - index = *self.projections.entry((index, elem)).or_insert_with(|| { - // Prepend new child to the linked list. - let next = self.places.push(PlaceInfo::new(Some(elem))); - self.places[next].next_sibling = self.places[index].first_child; - self.places[index].first_child = Some(next); - next - }); - } - - Ok(index) - } - /// Returns the number of tracked places, i.e., those for which a value can be stored. pub fn tracked_places(&self) -> usize { self.value_count @@ -750,7 +734,7 @@ impl Map { place: PlaceRef<'_>, extra: impl IntoIterator, ) -> Option { - let mut index = *self.locals.get(place.local)?.as_ref()?; + let mut index = *self.locals[place.local].as_ref()?; for &elem in place.projection { index = self.apply(index, elem.try_into().ok()?)?; @@ -794,7 +778,7 @@ impl Map { // We do not track indirect places. return; } - let Some(&Some(mut index)) = self.locals.get(place.local) else { + let Some(mut index) = self.locals[place.local] else { // The local is not tracked at all, so it does not alias anything. return; }; From 38fa676330eb938ca9e8f36397a9003509e8be07 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 23 Apr 2023 18:02:05 +0000 Subject: [PATCH 3/8] Precompute values to flood. --- .../rustc_mir_dataflow/src/value_analysis.rs | 70 ++++++++++++++----- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 7d8fc5ffeec85..882f9dc11a127 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -34,6 +34,7 @@ use std::collections::VecDeque; use std::fmt::{Debug, Formatter}; +use std::ops::Range; use rustc_data_structures::fx::FxHashMap; use rustc_index::bit_set::BitSet; @@ -448,10 +449,8 @@ impl State { pub fn flood_with(&mut self, place: PlaceRef<'_>, map: &Map, value: V) { let StateData::Reachable(values) = &mut self.0 else { return }; - map.for_each_aliasing_place(place, None, &mut |place| { - if let Some(vi) = map.places[place].value_index { - values[vi] = value.clone(); - } + map.for_each_aliasing_place(place, None, &mut |vi| { + values[vi] = value.clone(); }); } @@ -461,10 +460,8 @@ impl State { pub fn flood_discr_with(&mut self, place: PlaceRef<'_>, map: &Map, value: V) { let StateData::Reachable(values) = &mut self.0 else { return }; - map.for_each_aliasing_place(place, Some(TrackElem::Discriminant), &mut |place| { - if let Some(vi) = map.places[place].value_index { - values[vi] = value.clone(); - } + map.for_each_aliasing_place(place, Some(TrackElem::Discriminant), &mut |vi| { + values[vi] = value.clone(); }); } @@ -589,6 +586,9 @@ pub struct Map { projections: FxHashMap<(PlaceIndex, TrackElem), PlaceIndex>, places: IndexVec, value_count: usize, + // The Range corresponds to a slice into `inner_values_buffer`. + inner_values: IndexVec>, + inner_values_buffer: Vec, } impl Map { @@ -598,6 +598,8 @@ impl Map { projections: FxHashMap::default(), places: IndexVec::new(), value_count: 0, + inner_values: IndexVec::new(), + inner_values_buffer: Vec::new(), } } @@ -665,6 +667,14 @@ impl Map { // And push the eventual children places to the worklist. self.register_children(tcx, place, ty, &filter, &mut worklist); } + + self.inner_values_buffer = Vec::with_capacity(self.value_count); + self.inner_values = IndexVec::from_elem(0..0, &self.places); + for local in body.local_decls.indices() { + if let Some(place) = self.locals[local] { + self.cache_preorder_invoke(place); + } + } } /// Potentially register the (local, projection) place and its fields, recursively. @@ -718,6 +728,25 @@ impl Map { }); } + /// Precompute the list of values inside `root` and store it inside + /// as a slice within `inner_values_buffer`. + fn cache_preorder_invoke(&mut self, root: PlaceIndex) { + let start = self.inner_values_buffer.len(); + if let Some(vi) = self.places[root].value_index { + self.inner_values_buffer.push(vi); + } + + // We manually iterate instead of using `children` as we need to mutate `self`. + let mut next_child = self.places[root].first_child; + while let Some(child) = next_child { + self.cache_preorder_invoke(child); + next_child = self.places[child].next_sibling; + } + + let end = self.inner_values_buffer.len(); + self.inner_values[root] = start..end; + } + /// Returns the number of tracked places, i.e., those for which a value can be stored. pub fn tracked_places(&self) -> usize { self.value_count @@ -768,11 +797,11 @@ impl Map { /// /// `tail_elem` allows to support discriminants that are not a place in MIR, but that we track /// as such. - pub fn for_each_aliasing_place( + fn for_each_aliasing_place( &self, place: PlaceRef<'_>, tail_elem: Option, - f: &mut impl FnMut(PlaceIndex), + f: &mut impl FnMut(ValueIndex), ) { if place.is_indirect() { // We do not track indirect places. @@ -789,7 +818,9 @@ impl Map { .chain(tail_elem.map(Ok).into_iter()); for elem in elems { // A field aliases the parent place. - f(index); + if let Some(vi) = self.places[index].value_index { + f(vi); + } let Ok(elem) = elem else { return }; let sub = self.apply(index, elem); @@ -803,7 +834,7 @@ impl Map { return; } } - self.preorder_invoke(index, f); + self.for_each_value_inside(index, f); } /// Invoke the given function on all the descendants of the given place, except one branch. @@ -811,7 +842,7 @@ impl Map { &self, parent: PlaceIndex, preserved_child: Option, - f: &mut impl FnMut(PlaceIndex), + f: &mut impl FnMut(ValueIndex), ) { for sibling in self.children(parent) { let elem = self.places[sibling].proj_elem; @@ -821,16 +852,17 @@ impl Map { // Only invalidate the other variants, the current one is fine. && Some(sibling) != preserved_child { - self.preorder_invoke(sibling, f); + self.for_each_value_inside(sibling, f); } } } - /// Invoke a function on the given place and all descendants. - fn preorder_invoke(&self, root: PlaceIndex, f: &mut impl FnMut(PlaceIndex)) { - f(root); - for child in self.children(root) { - self.preorder_invoke(child, f); + /// Invoke a function on each value in the given place and all descendants. + fn for_each_value_inside(&self, root: PlaceIndex, f: &mut impl FnMut(ValueIndex)) { + let range = self.inner_values[root].clone(); + let values = &self.inner_values_buffer[range]; + for &v in values { + f(v) } } } From 2b0bf3cf59dc54b9a5721e085405b7647fe82c14 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 24 Apr 2023 16:54:11 +0000 Subject: [PATCH 4/8] Trim the places that will not be used. --- compiler/rustc_mir_dataflow/src/value_analysis.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 882f9dc11a127..ecdc6a6580ead 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -675,6 +675,15 @@ impl Map { self.cache_preorder_invoke(place); } } + + // Trim useless places. + for opt_place in self.locals.iter_mut() { + if let Some(place) = *opt_place && self.inner_values[place].is_empty() { + *opt_place = None; + } + } + #[allow(rustc::potential_query_instability)] + self.projections.retain(|_, child| !self.inner_values[*child].is_empty()); } /// Potentially register the (local, projection) place and its fields, recursively. @@ -803,7 +812,7 @@ impl Map { tail_elem: Option, f: &mut impl FnMut(ValueIndex), ) { - if place.is_indirect() { + if place.has_deref() { // We do not track indirect places. return; } From add5124dceb157f1f4157bf2b0ec106e78a4943c Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 25 Apr 2023 19:47:15 +0000 Subject: [PATCH 5/8] Extract handle_set_discriminant. --- .../rustc_mir_dataflow/src/value_analysis.rs | 22 ++++++++++++-- .../src/dataflow_const_prop.rs | 30 +++++++++---------- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index ecdc6a6580ead..7b92eb05fba6c 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -67,8 +67,8 @@ pub trait ValueAnalysis<'tcx> { StatementKind::Assign(box (place, rvalue)) => { self.handle_assign(*place, rvalue, state); } - StatementKind::SetDiscriminant { box ref place, .. } => { - state.flood_discr(place.as_ref(), self.map()); + StatementKind::SetDiscriminant { box place, variant_index } => { + self.handle_set_discriminant(*place, *variant_index, state); } StatementKind::Intrinsic(box intrinsic) => { self.handle_intrinsic(intrinsic, state); @@ -94,6 +94,24 @@ pub trait ValueAnalysis<'tcx> { } } + fn handle_set_discriminant( + &self, + place: Place<'tcx>, + variant_index: VariantIdx, + state: &mut State, + ) { + self.super_set_discriminant(place, variant_index, state) + } + + fn super_set_discriminant( + &self, + place: Place<'tcx>, + _variant_index: VariantIdx, + state: &mut State, + ) { + state.flood_discr(place.as_ref(), self.map()); + } + fn handle_intrinsic( &self, intrinsic: &NonDivergingIntrinsic<'tcx>, diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 5ef1bd00a6f9a..7adfc9dff2ae9 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -79,22 +79,22 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { &self.map } - fn handle_statement(&self, statement: &Statement<'tcx>, state: &mut State) { - match statement.kind { - StatementKind::SetDiscriminant { box ref place, variant_index } => { - state.flood_discr(place.as_ref(), &self.map); - if self.map.find_discr(place.as_ref()).is_some() { - let enum_ty = place.ty(self.local_decls, self.tcx).ty; - if let Some(discr) = self.eval_discriminant(enum_ty, variant_index) { - state.assign_discr( - place.as_ref(), - ValueOrPlace::Value(FlatSet::Elem(discr)), - &self.map, - ); - } - } + fn handle_set_discriminant( + &self, + place: Place<'tcx>, + variant_index: VariantIdx, + state: &mut State, + ) { + state.flood_discr(place.as_ref(), &self.map); + if self.map.find_discr(place.as_ref()).is_some() { + let enum_ty = place.ty(self.local_decls, self.tcx).ty; + if let Some(discr) = self.eval_discriminant(enum_ty, variant_index) { + state.assign_discr( + place.as_ref(), + ValueOrPlace::Value(FlatSet::Elem(discr)), + &self.map, + ); } - _ => self.super_statement(statement, state), } } From 79c073746b46d621a75a5617737093db84dabee3 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 25 Apr 2023 19:50:22 +0000 Subject: [PATCH 6/8] Do not flood on copy_nonoverlapping. --- compiler/rustc_mir_dataflow/src/value_analysis.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 7b92eb05fba6c..2c3d4c97ed338 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -123,16 +123,18 @@ pub trait ValueAnalysis<'tcx> { fn super_intrinsic( &self, intrinsic: &NonDivergingIntrinsic<'tcx>, - state: &mut State, + _state: &mut State, ) { match intrinsic { NonDivergingIntrinsic::Assume(..) => { // Could use this, but ignoring it is sound. } - NonDivergingIntrinsic::CopyNonOverlapping(CopyNonOverlapping { dst, .. }) => { - if let Some(place) = dst.place() { - state.flood(place.as_ref(), self.map()); - } + NonDivergingIntrinsic::CopyNonOverlapping(CopyNonOverlapping { + dst: _, + src: _, + count: _, + }) => { + // This statement represents `*dst = *src`, `count` times. } } } From 2aa1c23fed118900103f70e4ab3473e75215862c Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 8 May 2023 08:46:21 +0000 Subject: [PATCH 7/8] Add a few comments. --- .../rustc_mir_dataflow/src/value_analysis.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 2c3d4c97ed338..9232ea74d3692 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -632,11 +632,11 @@ impl Map { tcx: TyCtxt<'tcx>, body: &Body<'tcx>, filter: impl Fn(Ty<'tcx>) -> bool, - place_limit: Option, + value_limit: Option, ) -> Self { let mut map = Self::new(); let exclude = excluded_locals(body); - map.register_with_filter(tcx, body, filter, exclude, place_limit); + map.register_with_filter(tcx, body, filter, exclude, value_limit); debug!("registered {} places ({} nodes in total)", map.value_count, map.places.len()); map } @@ -648,10 +648,11 @@ impl Map { body: &Body<'tcx>, filter: impl Fn(Ty<'tcx>) -> bool, exclude: BitSet, - place_limit: Option, + value_limit: Option, ) { - // We use this vector as stack, pushing and popping projections. - let mut worklist = VecDeque::with_capacity(place_limit.unwrap_or(body.local_decls.len())); + let mut worklist = VecDeque::with_capacity(value_limit.unwrap_or(body.local_decls.len())); + + // Start by constructing the places for each bare local. self.locals = IndexVec::from_elem(None, &body.local_decls); for (local, decl) in body.local_decls.iter_enumerated() { if exclude.contains(local) { @@ -668,8 +669,10 @@ impl Map { } // `place.elem1.elem2` with type `ty`. + // `elem1` is either `Some(Variant(i))` or `None`. while let Some((mut place, elem1, elem2, ty)) = worklist.pop_front() { - if let Some(place_limit) = place_limit && self.value_count >= place_limit { + // The user requires a bound on the number of created values. + if let Some(value_limit) = value_limit && self.value_count >= value_limit { break } @@ -688,6 +691,9 @@ impl Map { self.register_children(tcx, place, ty, &filter, &mut worklist); } + // Pre-compute the tree of ValueIndex nested in each PlaceIndex. + // `inner_values_buffer[inner_values[place]]` is the set of all the values + // reachable by projecting `place`. self.inner_values_buffer = Vec::with_capacity(self.value_count); self.inner_values = IndexVec::from_elem(0..0, &self.places); for local in body.local_decls.indices() { From ccc1da247bf3be7e71932844484847da6e35f185 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 8 May 2023 08:48:16 +0000 Subject: [PATCH 8/8] Prevent stack overflow. --- compiler/rustc_mir_dataflow/src/value_analysis.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 9232ea74d3692..b74d06e5ae8dd 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -37,6 +37,7 @@ use std::fmt::{Debug, Formatter}; use std::ops::Range; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_index::bit_set::BitSet; use rustc_index::{IndexSlice, IndexVec}; use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor}; @@ -774,7 +775,7 @@ impl Map { // We manually iterate instead of using `children` as we need to mutate `self`. let mut next_child = self.places[root].first_child; while let Some(child) = next_child { - self.cache_preorder_invoke(child); + ensure_sufficient_stack(|| self.cache_preorder_invoke(child)); next_child = self.places[child].next_sibling; }