bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Rollup merge of #115291 - cjgillot:dest-prop-save, r=JakobDegen

Browse source 
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 https://github.com/rust-lang/rust/pull/115105#issuecomment-1689205908)

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 (https://github.com/rust-lang/rust/pull/115105#issuecomment-1694701904).

cc ````@JakobDegen````
This commit is contained in:
Matthias Krüger 2024-01-17 20:21:19 +01:00 • committed by GitHub
commit 3ae7ab698f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 226 additions and 158 deletions
Download patch file Download diff file Expand all files Collapse all files

81
compiler/rustc_mir_transform/src/dest_prop.rs
View file

@ -134,6 +134,7 @@
use crate::MirPass;
use rustc_data_structures::fx::{FxIndexMap, IndexEntry, IndexOccupiedEntry};
use rustc_index::bit_set::BitSet;
use rustc_index::interval::SparseIntervalMatrix;
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
use rustc_middle::mir::HasLocalDecls;
use rustc_middle::mir::{dump_mir, PassWhere};
@ -143,7 +144,8 @@ use rustc_middle::mir::{
};
use rustc_middle::ty::TyCtxt;
use rustc_mir_dataflow::impls::MaybeLiveLocals;
use rustc_mir_dataflow::{Analysis, ResultsCursor};
use rustc_mir_dataflow::points::{save_as_intervals, DenseLocationMap, PointIndex};
use rustc_mir_dataflow::Analysis;
pub struct DestinationPropagation;
@ -167,6 +169,13 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
let borrowed = rustc_mir_dataflow::impls::borrowed_locals(body);
let live = MaybeLiveLocals
.into_engine(tcx, body)
.pass_name("MaybeLiveLocals-DestinationPropagation")
.iterate_to_fixpoint();
let points = DenseLocationMap::new(body);
let mut live = save_as_intervals(&points, body, live);
// In order to avoid having to collect data for every single pair of locals in the body, we
// do not allow doing more than one merge for places that are derived from the same local at
// once. To avoid missed opportunities, we instead iterate to a fixed point - we'll refer to
@ -190,22 +199,19 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
&mut allocations.candidates_reverse,
);
trace!(?candidates);
let mut live = MaybeLiveLocals
.into_engine(tcx, body)
.iterate_to_fixpoint()
.into_results_cursor(body);
dest_prop_mir_dump(tcx, body, &mut live, round_count);
dest_prop_mir_dump(tcx, body, &points, &live, round_count);
FilterInformation::filter_liveness(
&mut candidates,
&mut live,
&points,
&live,
&mut allocations.write_info,
body,
);
// Because we do not update liveness information, it is unsound to use a local for more
// than one merge operation within a single round of optimizations. We store here which
// ones we have already used.
// Because we only filter once per round, it is unsound to use a local for more than
// one merge operation within a single round of optimizations. We store here which ones
// we have already used.
let mut merged_locals: BitSet<Local> = BitSet::new_empty(body.local_decls.len());
// This is the set of merges we will apply this round. It is a subset of the candidates.
@ -224,9 +230,15 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
}) {
break;
}
// Replace `src` by `dest` everywhere.
merges.insert(*src, *dest);
merged_locals.insert(*src);
merged_locals.insert(*dest);
// Update liveness information based on the merge we just performed.
// Every location where `src` was live, `dest` will be live.
live.union_rows(*src, *dest);
}
trace!(merging = ?merges);
@ -349,7 +361,8 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> {
struct FilterInformation<'a, 'body, 'alloc, 'tcx> {
body: &'body Body<'tcx>,
live: &'a mut ResultsCursor<'body, 'tcx, MaybeLiveLocals>,
points: &'a DenseLocationMap,
live: &'a SparseIntervalMatrix<Local, PointIndex>,
candidates: &'a mut Candidates<'alloc>,
write_info: &'alloc mut WriteInfo,
at: Location,
@ -452,12 +465,14 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
/// locals as also being read from.
fn filter_liveness<'b>(
candidates: &mut Candidates<'alloc>,
live: &mut ResultsCursor<'b, 'tcx, MaybeLiveLocals>,
points: &DenseLocationMap,
live: &SparseIntervalMatrix<Local, PointIndex>,
write_info_alloc: &'alloc mut WriteInfo,
body: &'b Body<'tcx>,
) {
let mut this = FilterInformation {
body,
points,
live,
candidates,
// We don't actually store anything at this scope, we just keep things here to be able
@ -472,13 +487,11 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
fn internal_filter_liveness(&mut self) {
for (block, data) in traversal::preorder(self.body) {
self.at = Location { block, statement_index: data.statements.len() };
self.live.seek_after_primary_effect(self.at);
self.write_info.for_terminator(&data.terminator().kind);
self.apply_conflicts();
for (i, statement) in data.statements.iter().enumerate().rev() {
self.at = Location { block, statement_index: i };
self.live.seek_after_primary_effect(self.at);
self.write_info.for_statement(&statement.kind, self.body);
self.apply_conflicts();
}
@ -497,6 +510,7 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
None
}
});
let at = self.points.point_from_location(self.at);
self.candidates.filter_candidates_by(
*p,
|q| {
@ -508,7 +522,7 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> {
// calls or inline asm. Because of this, we also mark locals as
// conflicting when both of them are written to in the same
// statement.
if self.live.contains(q) || writes.contains(&q) {
if self.live.contains(q, at) || writes.contains(&q) {
CandidateFilter::Remove
} else {
CandidateFilter::Keep
@ -801,38 +815,17 @@ fn is_local_required(local: Local, body: &Body<'_>) -> bool {
fn dest_prop_mir_dump<'body, 'tcx>(
tcx: TyCtxt<'tcx>,
body: &'body Body<'tcx>,
live: &mut ResultsCursor<'body, 'tcx, MaybeLiveLocals>,
points: &DenseLocationMap,
live: &SparseIntervalMatrix<Local, PointIndex>,
round: usize,
) {
let mut reachable = None;
let locals_live_at = |location| {
let location = points.point_from_location(location);
live.rows().filter(|&r| live.contains(r, location)).collect::<Vec<_>>()
};
dump_mir(tcx, false, "DestinationPropagation-dataflow", &round, body, |pass_where, w| {
let reachable = reachable.get_or_insert_with(|| traversal::reachable_as_bitset(body));
match pass_where {
PassWhere::BeforeLocation(loc) if reachable.contains(loc.block) => {
live.seek_after_primary_effect(loc);
writeln!(w, " // live: {:?}", live.get())?;
}
PassWhere::AfterTerminator(bb) if reachable.contains(bb) => {
let loc = body.terminator_loc(bb);
live.seek_before_primary_effect(loc);
writeln!(w, " // live: {:?}", live.get())?;
}
PassWhere::BeforeBlock(bb) if reachable.contains(bb) => {
live.seek_to_block_start(bb);
writeln!(w, " // live: {:?}", live.get())?;
}
PassWhere::BeforeCFG | PassWhere::AfterCFG | PassWhere::AfterLocation(_) => {}
PassWhere::BeforeLocation(_) | PassWhere::AfterTerminator(_) => {
writeln!(w, " // live: <unreachable>")?;
}
PassWhere::BeforeBlock(_) => {
writeln!(w, " // live: <unreachable>")?;
}
if let PassWhere::BeforeLocation(loc) = pass_where {
writeln!(w, " // live: {:?}", locals_live_at(loc))?;
}
Ok(())