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.
This commit is contained in:
Martin Nordholts 2024-01-03 06:35:31 +01:00
parent 11035f9f52
commit 95eb5bcb67
4 changed files with 33 additions and 34 deletions

View File

@ -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 FxIndexMap<K, V> = indexmap::IndexMap<K, V, BuildHasherDefault<FxHasher>>;
pub type FxIndexSet<V> = indexmap::IndexSet<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 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_export]
macro_rules! define_id_collections { macro_rules! define_id_collections {

View File

@ -131,10 +131,8 @@
//! [attempt 2]: https://github.com/rust-lang/rust/pull/71003 //! [attempt 2]: https://github.com/rust-lang/rust/pull/71003
//! [attempt 3]: https://github.com/rust-lang/rust/pull/72632 //! [attempt 3]: https://github.com/rust-lang/rust/pull/72632
use std::collections::hash_map::{Entry, OccupiedEntry};
use crate::MirPass; 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_index::bit_set::BitSet;
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor}; use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
use rustc_middle::mir::HasLocalDecls; use rustc_middle::mir::HasLocalDecls;
@ -211,7 +209,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
let mut merged_locals: BitSet<Local> = BitSet::new_empty(body.local_decls.len()); 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. // 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() { for (src, candidates) in candidates.c.iter() {
if merged_locals.contains(*src) { if merged_locals.contains(*src) {
@ -250,8 +248,8 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
/// frequently. Everything with a `&'alloc` lifetime points into here. /// frequently. Everything with a `&'alloc` lifetime points into here.
#[derive(Default)] #[derive(Default)]
struct Allocations { struct Allocations {
candidates: FxHashMap<Local, Vec<Local>>, candidates: FxIndexMap<Local, Vec<Local>>,
candidates_reverse: FxHashMap<Local, Vec<Local>>, candidates_reverse: FxIndexMap<Local, Vec<Local>>,
write_info: WriteInfo, write_info: WriteInfo,
// PERF: Do this for `MaybeLiveLocals` allocations too. // PERF: Do this for `MaybeLiveLocals` allocations too.
} }
@ -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 /// We will still report that we would like to merge `_1` and `_2` in an attempt to allow us to
/// remove that assignment. /// 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 }`, /// A reverse index of the `c` set; if the `c` set contains `a => Place { local: b, proj }`,
/// then this contains `b => a`. /// then this contains `b => a`.
// PERF: Possibly these should be `SmallVec`s? // PERF: Possibly these should be `SmallVec`s?
reverse: &'alloc mut FxHashMap<Local, Vec<Local>>, reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
} }
////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////
@ -287,7 +285,7 @@ struct Candidates<'alloc> {
fn apply_merges<'tcx>( fn apply_merges<'tcx>(
body: &mut Body<'tcx>, body: &mut Body<'tcx>,
tcx: TyCtxt<'tcx>, tcx: TyCtxt<'tcx>,
merges: &FxHashMap<Local, Local>, merges: &FxIndexMap<Local, Local>,
merged_locals: &BitSet<Local>, merged_locals: &BitSet<Local>,
) { ) {
let mut merger = Merger { tcx, merges, merged_locals }; let mut merger = Merger { tcx, merges, merged_locals };
@ -296,7 +294,7 @@ fn apply_merges<'tcx>(
struct Merger<'a, 'tcx> { struct Merger<'a, 'tcx> {
tcx: TyCtxt<'tcx>, tcx: TyCtxt<'tcx>,
merges: &'a FxHashMap<Local, Local>, merges: &'a FxIndexMap<Local, Local>,
merged_locals: &'a BitSet<Local>, merged_locals: &'a BitSet<Local>,
} }
@ -379,7 +377,7 @@ impl<'alloc> Candidates<'alloc> {
/// `vec_filter_candidates` but for an `Entry` /// `vec_filter_candidates` but for an `Entry`
fn entry_filter_candidates( fn entry_filter_candidates(
mut entry: OccupiedEntry<'_, Local, Vec<Local>>, mut entry: IndexOccupiedEntry<'_, Local, Vec<Local>>,
p: Local, p: Local,
f: impl FnMut(Local) -> CandidateFilter, f: impl FnMut(Local) -> CandidateFilter,
at: Location, at: Location,
@ -399,7 +397,7 @@ impl<'alloc> Candidates<'alloc> {
at: Location, at: Location,
) { ) {
// Cover the cases where `p` appears as a `src` // 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); Self::entry_filter_candidates(entry, p, &mut f, at);
} }
// And the cases where `p` appears as a `dest` // And the cases where `p` appears as a `dest`
@ -412,7 +410,7 @@ impl<'alloc> Candidates<'alloc> {
if f(*src) == CandidateFilter::Keep { if f(*src) == CandidateFilter::Keep {
return true; return true;
} }
let Entry::Occupied(entry) = self.c.entry(*src) else { let IndexEntry::Occupied(entry) = self.c.entry(*src) else {
return false; return false;
}; };
Self::entry_filter_candidates( Self::entry_filter_candidates(
@ -721,8 +719,8 @@ fn places_to_candidate_pair<'tcx>(
fn find_candidates<'alloc, 'tcx>( fn find_candidates<'alloc, 'tcx>(
body: &Body<'tcx>, body: &Body<'tcx>,
borrowed: &BitSet<Local>, borrowed: &BitSet<Local>,
candidates: &'alloc mut FxHashMap<Local, Vec<Local>>, candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
candidates_reverse: &'alloc mut FxHashMap<Local, Vec<Local>>, candidates_reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
) -> Candidates<'alloc> { ) -> Candidates<'alloc> {
candidates.clear(); candidates.clear();
candidates_reverse.clear(); candidates_reverse.clear();
@ -744,7 +742,7 @@ fn find_candidates<'alloc, 'tcx>(
struct FindAssignments<'a, 'alloc, 'tcx> { struct FindAssignments<'a, 'alloc, 'tcx> {
body: &'a Body<'tcx>, body: &'a Body<'tcx>,
candidates: &'alloc mut FxHashMap<Local, Vec<Local>>, candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
borrowed: &'a BitSet<Local>, borrowed: &'a BitSet<Local>,
} }

View File

@ -8,20 +8,20 @@
let mut _3: u8; let mut _3: u8;
bb0: { bb0: {
- StorageLive(_2); StorageLive(_2);
+ nop; - StorageLive(_3);
StorageLive(_3); - _3 = _1;
_3 = _1;
- _2 = dummy(move _3) -> [return: bb1, unwind unreachable]; - _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: { bb1: {
StorageDead(_3); - StorageDead(_3);
- _1 = move _2;
- StorageDead(_2);
+ nop;
+ nop; + nop;
_1 = move _2;
StorageDead(_2);
_0 = const (); _0 = const ();
return; return;
} }

View File

@ -8,20 +8,20 @@
let mut _3: u8; let mut _3: u8;
bb0: { bb0: {
- StorageLive(_2); StorageLive(_2);
+ nop; - StorageLive(_3);
StorageLive(_3); - _3 = _1;
_3 = _1;
- _2 = dummy(move _3) -> [return: bb1, unwind continue]; - _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: { bb1: {
StorageDead(_3); - StorageDead(_3);
- _1 = move _2;
- StorageDead(_2);
+ nop;
+ nop; + nop;
_1 = move _2;
StorageDead(_2);
_0 = const (); _0 = const ();
return; return;
} }