Skip to content

Commit

Permalink
Do not recompute liveness for DestinationPropagation.
Browse files Browse the repository at this point in the history
  • Loading branch information
cjgillot committed Oct 21, 2023
1 parent 5f389e7 commit d3f39d7
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 44 deletions.
65 changes: 64 additions & 1 deletion compiler/rustc_mir_dataflow/src/points.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use crate::framework::{visit_results, ResultsVisitable, ResultsVisitor};
use rustc_index::bit_set::ChunkedBitSet;
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::{self, BasicBlock, Body, Location};
use std::rc::Rc;

/// Maps between a `Location` and a `PointIndex` (and vice versa).
Expand Down Expand Up @@ -161,4 +163,65 @@ impl<N: Idx> LivenessValues<N> {
pub fn get_intervals(&self, region: N) -> Option<&IntervalSet<PointIndex>> {
self.points.row(region)
}

/// Insert all the elements from the `src` region into the `dest` region.
pub fn union_regions(&mut self, src: N, dest: N) {
self.points.union_rows(src, dest);
}

/// Add points depending on the result of the given dataflow analysis.
pub fn fill_from_dataflow<'tcx, R>(body: &mir::Body<'tcx>, mut results: R) -> Self
where
R: ResultsVisitable<'tcx, FlowState = ChunkedBitSet<N>>,
{
let elements = Rc::new(DenseLocationMap::new(body));
let values = Self::new(elements);
let mut visitor = Visitor { values };
visit_results(
body,
body.basic_blocks.reverse_postorder().iter().copied(),
&mut results,
&mut visitor,
);
visitor.values
}
}

struct Visitor<N: Idx> {
values: LivenessValues<N>,
}

impl<'mir, 'tcx, R, N> ResultsVisitor<'mir, 'tcx, R> for Visitor<N>
where
N: Idx,
{
type FlowState = ChunkedBitSet<N>;

fn visit_statement_after_primary_effect(
&mut self,
_results: &mut R,
state: &Self::FlowState,
_statement: &'mir mir::Statement<'tcx>,
location: Location,
) {
let point = self.values.elements.point_from_location(location);
// Use internal iterator manually as it is much more efficient.
state.iter().fold((), |(), node| {
self.values.points.insert(node, point);
});
}

fn visit_terminator_after_primary_effect(
&mut self,
_results: &mut R,
state: &Self::FlowState,
_terminator: &'mir mir::Terminator<'tcx>,
location: Location,
) {
let point = self.values.elements.point_from_location(location);
// Use internal iterator manually as it is much more efficient.
state.iter().fold((), |(), node| {
self.values.points.insert(node, point);
});
}
}
69 changes: 26 additions & 43 deletions compiler/rustc_mir_transform/src/dest_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,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::LivenessValues;
use rustc_mir_dataflow::Analysis;

pub struct DestinationPropagation;

Expand All @@ -170,6 +171,12 @@ 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 mut live = LivenessValues::fill_from_dataflow(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
Expand All @@ -193,11 +200,7 @@ 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, &live, round_count);

FilterInformation::filter_liveness(
&mut candidates,
Expand All @@ -206,9 +209,9 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
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.
Expand All @@ -227,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_regions(*src, *dest);
}
trace!(merging = ?merges);

Expand Down Expand Up @@ -358,7 +367,7 @@ 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>,
live: &'a LivenessValues<Local>,
candidates: &'a mut Candidates<'alloc>,
write_info: &'alloc mut WriteInfo,
at: Location,
Expand Down Expand Up @@ -461,7 +470,7 @@ 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>,
live: &LivenessValues<Local>,
write_info_alloc: &'alloc mut WriteInfo,
body: &'b Body<'tcx>,
) {
Expand All @@ -481,13 +490,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();
}
Expand Down Expand Up @@ -517,7 +524,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, self.at) || writes.contains(&q) {
CandidateFilter::Remove
} else {
CandidateFilter::Keep
Expand Down Expand Up @@ -810,38 +817,14 @@ 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>,
live: &LivenessValues<Local>,
round: usize,
) {
let mut reachable = None;
let locals_live_at =
|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(())
Expand Down

0 comments on commit d3f39d7

Please sign in to comment.