Skip to content

Commit f603bab

Browse files
committed
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.
1 parent 090d5ea commit f603bab

File tree

4 files changed

+33
-34
lines changed

4 files changed

+33
-34
lines changed

compiler/rustc_data_structures/src/fx.rs

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ pub type StdEntry<'a, K, V> = std::collections::hash_map::Entry<'a, K, V>;
77
pub type FxIndexMap<K, V> = indexmap::IndexMap<K, V, BuildHasherDefault<FxHasher>>;
88
pub type FxIndexSet<V> = indexmap::IndexSet<V, BuildHasherDefault<FxHasher>>;
99
pub type IndexEntry<'a, K, V> = indexmap::map::Entry<'a, K, V>;
10+
pub type IndexOccupiedEntry<'a, K, V> = indexmap::map::OccupiedEntry<'a, K, V>;
1011

1112
#[macro_export]
1213
macro_rules! define_id_collections {

compiler/rustc_mir_transform/src/dest_prop.rs

+14-16
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,9 @@
131131
//! [attempt 2]: https://github.com/rust-lang/rust/pull/71003
132132
//! [attempt 3]: https://github.com/rust-lang/rust/pull/72632
133133
134-
use std::collections::hash_map::{Entry, OccupiedEntry};
135-
136134
use crate::simplify::remove_dead_blocks;
137135
use crate::MirPass;
138-
use rustc_data_structures::fx::FxHashMap;
136+
use rustc_data_structures::fx::{FxIndexMap, IndexEntry, IndexOccupiedEntry};
139137
use rustc_index::bit_set::BitSet;
140138
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
141139
use rustc_middle::mir::HasLocalDecls;
@@ -212,7 +210,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
212210
let mut merged_locals: BitSet<Local> = BitSet::new_empty(body.local_decls.len());
213211

214212
// This is the set of merges we will apply this round. It is a subset of the candidates.
215-
let mut merges = FxHashMap::default();
213+
let mut merges = FxIndexMap::default();
216214

217215
for (src, candidates) in candidates.c.iter() {
218216
if merged_locals.contains(*src) {
@@ -257,8 +255,8 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
257255
/// frequently. Everything with a `&'alloc` lifetime points into here.
258256
#[derive(Default)]
259257
struct Allocations {
260-
candidates: FxHashMap<Local, Vec<Local>>,
261-
candidates_reverse: FxHashMap<Local, Vec<Local>>,
258+
candidates: FxIndexMap<Local, Vec<Local>>,
259+
candidates_reverse: FxIndexMap<Local, Vec<Local>>,
262260
write_info: WriteInfo,
263261
// PERF: Do this for `MaybeLiveLocals` allocations too.
264262
}
@@ -279,11 +277,11 @@ struct Candidates<'alloc> {
279277
///
280278
/// We will still report that we would like to merge `_1` and `_2` in an attempt to allow us to
281279
/// remove that assignment.
282-
c: &'alloc mut FxHashMap<Local, Vec<Local>>,
280+
c: &'alloc mut FxIndexMap<Local, Vec<Local>>,
283281
/// A reverse index of the `c` set; if the `c` set contains `a => Place { local: b, proj }`,
284282
/// then this contains `b => a`.
285283
// PERF: Possibly these should be `SmallVec`s?
286-
reverse: &'alloc mut FxHashMap<Local, Vec<Local>>,
284+
reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
287285
}
288286

289287
//////////////////////////////////////////////////////////
@@ -294,7 +292,7 @@ struct Candidates<'alloc> {
294292
fn apply_merges<'tcx>(
295293
body: &mut Body<'tcx>,
296294
tcx: TyCtxt<'tcx>,
297-
merges: &FxHashMap<Local, Local>,
295+
merges: &FxIndexMap<Local, Local>,
298296
merged_locals: &BitSet<Local>,
299297
) {
300298
let mut merger = Merger { tcx, merges, merged_locals };
@@ -303,7 +301,7 @@ fn apply_merges<'tcx>(
303301

304302
struct Merger<'a, 'tcx> {
305303
tcx: TyCtxt<'tcx>,
306-
merges: &'a FxHashMap<Local, Local>,
304+
merges: &'a FxIndexMap<Local, Local>,
307305
merged_locals: &'a BitSet<Local>,
308306
}
309307

@@ -386,7 +384,7 @@ impl<'alloc> Candidates<'alloc> {
386384

387385
/// `vec_filter_candidates` but for an `Entry`
388386
fn entry_filter_candidates(
389-
mut entry: OccupiedEntry<'_, Local, Vec<Local>>,
387+
mut entry: IndexOccupiedEntry<'_, Local, Vec<Local>>,
390388
p: Local,
391389
f: impl FnMut(Local) -> CandidateFilter,
392390
at: Location,
@@ -406,7 +404,7 @@ impl<'alloc> Candidates<'alloc> {
406404
at: Location,
407405
) {
408406
// Cover the cases where `p` appears as a `src`
409-
if let Entry::Occupied(entry) = self.c.entry(p) {
407+
if let IndexEntry::Occupied(entry) = self.c.entry(p) {
410408
Self::entry_filter_candidates(entry, p, &mut f, at);
411409
}
412410
// And the cases where `p` appears as a `dest`
@@ -419,7 +417,7 @@ impl<'alloc> Candidates<'alloc> {
419417
if f(*src) == CandidateFilter::Keep {
420418
return true;
421419
}
422-
let Entry::Occupied(entry) = self.c.entry(*src) else {
420+
let IndexEntry::Occupied(entry) = self.c.entry(*src) else {
423421
return false;
424422
};
425423
Self::entry_filter_candidates(
@@ -728,8 +726,8 @@ fn places_to_candidate_pair<'tcx>(
728726
fn find_candidates<'alloc, 'tcx>(
729727
body: &Body<'tcx>,
730728
borrowed: &BitSet<Local>,
731-
candidates: &'alloc mut FxHashMap<Local, Vec<Local>>,
732-
candidates_reverse: &'alloc mut FxHashMap<Local, Vec<Local>>,
729+
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
730+
candidates_reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
733731
) -> Candidates<'alloc> {
734732
candidates.clear();
735733
candidates_reverse.clear();
@@ -751,7 +749,7 @@ fn find_candidates<'alloc, 'tcx>(
751749

752750
struct FindAssignments<'a, 'alloc, 'tcx> {
753751
body: &'a Body<'tcx>,
754-
candidates: &'alloc mut FxHashMap<Local, Vec<Local>>,
752+
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
755753
borrowed: &'a BitSet<Local>,
756754
}
757755

tests/mir-opt/dest-prop/copy_propagation_arg.foo.DestinationPropagation.panic-abort.diff

+9-9
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,20 @@
88
let mut _3: u8;
99

1010
bb0: {
11-
- StorageLive(_2);
12-
+ nop;
13-
StorageLive(_3);
14-
_3 = _1;
11+
StorageLive(_2);
12+
- StorageLive(_3);
13+
- _3 = _1;
1514
- _2 = dummy(move _3) -> [return: bb1, unwind unreachable];
16-
+ _1 = dummy(move _3) -> [return: bb1, unwind unreachable];
15+
+ nop;
16+
+ nop;
17+
+ _2 = dummy(move _1) -> [return: bb1, unwind unreachable];
1718
}
1819

1920
bb1: {
20-
StorageDead(_3);
21-
- _1 = move _2;
22-
- StorageDead(_2);
23-
+ nop;
21+
- StorageDead(_3);
2422
+ nop;
23+
_1 = move _2;
24+
StorageDead(_2);
2525
_0 = const ();
2626
return;
2727
}

tests/mir-opt/dest-prop/copy_propagation_arg.foo.DestinationPropagation.panic-unwind.diff

+9-9
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,20 @@
88
let mut _3: u8;
99

1010
bb0: {
11-
- StorageLive(_2);
12-
+ nop;
13-
StorageLive(_3);
14-
_3 = _1;
11+
StorageLive(_2);
12+
- StorageLive(_3);
13+
- _3 = _1;
1514
- _2 = dummy(move _3) -> [return: bb1, unwind continue];
16-
+ _1 = dummy(move _3) -> [return: bb1, unwind continue];
15+
+ nop;
16+
+ nop;
17+
+ _2 = dummy(move _1) -> [return: bb1, unwind continue];
1718
}
1819

1920
bb1: {
20-
StorageDead(_3);
21-
- _1 = move _2;
22-
- StorageDead(_2);
23-
+ nop;
21+
- StorageDead(_3);
2422
+ nop;
23+
_1 = move _2;
24+
StorageDead(_2);
2525
_0 = const ();
2626
return;
2727
}

0 commit comments

Comments
 (0)