Skip to content

Commit 4c76c16

Browse files
authored
Unrolled build for rust-lang#119591
Rollup merge of rust-lang#119591 - Enselic:DestinationPropagation-stable, r=cjgillot 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 files 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. In other words, compare this diff (before fix): * https://github.com/rust-lang/rust/blob/090d5eac722000906cc00d991f2bf052b0e388c3/tests/mir-opt/dest-prop/copy_propagation_arg.foo.DestinationPropagation.panic-unwind.diff With this diff (after fix): * https://github.com/rust-lang/rust/blob/f603babd63a607e155609dc0277806e559626ea0/tests/mir-opt/dest-prop/copy_propagation_arg.foo.DestinationPropagation.panic-unwind.diff and you can see that both before and after the fix, we replace 3 statements with `nop`s. I find it _slightly_ surprising that the test this PR affects did not previously fail spuriously due to the indeterminism of `FxHashMap`, but I guess in can be explained with the predictability of small `FxHashMap`s with `usize` (`Local`) keys, or something along those lines. This should fix [this](rust-lang#119252 (comment)) comment, but I wanted to make a separate PR for this fix for a simpler development and review process. Part of rust-lang#84447 which is E-help-wanted. r? `@cjgillot` who is reviewer for the highly related PR rust-lang#119252.
2 parents 9212108 + 95eb5bc commit 4c76c16

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,10 +131,8 @@
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::MirPass;
137-
use rustc_data_structures::fx::FxHashMap;
135+
use rustc_data_structures::fx::{FxIndexMap, IndexEntry, IndexOccupiedEntry};
138136
use rustc_index::bit_set::BitSet;
139137
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
140138
use rustc_middle::mir::HasLocalDecls;
@@ -211,7 +209,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
211209
let mut merged_locals: BitSet<Local> = BitSet::new_empty(body.local_decls.len());
212210

213211
// This is the set of merges we will apply this round. It is a subset of the candidates.
214-
let mut merges = FxHashMap::default();
212+
let mut merges = FxIndexMap::default();
215213

216214
for (src, candidates) in candidates.c.iter() {
217215
if merged_locals.contains(*src) {
@@ -250,8 +248,8 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
250248
/// frequently. Everything with a `&'alloc` lifetime points into here.
251249
#[derive(Default)]
252250
struct Allocations {
253-
candidates: FxHashMap<Local, Vec<Local>>,
254-
candidates_reverse: FxHashMap<Local, Vec<Local>>,
251+
candidates: FxIndexMap<Local, Vec<Local>>,
252+
candidates_reverse: FxIndexMap<Local, Vec<Local>>,
255253
write_info: WriteInfo,
256254
// PERF: Do this for `MaybeLiveLocals` allocations too.
257255
}
@@ -272,11 +270,11 @@ struct Candidates<'alloc> {
272270
///
273271
/// We will still report that we would like to merge `_1` and `_2` in an attempt to allow us to
274272
/// remove that assignment.
275-
c: &'alloc mut FxHashMap<Local, Vec<Local>>,
273+
c: &'alloc mut FxIndexMap<Local, Vec<Local>>,
276274
/// A reverse index of the `c` set; if the `c` set contains `a => Place { local: b, proj }`,
277275
/// then this contains `b => a`.
278276
// PERF: Possibly these should be `SmallVec`s?
279-
reverse: &'alloc mut FxHashMap<Local, Vec<Local>>,
277+
reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
280278
}
281279

282280
//////////////////////////////////////////////////////////
@@ -287,7 +285,7 @@ struct Candidates<'alloc> {
287285
fn apply_merges<'tcx>(
288286
body: &mut Body<'tcx>,
289287
tcx: TyCtxt<'tcx>,
290-
merges: &FxHashMap<Local, Local>,
288+
merges: &FxIndexMap<Local, Local>,
291289
merged_locals: &BitSet<Local>,
292290
) {
293291
let mut merger = Merger { tcx, merges, merged_locals };
@@ -296,7 +294,7 @@ fn apply_merges<'tcx>(
296294

297295
struct Merger<'a, 'tcx> {
298296
tcx: TyCtxt<'tcx>,
299-
merges: &'a FxHashMap<Local, Local>,
297+
merges: &'a FxIndexMap<Local, Local>,
300298
merged_locals: &'a BitSet<Local>,
301299
}
302300

@@ -379,7 +377,7 @@ impl<'alloc> Candidates<'alloc> {
379377

380378
/// `vec_filter_candidates` but for an `Entry`
381379
fn entry_filter_candidates(
382-
mut entry: OccupiedEntry<'_, Local, Vec<Local>>,
380+
mut entry: IndexOccupiedEntry<'_, Local, Vec<Local>>,
383381
p: Local,
384382
f: impl FnMut(Local) -> CandidateFilter,
385383
at: Location,
@@ -399,7 +397,7 @@ impl<'alloc> Candidates<'alloc> {
399397
at: Location,
400398
) {
401399
// Cover the cases where `p` appears as a `src`
402-
if let Entry::Occupied(entry) = self.c.entry(p) {
400+
if let IndexEntry::Occupied(entry) = self.c.entry(p) {
403401
Self::entry_filter_candidates(entry, p, &mut f, at);
404402
}
405403
// And the cases where `p` appears as a `dest`
@@ -412,7 +410,7 @@ impl<'alloc> Candidates<'alloc> {
412410
if f(*src) == CandidateFilter::Keep {
413411
return true;
414412
}
415-
let Entry::Occupied(entry) = self.c.entry(*src) else {
413+
let IndexEntry::Occupied(entry) = self.c.entry(*src) else {
416414
return false;
417415
};
418416
Self::entry_filter_candidates(
@@ -721,8 +719,8 @@ fn places_to_candidate_pair<'tcx>(
721719
fn find_candidates<'alloc, 'tcx>(
722720
body: &Body<'tcx>,
723721
borrowed: &BitSet<Local>,
724-
candidates: &'alloc mut FxHashMap<Local, Vec<Local>>,
725-
candidates_reverse: &'alloc mut FxHashMap<Local, Vec<Local>>,
722+
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
723+
candidates_reverse: &'alloc mut FxIndexMap<Local, Vec<Local>>,
726724
) -> Candidates<'alloc> {
727725
candidates.clear();
728726
candidates_reverse.clear();
@@ -744,7 +742,7 @@ fn find_candidates<'alloc, 'tcx>(
744742

745743
struct FindAssignments<'a, 'alloc, 'tcx> {
746744
body: &'a Body<'tcx>,
747-
candidates: &'alloc mut FxHashMap<Local, Vec<Local>>,
745+
candidates: &'alloc mut FxIndexMap<Local, Vec<Local>>,
748746
borrowed: &'a BitSet<Local>,
749747
}
750748

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)