Skip to content

Commit

Permalink
Rollup merge of rust-lang#115291 - cjgillot:dest-prop-save, r=JakobDegen
Browse files Browse the repository at this point in the history
Save liveness results for DestinationPropagation

`DestinationPropagation` needs to verify that merge candidates do not conflict with each other. This is done by verifying that a local is not live when its counterpart is written to.

To get the liveness information, the pass runs `MaybeLiveLocals` dataflow analysis repeatedly, once for each propagation round. This is quite costly, and the main driver for the perf impact on `ucd` and `diesel`. (See rust-lang#115105 (comment))

In order to mitigate this cost, this PR proposes to save the result of the analysis into a `SparseIntervalMatrix`, and mirror merges of locals into that matrix: `liveness(destination) := liveness(destination) union liveness(source)`.

<details>
<summary>Proof</summary>

We denote by `'` all the quantities of the transformed program. Let $\varphi$ be a mapping of locals, which maps `source` to `destination`, and is identity otherwise. The exact liveness set after a statement is $out'(statement)$, and the proposed liveness set is $\varphi(out(statement))$.

Consider a statement. Suppose that the output state verifies $out' \subset phi(out)$. We want to prove that $in' \subset \varphi(in)$ where $in = (out - kill) \cup gen$, and conclude by induction.

We have 2 cases: either that statement is kept with locals renumbered by $\varphi$, or it is a tautological assignment and it removed.

1. If the statement is kept: the gen-set and the kill-set of $statement' = \varphi(statement)$ are $gen' = \varphi(gen)$ and $kill' = \varphi(kill)$ exactly.
From soundness requirement 3, $\varphi(in)$ is disjoint from $\varphi(kill)$.
This implies that $\varphi(out - kill)$ is disjoint from $\varphi(kill)$, and so $\varphi(out - kill) = \varphi(out) - \varphi(kill)$. Then $\varphi(in) = (\varphi(out) - \varphi(kill)) \cup \varphi(gen) = (\varphi(out) - kill') \cup gen'$.
We can conclude that $out' \subset \varphi(out) \implies in' \subset \varphi(in)$.

2. If the statement is removed. As $\varphi(statement)$ is a tautological assignment, we know that $\varphi(gen) = \varphi(kill) = \\{ destination \\}$, while $gen' = kill' = \emptyset$. So $\varphi(in) = \varphi(out) \cup \\{ destination \\}$. Then $in' = out' \subset out \subset \varphi(in)$.

By recursion, we can conclude by that $in' \subset \varphi(in)$ everywhere.
</details>

This approximate liveness results is only suboptimal if there are locals that fully disappear from the CFG due to an assignment cycle. These cases are quite unlikely, so we do not bother with them.

This change allows to reduce the perf impact of DestinationPropagation by half on diesel and ucd (rust-lang#115105 (comment)).

cc `@JakobDegen`
  • Loading branch information
compiler-errors authored Jan 17, 2024
2 parents 6ed31ab + 1d6723a commit cf3bedb
Show file tree
Hide file tree
Showing 10 changed files with 226 additions and 158 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_borrowck/src/nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, OpaqueHiddenType, TyCtxt};
use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
use rustc_mir_dataflow::move_paths::MoveData;
use rustc_mir_dataflow::points::DenseLocationMap;
use rustc_mir_dataflow::ResultsCursor;
use rustc_span::symbol::sym;
use std::env;
Expand All @@ -27,7 +28,7 @@ use crate::{
facts::{AllFacts, AllFactsExt, RustcFacts},
location::LocationTable,
polonius,
region_infer::{values::RegionValueElements, RegionInferenceContext},
region_infer::RegionInferenceContext,
renumber,
type_check::{self, MirTypeckRegionConstraints, MirTypeckResults},
universal_regions::UniversalRegions,
Expand Down Expand Up @@ -98,7 +99,7 @@ pub(crate) fn compute_regions<'cx, 'tcx>(

let universal_regions = Rc::new(universal_regions);

let elements = &Rc::new(RegionValueElements::new(body));
let elements = &Rc::new(DenseLocationMap::new(body));

// Run the MIR type-checker.
let MirTypeckResults { constraints, universal_region_relations, opaque_type_values } =
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc_middle::mir::{
use rustc_middle::traits::ObligationCause;
use rustc_middle::traits::ObligationCauseCode;
use rustc_middle::ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable, TypeVisitableExt};
use rustc_mir_dataflow::points::DenseLocationMap;
use rustc_span::Span;

use crate::constraints::graph::{self, NormalConstraintGraph, RegionGraph};
Expand All @@ -30,8 +31,7 @@ use crate::{
nll::PoloniusOutput,
region_infer::reverse_sccs::ReverseSccGraph,
region_infer::values::{
LivenessValues, PlaceholderIndices, RegionElement, RegionValueElements, RegionValues,
ToElementIndex,
LivenessValues, PlaceholderIndices, RegionElement, RegionValues, ToElementIndex,
},
type_check::{free_region_relations::UniversalRegionRelations, Locations},
universal_regions::UniversalRegions,
Expand Down Expand Up @@ -330,7 +330,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
universe_causes: FxIndexMap<ty::UniverseIndex, UniverseInfo<'tcx>>,
type_tests: Vec<TypeTest<'tcx>>,
liveness_constraints: LivenessValues,
elements: &Rc<RegionValueElements>,
elements: &Rc<DenseLocationMap>,
) -> Self {
debug!("universal_regions: {:#?}", universal_regions);
debug!("outlives constraints: {:#?}", outlives_constraints);
Expand Down
104 changes: 10 additions & 94 deletions compiler/rustc_borrowck/src/region_infer/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,97 +5,13 @@ use rustc_index::bit_set::SparseBitMatrix;
use rustc_index::interval::IntervalSet;
use rustc_index::interval::SparseIntervalMatrix;
use rustc_index::Idx;
use rustc_index::IndexVec;
use rustc_middle::mir::{BasicBlock, Body, Location};
use rustc_middle::mir::{BasicBlock, Location};
use rustc_middle::ty::{self, RegionVid};
use rustc_mir_dataflow::points::{DenseLocationMap, PointIndex};
use std::fmt::Debug;
use std::rc::Rc;

use crate::dataflow::BorrowIndex;

/// Maps between a `Location` and a `PointIndex` (and vice versa).
pub(crate) struct RegionValueElements {
/// For each basic block, how many points are contained within?
statements_before_block: IndexVec<BasicBlock, usize>,

/// Map backward from each point to the basic block that it
/// belongs to.
basic_blocks: IndexVec<PointIndex, BasicBlock>,

num_points: usize,
}

impl RegionValueElements {
pub(crate) fn new(body: &Body<'_>) -> Self {
let mut num_points = 0;
let statements_before_block: IndexVec<BasicBlock, usize> = body
.basic_blocks
.iter()
.map(|block_data| {
let v = num_points;
num_points += block_data.statements.len() + 1;
v
})
.collect();
debug!("RegionValueElements: statements_before_block={:#?}", statements_before_block);
debug!("RegionValueElements: num_points={:#?}", num_points);

let mut basic_blocks = IndexVec::with_capacity(num_points);
for (bb, bb_data) in body.basic_blocks.iter_enumerated() {
basic_blocks.extend((0..=bb_data.statements.len()).map(|_| bb));
}

Self { statements_before_block, basic_blocks, num_points }
}

/// Total number of point indices
pub(crate) fn num_points(&self) -> usize {
self.num_points
}

/// Converts a `Location` into a `PointIndex`. O(1).
pub(crate) fn point_from_location(&self, location: Location) -> PointIndex {
let Location { block, statement_index } = location;
let start_index = self.statements_before_block[block];
PointIndex::new(start_index + statement_index)
}

/// Converts a `Location` into a `PointIndex`. O(1).
pub(crate) fn entry_point(&self, block: BasicBlock) -> PointIndex {
let start_index = self.statements_before_block[block];
PointIndex::new(start_index)
}

/// Return the PointIndex for the block start of this index.
pub(crate) fn to_block_start(&self, index: PointIndex) -> PointIndex {
PointIndex::new(self.statements_before_block[self.basic_blocks[index]])
}

/// Converts a `PointIndex` back to a location. O(1).
pub(crate) fn to_location(&self, index: PointIndex) -> Location {
assert!(index.index() < self.num_points);
let block = self.basic_blocks[index];
let start_index = self.statements_before_block[block];
let statement_index = index.index() - start_index;
Location { block, statement_index }
}

/// Sometimes we get point-indices back from bitsets that may be
/// out of range (because they round up to the nearest 2^N number
/// of bits). Use this function to filter such points out if you
/// like.
pub(crate) fn point_in_range(&self, index: PointIndex) -> bool {
index.index() < self.num_points
}
}

rustc_index::newtype_index! {
/// A single integer representing a `Location` in the MIR control-flow
/// graph. Constructed efficiently from `RegionValueElements`.
#[orderable]
#[debug_format = "PointIndex({})"]
pub struct PointIndex {}
}
use crate::BorrowIndex;

rustc_index::newtype_index! {
/// A single integer representing a `ty::Placeholder`.
Expand Down Expand Up @@ -123,7 +39,7 @@ pub(crate) enum RegionElement {
/// an interval matrix storing liveness ranges for each region-vid.
pub(crate) struct LivenessValues {
/// The map from locations to points.
elements: Rc<RegionValueElements>,
elements: Rc<DenseLocationMap>,

/// For each region: the points where it is live.
points: SparseIntervalMatrix<RegionVid, PointIndex>,
Expand Down Expand Up @@ -155,9 +71,9 @@ impl LiveLoans {

impl LivenessValues {
/// Create an empty map of regions to locations where they're live.
pub(crate) fn new(elements: Rc<RegionValueElements>) -> Self {
pub(crate) fn new(elements: Rc<DenseLocationMap>) -> Self {
LivenessValues {
points: SparseIntervalMatrix::new(elements.num_points),
points: SparseIntervalMatrix::new(elements.num_points()),
elements,
loans: None,
}
Expand Down Expand Up @@ -298,7 +214,7 @@ impl PlaceholderIndices {
/// it would also contain various points from within the function.
#[derive(Clone)]
pub(crate) struct RegionValues<N: Idx> {
elements: Rc<RegionValueElements>,
elements: Rc<DenseLocationMap>,
placeholder_indices: Rc<PlaceholderIndices>,
points: SparseIntervalMatrix<N, PointIndex>,
free_regions: SparseBitMatrix<N, RegionVid>,
Expand All @@ -313,14 +229,14 @@ impl<N: Idx> RegionValues<N> {
/// Each of the regions in num_region_variables will be initialized with an
/// empty set of points and no causal information.
pub(crate) fn new(
elements: &Rc<RegionValueElements>,
elements: &Rc<DenseLocationMap>,
num_universal_regions: usize,
placeholder_indices: &Rc<PlaceholderIndices>,
) -> Self {
let num_placeholders = placeholder_indices.len();
Self {
elements: elements.clone(),
points: SparseIntervalMatrix::new(elements.num_points),
points: SparseIntervalMatrix::new(elements.num_points()),
placeholder_indices: placeholder_indices.clone(),
free_regions: SparseBitMatrix::new(num_universal_regions),
placeholders: SparseBitMatrix::new(num_placeholders),
Expand Down Expand Up @@ -486,7 +402,7 @@ impl ToElementIndex for ty::PlaceholderRegion {

/// For debugging purposes, returns a pretty-printed string of the given points.
pub(crate) fn pretty_print_points(
elements: &RegionValueElements,
elements: &DenseLocationMap,
points: impl IntoIterator<Item = PointIndex>,
) -> String {
pretty_print_region_elements(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use rustc_data_structures::vec_linked_list as vll;
use rustc_index::IndexVec;
use rustc_middle::mir::visit::{PlaceContext, Visitor};
use rustc_middle::mir::{Body, Local, Location};
use rustc_mir_dataflow::points::{DenseLocationMap, PointIndex};

use crate::def_use::{self, DefUse};
use crate::region_infer::values::{PointIndex, RegionValueElements};

/// A map that cross references each local with the locations where it
/// is defined (assigned), used, or dropped. Used during liveness
Expand Down Expand Up @@ -60,7 +60,7 @@ impl vll::LinkElem for Appearance {
impl LocalUseMap {
pub(crate) fn build(
live_locals: &[Local],
elements: &RegionValueElements,
elements: &DenseLocationMap,
body: &Body<'_>,
) -> Self {
let nones = IndexVec::from_elem(None, &body.local_decls);
Expand Down Expand Up @@ -103,7 +103,7 @@ impl LocalUseMap {

struct LocalUseMapBuild<'me> {
local_use_map: &'me mut LocalUseMap,
elements: &'me RegionValueElements,
elements: &'me DenseLocationMap,

// Vector used in `visit_local` to signal which `Local`s do we need
// def/use/drop information on, constructed from `live_locals` (that
Expand Down Expand Up @@ -144,7 +144,7 @@ impl LocalUseMapBuild<'_> {
}

fn insert(
elements: &RegionValueElements,
elements: &DenseLocationMap,
first_appearance: &mut Option<AppearanceIndex>,
appearances: &mut IndexVec<AppearanceIndex, Appearance>,
location: Location,
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_borrowck/src/type_check/liveness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ use rustc_middle::ty::visit::TypeVisitable;
use rustc_middle::ty::{GenericArgsRef, Region, RegionVid, Ty, TyCtxt};
use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
use rustc_mir_dataflow::move_paths::MoveData;
use rustc_mir_dataflow::points::DenseLocationMap;
use rustc_mir_dataflow::ResultsCursor;
use std::rc::Rc;

use crate::{
constraints::OutlivesConstraintSet,
facts::{AllFacts, AllFactsExt},
location::LocationTable,
region_infer::values::{LivenessValues, RegionValueElements},
region_infer::values::LivenessValues,
universal_regions::UniversalRegions,
};

Expand All @@ -34,7 +35,7 @@ mod trace;
pub(super) fn generate<'mir, 'tcx>(
typeck: &mut TypeChecker<'_, 'tcx>,
body: &Body<'tcx>,
elements: &Rc<RegionValueElements>,
elements: &Rc<DenseLocationMap>,
flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>,
move_data: &MoveData<'tcx>,
location_table: &LocationTable,
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_borrowck/src/type_check/liveness/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_infer::infer::outlives::for_liveness;
use rustc_middle::mir::{BasicBlock, Body, ConstraintCategory, Local, Location};
use rustc_middle::traits::query::DropckOutlivesResult;
use rustc_middle::ty::{Ty, TyCtxt, TypeVisitable, TypeVisitableExt};
use rustc_mir_dataflow::points::{DenseLocationMap, PointIndex};
use rustc_span::DUMMY_SP;
use rustc_trait_selection::traits::query::type_op::outlives::DropckOutlives;
use rustc_trait_selection::traits::query::type_op::{TypeOp, TypeOpOutput};
Expand All @@ -17,7 +18,7 @@ use rustc_mir_dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex};
use rustc_mir_dataflow::ResultsCursor;

use crate::{
region_infer::values::{self, LiveLoans, PointIndex, RegionValueElements},
region_infer::values::{self, LiveLoans},
type_check::liveness::local_use_map::LocalUseMap,
type_check::liveness::polonius,
type_check::NormalizeLocation,
Expand All @@ -41,7 +42,7 @@ use crate::{
pub(super) fn trace<'mir, 'tcx>(
typeck: &mut TypeChecker<'_, 'tcx>,
body: &Body<'tcx>,
elements: &Rc<RegionValueElements>,
elements: &Rc<DenseLocationMap>,
flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>,
move_data: &MoveData<'tcx>,
relevant_live_locals: Vec<Local>,
Expand Down Expand Up @@ -105,7 +106,7 @@ struct LivenessContext<'me, 'typeck, 'flow, 'tcx> {
typeck: &'me mut TypeChecker<'typeck, 'tcx>,

/// Defines the `PointIndex` mapping
elements: &'me RegionValueElements,
elements: &'me DenseLocationMap,

/// MIR we are analyzing.
body: &'me Body<'tcx>,
Expand Down Expand Up @@ -570,7 +571,7 @@ impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> {
}

fn make_all_regions_live(
elements: &RegionValueElements,
elements: &DenseLocationMap,
typeck: &mut TypeChecker<'_, 'tcx>,
value: impl TypeVisitable<TyCtxt<'tcx>>,
live_at: &IntervalSet<PointIndex>,
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use rustc_middle::ty::{
OpaqueHiddenType, OpaqueTypeKey, RegionVid, Ty, TyCtxt, UserType, UserTypeAnnotationIndex,
};
use rustc_middle::ty::{GenericArgsRef, UserArgs};
use rustc_mir_dataflow::points::DenseLocationMap;
use rustc_span::def_id::CRATE_DEF_ID;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::sym;
Expand All @@ -59,9 +60,7 @@ use crate::{
location::LocationTable,
member_constraints::MemberConstraintSet,
path_utils,
region_infer::values::{
LivenessValues, PlaceholderIndex, PlaceholderIndices, RegionValueElements,
},
region_infer::values::{LivenessValues, PlaceholderIndex, PlaceholderIndices},
region_infer::TypeTest,
type_check::free_region_relations::{CreateResult, UniversalRegionRelations},
universal_regions::{DefiningTy, UniversalRegions},
Expand Down Expand Up @@ -134,7 +133,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
all_facts: &mut Option<AllFacts>,
flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>,
move_data: &MoveData<'tcx>,
elements: &Rc<RegionValueElements>,
elements: &Rc<DenseLocationMap>,
upvars: &[&ty::CapturedPlace<'tcx>],
use_polonius: bool,
) -> MirTypeckResults<'tcx> {
Expand Down Expand Up @@ -546,7 +545,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(RegionValueElements::new(promoted_body)));
LivenessValues::new(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
1 change: 1 addition & 0 deletions compiler/rustc_mir_dataflow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ mod errors;
mod framework;
pub mod impls;
pub mod move_paths;
pub mod points;
pub mod rustc_peek;
pub mod storage;
pub mod un_derefer;
Expand Down
Loading

0 comments on commit cf3bedb

Please sign in to comment.