Skip to content

Commit

Permalink
Auto merge of #120003 - Mark-Simulacrum:opt-promoted, r=davidtwco
Browse files Browse the repository at this point in the history
perf: Don't track specific live points for promoteds

We don't query this information out of the promoted (it's basically a single "unit" regardless of the complexity within it) and this saves on re-initializing the SparseIntervalMatrix's backing IndexVec with mostly empty rows for all of the leading regions in the function. Typical promoteds will only contain a few regions that need up be uplifted, while the parent function can have thousands.

For a simple function repeating println!("Hello world"); 50,000 times this reduces compile times from 90 to 15 seconds in debug mode. The previous implementations re-initialization led to an overall roughly n^2 runtime as each promoted initialized slots for ~n regions, now we scale closer to linearly (5000 hello worlds takes 1.1 seconds).

cc #50994, #86244
  • Loading branch information
bors committed Jan 20, 2024
2 parents 314384b + c3364a2 commit 227abac
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 30 deletions.
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

0 comments on commit 227abac

Please sign in to comment.