Skip to content

Commit 33b8687

Browse files
cjgillotcjgillot
committed
Unify a source with all possible destinations.
1 parent 808ce08 commit 33b8687

15 files changed

Lines changed: 84 additions & 102 deletions

‎compiler/rustc_mir_transform/src/dest_prop.rs‎

Lines changed: 39 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -182,29 +182,51 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
182182
for (src, candidates) in candidates.c.into_iter() {
183183
trace!(?src, ?candidates);
184184

185-
let Some(src) = relevant.find(src) else { continue };
186-
let Some(src_live_ranges) = &live.row(src) else { continue };
187-
trace!(?src, ?src_live_ranges);
188-
189-
let dst = candidates.into_iter().find_map(|dst| {
190-
let dst = relevant.find(dst)?;
191-
let dst_live_ranges = &live.row(dst)?;
185+
for dst in candidates {
186+
// We call `relevant.find(src)` inside the loop, as a previous iteration may have
187+
// renamed `src` to one of the locals in `dst`.
188+
let Some(mut src) = relevant.find(src) else { continue };
189+
let Some(src_live_ranges) = live.row(src) else { continue };
190+
trace!(?src, ?src_live_ranges);
191+
192+
let Some(mut dst) = relevant.find(dst) else { continue };
193+
let Some(dst_live_ranges) = live.row(dst) else { continue };
192194
trace!(?dst, ?dst_live_ranges);
193195

194-
let disjoint = src_live_ranges.disjoint(dst_live_ranges);
195-
disjoint.then_some(dst)
196-
});
197-
let Some(dst) = dst else { continue };
196+
if src_live_ranges.disjoint(dst_live_ranges) {
197+
// We want to replace `src` by `dst`.
198+
let mut orig_src = relevant.original[src];
199+
let mut orig_dst = relevant.original[dst];
200+
201+
// The return place and function arguments are required and cannot be renamed.
202+
// This check cannot be made during candidate collection, as we may want to
203+
// unify the same non-required local with several required locals.
204+
match (is_local_required(orig_src, body), is_local_required(orig_dst, body)) {
205+
// Renaming `src` is ok.
206+
(false, _) => {}
207+
// Renaming `src` is wrong, but renaming `dst` is ok.
208+
(true, false) => {
209+
std::mem::swap(&mut src, &mut dst);
210+
std::mem::swap(&mut orig_src, &mut orig_dst);
211+
}
212+
// Neither local can be renamed, so skip this case.
213+
(true, true) => continue,
214+
}
198215

199-
merged_locals.insert(relevant.original[src]);
200-
merged_locals.insert(relevant.original[dst]);
216+
trace!(?src, ?dst, "merge");
217+
merged_locals.insert(orig_src);
218+
merged_locals.insert(orig_dst);
201219

202-
relevant.union(src, dst);
203-
live.union_rows(src, dst);
220+
// Replace `src` by `dst`.
221+
relevant.union(src, dst);
222+
live.union_rows(/* read */ src, /* write */ dst);
223+
}
224+
}
204225
}
205226
trace!(?merged_locals);
206227

207228
relevant.make_idempotent();
229+
trace!(?relevant.renames);
208230

209231
if merged_locals.is_empty() {
210232
return;
@@ -394,41 +416,6 @@ impl Candidates {
394416
}
395417
}
396418

397-
/// If the pair of places is being considered for merging, returns the candidate which would be
398-
/// merged in order to accomplish this.
399-
///
400-
/// The contract here is in one direction - there is a guarantee that merging the locals that are
401-
/// outputted by this function would result in an assignment between the inputs becoming a
402-
/// self-assignment. However, there is no guarantee that the returned pair is actually suitable for
403-
/// merging - candidate collection must still check this independently.
404-
///
405-
/// This output is unique for each unordered pair of input places.
406-
fn places_to_candidate_pair<'tcx>(
407-
a: Place<'tcx>,
408-
b: Place<'tcx>,
409-
body: &Body<'tcx>,
410-
) -> Option<(Local, Local)> {
411-
let (mut a, mut b) = if a.projection.len() == 0 && b.projection.len() == 0 {
412-
(a.local, b.local)
413-
} else {
414-
return None;
415-
};
416-
417-
// By sorting, we make sure we're input order independent
418-
if a > b {
419-
std::mem::swap(&mut a, &mut b);
420-
}
421-
422-
// We could now return `(a, b)`, but then we miss some candidates in the case where `a` can't be
423-
// used as a `src`.
424-
if is_local_required(a, body) {
425-
std::mem::swap(&mut a, &mut b);
426-
}
427-
// We could check `is_local_required` again here, but there's no need - after all, we make no
428-
// promise that the candidate pair is actually valid
429-
Some((a, b))
430-
}
431-
432419
struct FindAssignments<'a, 'tcx> {
433420
body: &'a Body<'tcx>,
434421
candidates: FxIndexMap<Local, Vec<Local>>,
@@ -441,11 +428,9 @@ impl<'tcx> Visitor<'tcx> for FindAssignments<'_, 'tcx> {
441428
lhs,
442429
Rvalue::CopyForDeref(rhs) | Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)),
443430
)) = &statement.kind
431+
&& let Some(src) = lhs.as_local()
432+
&& let Some(dest) = rhs.as_local()
444433
{
445-
let Some((src, dest)) = places_to_candidate_pair(*lhs, *rhs, self.body) else {
446-
return;
447-
};
448-
449434
// As described at the top of the file, we do not go near things that have
450435
// their address taken.
451436
if self.borrowed.contains(src) || self.borrowed.contains(dest) {
@@ -462,11 +447,6 @@ impl<'tcx> Visitor<'tcx> for FindAssignments<'_, 'tcx> {
462447
return;
463448
}
464449

465-
// Also, we need to make sure that MIR actually allows the `src` to be removed
466-
if is_local_required(src, self.body) {
467-
return;
468-
}
469-
470450
// We may insert duplicates here, but that's fine
471451
self.candidates.entry(src).or_default().push(dest);
472452
}

‎tests/mir-opt/dest-prop/cycle.main.DestinationPropagation.panic-abort.diff‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,23 @@
88
let _5: ();
99
let mut _6: i32;
1010
scope 1 {
11-
- debug x => _1;
12-
+ debug x => _4;
11+
debug x => _1;
1312
let _2: i32;
1413
scope 2 {
1514
- debug y => _2;
16-
+ debug y => _4;
15+
+ debug y => _1;
1716
let _3: i32;
1817
scope 3 {
1918
- debug z => _3;
20-
+ debug z => _4;
19+
+ debug z => _1;
2120
}
2221
}
2322
}
2423

2524
bb0: {
2625
- StorageLive(_1);
27-
- _1 = val() -> [return: bb1, unwind unreachable];
2826
+ nop;
29-
+ _4 = val() -> [return: bb1, unwind unreachable];
27+
_1 = val() -> [return: bb1, unwind unreachable];
3028
}
3129

3230
bb1: {
@@ -47,14 +45,17 @@
4745
+ nop;
4846
+ nop;
4947
StorageLive(_5);
50-
StorageLive(_6);
48+
- StorageLive(_6);
5149
- _6 = copy _1;
52-
+ _6 = copy _4;
53-
_5 = std::mem::drop::<i32>(move _6) -> [return: bb2, unwind unreachable];
50+
- _5 = std::mem::drop::<i32>(move _6) -> [return: bb2, unwind unreachable];
51+
+ nop;
52+
+ nop;
53+
+ _5 = std::mem::drop::<i32>(move _1) -> [return: bb2, unwind unreachable];
5454
}
5555

5656
bb2: {
57-
StorageDead(_6);
57+
- StorageDead(_6);
58+
+ nop;
5859
StorageDead(_5);
5960
_0 = const ();
6061
- StorageDead(_3);

‎tests/mir-opt/dest-prop/cycle.main.DestinationPropagation.panic-unwind.diff‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,23 @@
88
let _5: ();
99
let mut _6: i32;
1010
scope 1 {
11-
- debug x => _1;
12-
+ debug x => _4;
11+
debug x => _1;
1312
let _2: i32;
1413
scope 2 {
1514
- debug y => _2;
16-
+ debug y => _4;
15+
+ debug y => _1;
1716
let _3: i32;
1817
scope 3 {
1918
- debug z => _3;
20-
+ debug z => _4;
19+
+ debug z => _1;
2120
}
2221
}
2322
}
2423

2524
bb0: {
2625
- StorageLive(_1);
27-
- _1 = val() -> [return: bb1, unwind continue];
2826
+ nop;
29-
+ _4 = val() -> [return: bb1, unwind continue];
27+
_1 = val() -> [return: bb1, unwind continue];
3028
}
3129

3230
bb1: {
@@ -47,14 +45,17 @@
4745
+ nop;
4846
+ nop;
4947
StorageLive(_5);
50-
StorageLive(_6);
48+
- StorageLive(_6);
5149
- _6 = copy _1;
52-
+ _6 = copy _4;
53-
_5 = std::mem::drop::<i32>(move _6) -> [return: bb2, unwind continue];
50+
- _5 = std::mem::drop::<i32>(move _6) -> [return: bb2, unwind continue];
51+
+ nop;
52+
+ nop;
53+
+ _5 = std::mem::drop::<i32>(move _1) -> [return: bb2, unwind continue];
5454
}
5555

5656
bb2: {
57-
StorageDead(_6);
57+
- StorageDead(_6);
58+
+ nop;
5859
StorageDead(_5);
5960
_0 = const ();
6061
- StorageDead(_3);

‎tests/mir-opt/dest-prop/dead_stores_79191.f.DestinationPropagation.after.panic-abort.mir‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@ fn f(_1: usize) -> usize {
77
let mut _3: usize;
88
let mut _4: usize;
99
scope 1 {
10-
debug b => _3;
10+
debug b => _2;
1111
}
1212

1313
bb0: {
1414
nop;
15-
_3 = copy _1;
15+
_2 = copy _1;
1616
_1 = const 5_usize;
1717
nop;
1818
nop;
19-
_1 = move _3;
19+
_1 = move _2;
2020
nop;
2121
nop;
2222
nop;

‎tests/mir-opt/dest-prop/dead_stores_79191.f.DestinationPropagation.after.panic-unwind.mir‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@ fn f(_1: usize) -> usize {
77
let mut _3: usize;
88
let mut _4: usize;
99
scope 1 {
10-
debug b => _3;
10+
debug b => _2;
1111
}
1212

1313
bb0: {
1414
nop;
15-
_3 = copy _1;
15+
_2 = copy _1;
1616
_1 = const 5_usize;
1717
nop;
1818
nop;
19-
_1 = move _3;
19+
_1 = move _2;
2020
nop;
2121
nop;
2222
nop;

‎tests/mir-opt/dest-prop/dead_stores_better.f.DestinationPropagation.after.panic-abort.mir‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@ fn f(_1: usize) -> usize {
77
let mut _3: usize;
88
let mut _4: usize;
99
scope 1 {
10-
debug b => _3;
10+
debug b => _2;
1111
}
1212

1313
bb0: {
1414
nop;
15-
_3 = copy _1;
15+
_2 = copy _1;
1616
_1 = const 5_usize;
1717
nop;
1818
nop;
19-
_1 = move _3;
19+
_1 = move _2;
2020
nop;
2121
nop;
2222
nop;

‎tests/mir-opt/dest-prop/dead_stores_better.f.DestinationPropagation.after.panic-unwind.mir‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@ fn f(_1: usize) -> usize {
77
let mut _3: usize;
88
let mut _4: usize;
99
scope 1 {
10-
debug b => _3;
10+
debug b => _2;
1111
}
1212

1313
bb0: {
1414
nop;
15-
_3 = copy _1;
15+
_2 = copy _1;
1616
_1 = const 5_usize;
1717
nop;
1818
nop;
19-
_1 = move _3;
19+
_1 = move _2;
2020
nop;
2121
nop;
2222
nop;

‎tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
44
debug slice => _1;
55
debug f => _2;
66
let mut _0: ();
7-
let mut _6: std::ptr::NonNull<T>;
8-
let mut _9: *const T;
97
let mut _10: usize;
108
let mut _28: std::option::Option<(usize, &T)>;
119
let mut _31: &impl Fn(usize, &T);
@@ -47,6 +45,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
4745
}
4846
}
4947
scope 28 (inlined <std::slice::Iter<'_, T> as Iterator>::next) {
48+
let mut _6: std::ptr::NonNull<T>;
5049
let _11: std::ptr::NonNull<T>;
5150
let _13: std::ptr::NonNull<T>;
5251
let mut _16: bool;
@@ -103,6 +102,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
103102
let mut _8: *mut T;
104103
scope 5 {
105104
scope 6 {
105+
let _9: *const T;
106106
scope 7 {
107107
}
108108
scope 11 (inlined std::ptr::without_provenance::<T>) {

‎tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-unwind.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
2727
let _3: usize;
2828
let mut _7: *mut T;
2929
let mut _8: *mut T;
30-
let mut _9: *const T;
3130
scope 5 {
3231
let _6: std::ptr::NonNull<T>;
3332
scope 6 {
33+
let _9: *const T;
3434
scope 7 {
3535
}
3636
scope 11 (inlined std::ptr::without_provenance::<T>) {

‎tests/mir-opt/pre-codegen/slice_iter.forward_loop.PreCodegen.after.panic-abort.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
44
debug slice => _1;
55
debug f => _2;
66
let mut _0: ();
7-
let mut _6: std::ptr::NonNull<T>;
8-
let mut _9: *const T;
97
let mut _22: std::option::Option<&T>;
108
let mut _24: &impl Fn(&T);
119
let mut _25: (&T,);
@@ -19,6 +17,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
1917
debug x => _23;
2018
}
2119
scope 16 (inlined <std::slice::Iter<'_, T> as Iterator>::next) {
20+
let mut _6: std::ptr::NonNull<T>;
2221
let _10: std::ptr::NonNull<T>;
2322
let _12: std::ptr::NonNull<T>;
2423
let mut _15: bool;
@@ -74,6 +73,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
7473
let mut _8: *mut T;
7574
scope 5 {
7675
scope 6 {
76+
let _9: *const T;
7777
scope 7 {
7878
}
7979
scope 11 (inlined std::ptr::without_provenance::<T>) {

0 commit comments

Comments
 (0)