Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Don't track specific live points for promoteds #120003

Merged
merged 1 commit into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 72 additions & 17 deletions compiler/rustc_borrowck/src/region_infer/values.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::fx::FxIndexSet;
use rustc_index::bit_set::SparseBitMatrix;
use rustc_index::interval::IntervalSet;
Expand Down Expand Up @@ -41,8 +42,15 @@ pub(crate) struct LivenessValues {
/// The map from locations to points.
elements: Rc<DenseLocationMap>,

/// Which regions are live. This is exclusive with the fine-grained tracking in `points`, and
/// currently only used for validating promoteds (which don't care about more precise tracking).
live_regions: Option<FxHashSet<RegionVid>>,

/// For each region: the points where it is live.
points: SparseIntervalMatrix<RegionVid, PointIndex>,
///
/// This is not initialized for promoteds, because we don't care *where* within a promoted a
/// region is live, only that it is.
points: Option<SparseIntervalMatrix<RegionVid, PointIndex>>,

/// When using `-Zpolonius=next`, for each point: the loans flowing into the live regions at
/// that point.
Expand Down Expand Up @@ -71,24 +79,52 @@ impl LiveLoans {

impl LivenessValues {
/// Create an empty map of regions to locations where they're live.
pub(crate) fn new(elements: Rc<DenseLocationMap>) -> Self {
pub(crate) fn with_specific_points(elements: Rc<DenseLocationMap>) -> Self {
LivenessValues {
points: SparseIntervalMatrix::new(elements.num_points()),
live_regions: None,
points: Some(SparseIntervalMatrix::new(elements.num_points())),
elements,
loans: None,
}
}

/// Create an empty map of regions to locations where they're live.
///
/// Unlike `with_specific_points`, does not track exact locations where something is live, only
/// which regions are live.
pub(crate) fn without_specific_points(elements: Rc<DenseLocationMap>) -> Self {
LivenessValues {
live_regions: Some(Default::default()),
points: None,
elements,
loans: None,
}
}

/// Iterate through each region that has a value in this set.
pub(crate) fn regions(&self) -> impl Iterator<Item = RegionVid> {
self.points.rows()
pub(crate) fn regions(&self) -> impl Iterator<Item = RegionVid> + '_ {
self.points.as_ref().expect("use with_specific_points").rows()
}

/// Iterate through each region that has a value in this set.
// We are passing query instability implications to the caller.
#[rustc_lint_query_instability]
#[allow(rustc::potential_query_instability)]
pub(crate) fn live_regions_unordered(&self) -> impl Iterator<Item = RegionVid> + '_ {
self.live_regions.as_ref().unwrap().iter().copied()
}

/// Records `region` as being live at the given `location`.
pub(crate) fn add_location(&mut self, region: RegionVid, location: Location) {
debug!("LivenessValues::add_location(region={:?}, location={:?})", region, location);
let point = self.elements.point_from_location(location);
self.points.insert(region, point);
debug!("LivenessValues::add_location(region={:?}, location={:?})", region, location);
if let Some(points) = &mut self.points {
points.insert(region, point);
} else {
if self.elements.point_in_range(point) {
self.live_regions.as_mut().unwrap().insert(region);
}
}

// When available, record the loans flowing into this region as live at the given point.
if let Some(loans) = self.loans.as_mut() {
Expand All @@ -101,7 +137,13 @@ impl LivenessValues {
/// Records `region` as being live at all the given `points`.
pub(crate) fn add_points(&mut self, region: RegionVid, points: &IntervalSet<PointIndex>) {
debug!("LivenessValues::add_points(region={:?}, points={:?})", region, points);
self.points.union_row(region, points);
if let Some(this) = &mut self.points {
this.union_row(region, points);
} else {
if points.iter().any(|point| self.elements.point_in_range(point)) {
self.live_regions.as_mut().unwrap().insert(region);
}
}

// When available, record the loans flowing into this region as live at the given points.
if let Some(loans) = self.loans.as_mut() {
Expand All @@ -117,23 +159,33 @@ impl LivenessValues {

/// Records `region` as being live at all the control-flow points.
pub(crate) fn add_all_points(&mut self, region: RegionVid) {
self.points.insert_all_into_row(region);
if let Some(points) = &mut self.points {
points.insert_all_into_row(region);
} else {
self.live_regions.as_mut().unwrap().insert(region);
}
}

/// Returns whether `region` is marked live at the given `location`.
pub(crate) fn is_live_at(&self, region: RegionVid, location: Location) -> bool {
let point = self.elements.point_from_location(location);
self.points.row(region).is_some_and(|r| r.contains(point))
}

/// Returns whether `region` is marked live at any location.
pub(crate) fn is_live_anywhere(&self, region: RegionVid) -> bool {
self.live_points(region).next().is_some()
if let Some(points) = &self.points {
points.row(region).is_some_and(|r| r.contains(point))
} else {
unreachable!(
"Should be using LivenessValues::with_specific_points to ask whether live at a location"
)
}
}

/// Returns an iterator of all the points where `region` is live.
fn live_points(&self, region: RegionVid) -> impl Iterator<Item = PointIndex> + '_ {
self.points
let Some(points) = &self.points else {
unreachable!(
"Should be using LivenessValues::with_specific_points to ask whether live at a location"
)
};
points
.row(region)
.into_iter()
.flat_map(|set| set.iter())
Expand Down Expand Up @@ -288,7 +340,10 @@ impl<N: Idx> RegionValues<N> {
/// elements for the region `from` from `values` and add them to
/// the region `to` in `self`.
pub(crate) fn merge_liveness(&mut self, to: N, from: RegionVid, values: &LivenessValues) {
if let Some(set) = values.points.row(from) {
let Some(value_points) = &values.points else {
panic!("LivenessValues must track specific points for use in merge_liveness");
};
if let Some(set) = value_points.row(from) {
self.points.union_row(to, set);
}
}
Expand Down
28 changes: 15 additions & 13 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
let mut constraints = MirTypeckRegionConstraints {
placeholder_indices: PlaceholderIndices::default(),
placeholder_index_to_region: IndexVec::default(),
liveness_constraints: LivenessValues::new(elements.clone()),
liveness_constraints: LivenessValues::with_specific_points(elements.clone()),
outlives_constraints: OutlivesConstraintSet::default(),
member_constraints: MemberConstraintSet::default(),
type_tests: Vec::default(),
Expand Down Expand Up @@ -555,7 +555,7 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
let all_facts = &mut None;
let mut constraints = Default::default();
let mut liveness_constraints =
LivenessValues::new(Rc::new(DenseLocationMap::new(promoted_body)));
LivenessValues::without_specific_points(Rc::new(DenseLocationMap::new(promoted_body)));
// Don't try to add borrow_region facts for the promoted MIR

let mut swap_constraints = |this: &mut Self| {
Expand Down Expand Up @@ -594,17 +594,19 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
}
self.cx.borrowck_context.constraints.outlives_constraints.push(constraint)
}
for region in liveness_constraints.regions() {
// If the region is live at at least one location in the promoted MIR,
// then add a liveness constraint to the main MIR for this region
// at the location provided as an argument to this method
if liveness_constraints.is_live_anywhere(region) {
self.cx
.borrowck_context
.constraints
.liveness_constraints
.add_location(region, location);
}
// If the region is live at at least one location in the promoted MIR,
// then add a liveness constraint to the main MIR for this region
// at the location provided as an argument to this method
//
// add_location doesn't care about ordering so not a problem for the live regions to be
// unordered.
#[allow(rustc::potential_query_instability)]
for region in liveness_constraints.live_regions_unordered() {
self.cx
.borrowck_context
.constraints
.liveness_constraints
.add_location(region, location);
}
}

Expand Down
Loading