8000 rustc_mir_transform: Make DestinationPropagation stable for queries by Enselic · Pull Request #119591 · rust-lang/rust · GitHub
[go: up one dir, main page]
Skip to content

rustc_mir_transform: Make DestinationPropagation stable for queries #119591

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 6, 2024
Merged

rustc_mir_transform: Make DestinationPropagation stable for queries #119591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
rustc_mir_transform: Make DestinationPropagation stable for queries 
By using FxIndexMap instead of FxHashMap, so that the order of visiting
of locals is deterministic.

We also need to bless
copy_propagation_arg.foo.DestinationPropagation.panic*.diff. Do not
review the diff of the diff. Instead look at the diff file before and
after this commit. Both before and after this commit, 3 statements are
replaced with nop. It's just that due to change in ordering, different
statements are replaced. But the net result is the same.
  • Loading branch information
@Enselic
Enselic committed Jan 5, 2024
commit 95eb5bcb67b97e7a1f3f6e6e68b3a8a0737ca24e
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/fx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub type StdEntry<'a, K, V> = std::collections::hash_map::Entry<'a, K, V>;
pub type FxIndexMap<K, V> = indexmap::IndexMap<K, V, BuildHasherDefault<FxHasher>>;
pub type FxIndexSet<V> = indexmap::IndexSet<V, BuildHasherDefault<FxHasher>>;
pub type IndexEntry<'a, K, V> = indexmap::map::Entry<'a, K, V>;
pub type IndexOccupiedEntry<'a, K, V> = indexmap::map::OccupiedEntry<'a, K, V>;

#[macro_export]
macro_rules! define_id_collections {
Expand Down
30 changes: 14 additions & 16 deletions compiler/rustc_mir_transform/src/dest_prop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,8 @@
//! [attempt 2]: https://github.com/rust-lang/rust/pull/71003
//! [attempt 3]: https://github.com/rust-lang/rust/pull/72632

use std::collections::hash_map::{Entry, OccupiedEntry};

use crate::MirPass;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::{FxIndexMap, IndexEntry, IndexOccupiedEntry};
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
use rustc_middle::mir::HasLocalDecls;
Expand Down Expand Up @@ -211,7 +209,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
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.
let mut merges = FxHashMap::default();
let mut merges = FxIndexMap::default();

for (src, candidates) in candidates.c.iter() {
if merged_locals.contains(*src) {
Expand Down Expand Up @@ -250,8 +248,8 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
/// frequently. Everything with a `&'alloc` lifetime points into here.
#[derive(Default)]
struct Allocations {
candidates: FxHashMap<Local, Vec<Local>>,
candidates_reverse: FxHashMap<Local, Vec<Local>>,
candidates: FxIndexMap<Local, Vec<Local>>,
candidates_reverse: FxIndexMap<Local, Vec<Local>>,
write_info: WriteInfo,
// PERF: Do this for `MaybeLiveLocals` allocations too.
}
Expand All @@ -272,11 +270,11 @@ struct Candidates<'alloc> {
///
/// We will still report that we would like to merge `_1` and `_2` in an attempt to allow us to
/// remove that assignment.
c: &'alloc mut FxHashMap<Local, Vec<Local>>,
c: &'alloc mut FxIndexMap<Local, Vec<Local>>,
/// A reverse index of the `c` set; if the `c` set contains `a => Place { local: b, proj }`,
/// then this contains `b => a`.
// PERF: Possibly these should be `SmallVec`s?
reverse: &'alloc mut FxHashMap<Local, Vec<Local>>,
reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
}

//////////////////////////////////////////////////////////
Expand All @@ -287,7 +285,7 @@ struct Candidates<'alloc> {
fn apply_merges<'tcx>(
body: &mut Body<'tcx>,
tcx: TyCtxt<'tcx>,
merges: &FxHashMap<Local, Local>,
merges: &FxIndexMap<Local, Local>,
merged_locals: &BitSet<Local>,
) {
let mut merger = Merger { tcx, merges, merged_locals };
Expand All @@ -296,7 +294,7 @@ fn apply_merges<'tcx>(

struct Merger<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
merges: &'a FxHashMap<Local, Local>,
merges: &'a FxIndexMap<Local, Local>,
merged_locals: &'a BitSet<Local>,
}

Expand Down Expand Up @@ -379,7 +377,7 @@ impl<'alloc> Candidates<'alloc> {

/// `vec_filter_candidates` but for an `Entry`
fn entry_filter_candidates(
mut entry: OccupiedEntry<'_, Local, Vec<Local>>,
mut entry: IndexOccupiedEntry<'_, Local, Vec<Local>>,
p: Local,
f: impl FnMut(Local) -> CandidateFilter,
at: Location,
Expand All @@ -399,7 +397,7 @@ impl<'alloc> Candidates<'alloc> {
at: Location,
) {
// Cover the cases where `p` appears as a `src`
if let Entry::Occupied(entry) = self.c.entry(p) {
if let IndexEntry::Occupied(entry) = self.c.entry(p) {
Self::entry_filter_candidates(entry, p, &mut f, at);
}
// And the cases where `p` appears as a `dest`
Expand All @@ -412,7 +410,7 @@ impl<'alloc> Candidates<'alloc> {
if f(*src) == CandidateFilter::Keep {
return true;
}
let Entry::Occupied(entry) = self.c.entry(*src) else {
let IndexEntry::Occupied(entry) = self.c.entry(*src) else {
return false;
};
Self::entry_filter_candidates(
Expand Down Expand Up @@ -721,8 +719,8 @@ fn places_to_candidate_pair<'tcx>(
fn find_candidates<'alloc, 'tcx>(
body: &Body<'tcx>,
borrowed: &BitSet<Local>,
candidates: &'alloc mut FxHashMap<Local, Vec<Local>>,
candidates_reverse: &'alloc mut < 8000 span class="x x-first x-last">FxHashMap<Local, Vec<Local>>,
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
candidates_reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
) -> Candidates<'alloc> {
candidates.clear();
candidates_reverse.clear();
Expand All @@ -744,7 +742,7 @@ fn find_candidates<'alloc, 'tcx>(

struct FindAssignments<'a, 'alloc, 'tcx> {
body: &'a Body<'tcx>,
candidates: &'alloc mut FxHashMap<Local, Vec<Local>>,
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
borrowed: &'a BitSet<Local>,
}

Expand Down
18 changes: 9 additions & 9 deletions tests/mir-opt/dest-prop/copy_propagation_arg.foo.DestinationPropagation.panic-abort.diff
View file
Open in desktop
ED4F
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@
let mut _3: u8;

bb0: {
- StorageLive(_2);
+ nop;
StorageLive(_3);
_3 = _1;
StorageLive(_2);
- StorageLive(_3);
- _3 = _1;
- _2 = dummy(move _3) -> [return: bb1, unwind unreachable];
+ _1 = dummy(move _3) -> [return: bb1, unwind unreachable];
+ nop;
+ nop;
+ _2 = dummy(move _1) -> [return: bb1, unwind unreachable];
}

bb1: {
StorageDead(_3);
- _1 = move _2;
- StorageDead(_2);
+ nop;
- StorageDead(_3);
+ nop;
_1 = move _2;
StorageDead(_2);
_0 = const ();
return;
}
Expand Down
18 changes: 9 additions & 9 deletions tests/mir-opt/dest-prop/copy_propagation_arg.foo.DestinationPropagation.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@
let mut _3: u8;

bb0: {
- StorageLive(_2);
+ nop;
StorageLive(_3);
_3 = _1;
StorageLive(_2);
- StorageLive(_3);
- _3 = _1;
- _2 = dummy(move _3) -> [return: bb1, unwind continue];
+ _1 = dummy(move _3) -> [return: bb1, unwind continue];
+ nop;
+ nop;
+ _2 = dummy(move _1) -> [return: bb1, unwind continue];
}

bb1: {
StorageDead(_3);
- _1 = move _2;
- StorageDead(_2);
+ nop;
- StorageDead(_3);
+ nop;
_1 = move _2;
StorageDead(_2);
_0 = const ();
return;
}
Expand Down
0