Skip to content

Commit e92550a

Browse files
rust-bors[bot]rust-bors[bot]
authored
Auto merge of #142819 - ohadravid:better-storage-calls-gvn-v2, r=<try>
Remove fewer Storage calls in GVN Followup to #142531 (Remove fewer Storage calls in `copy_prop`) Modify the GVN MIR optimization pass to remove fewer Storage{Live,Dead} calls, allowing for better optimizations by LLVM - see #141649. After replacing locals with values, use the `MaybeStorageDead` analysis to check that the replaced locals are storage-live. **A slight problem**: In #142531, `@tmiasko` noted #142531 (comment) that `MaybeStorageDead` isn't enough since there can be a `Live(_1); Dead(_1); Live(_1);` block which forces the optimization to check that each value is initialised (and not only storage-live). This is easy enough in `copy_prop` (because we are checking _before_ the replacement), but in GVN it is actually hard to tell for each statement if the local must be initialized or not after the fact (and modifying `VnState` seems even harder). I opted for something else which might be wrong (implemented in the last two commits): If we consider `Dead->Live` to be the same as `Deinit`, than such a local shouldn't be considered SSA - so I updated `SsaVisitor` to mark such cases as non-SSA. r? tmiasko
2 parents df4ad9e + 1ea73cf commit e92550a

File tree

140 files changed

+867
-812
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

140 files changed

+867
-812
lines changed

‎compiler/rustc_mir_transform/src/gvn.rs‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ use rustc_middle::mir::visit::*;
104104
use rustc_middle::mir::*;
105105
use rustc_middle::ty::layout::{HasTypingEnv, LayoutOf};
106106
use rustc_middle::ty::{self, Ty, TyCtxt};
107+
use rustc_mir_dataflow::impls::{MaybeStorageDead, always_storage_live_locals};
108+
use rustc_mir_dataflow::{Analysis, ResultsCursor};
107109
use rustc_span::DUMMY_SP;
108110
use rustc_span::def_id::DefId;
109111
use smallvec::SmallVec;
@@ -140,10 +142,33 @@ impl<'tcx> crate::MirPass<'tcx> for GVN {
140142
state.visit_basic_block_data(bb, data);
141143
}
142144

143-
// For each local that is reused (`y` above), we remove its storage statements do avoid any
144-
// difficulty. Those locals are SSA, so should be easy to optimize by LLVM without storage
145-
// statements.
146-
StorageRemover { tcx, reused_locals: state.reused_locals }.visit_body_preserves_cfg(body);
145+
// If we emit storage annotations, use `MaybeStorageDead` to check which reused locals
146+
// require storage removal (making them alive for the duration of the function).
147+
let storage_to_remove = if tcx.sess.emit_lifetime_markers() {
148+
let always_live_locals = always_storage_live_locals(body);
149+
150+
let maybe_storage_dead = MaybeStorageDead::new(Cow::Owned(always_live_locals))
151+
.iterate_to_fixpoint(tcx, body, None)
152+
.into_results_cursor(body);
153+
154+
let mut storage_checker = StorageChecker {
155+
storage_to_check: state.reused_locals.clone(),
156+
storage_to_remove: DenseBitSet::new_empty(body.local_decls.len()),
157+
maybe_storage_dead,
158+
};
159+
160+
storage_checker.visit_body(body);
161+
162+
storage_checker.storage_to_remove
163+
} else {
164+
// Conservatively remove all storage statements for reused locals.
165+
state.reused_locals.clone()
166+
};
167+
168+
debug!(?storage_to_remove);
169+
170+
StorageRemover { tcx, reused_locals: state.reused_locals, storage_to_remove }
171+
.visit_body_preserves_cfg(body);
147172
}
148173

149174
fn is_required(&self) -> bool {
@@ -1824,6 +1849,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
18241849
struct StorageRemover<'tcx> {
18251850
tcx: TyCtxt<'tcx>,
18261851
reused_locals: DenseBitSet<Local>,
1852+
storage_to_remove: DenseBitSet<Local>,
18271853
}
18281854

18291855
impl<'tcx> MutVisitor<'tcx> for StorageRemover<'tcx> {
@@ -1844,11 +1870,35 @@ impl<'tcx> MutVisitor<'tcx> for StorageRemover<'tcx> {
18441870
match stmt.kind {
18451871
// When removing storage statements, we need to remove both (#107511).
18461872
StatementKind::StorageLive(l) | StatementKind::StorageDead(l)
1847-
if self.reused_locals.contains(l) =>
1873+
if self.storage_to_remove.contains(l) =>
18481874
{
18491875
stmt.make_nop()
18501876
}
18511877
_ => self.super_statement(stmt, loc),
18521878
}
18531879
}
18541880
}
1881+
1882+
struct StorageChecker<'a, 'tcx> {
1883+
storage_to_check: DenseBitSet<Local>,
1884+
storage_to_remove: DenseBitSet<Local>,
1885+
maybe_storage_dead: ResultsCursor<'a, 'tcx, MaybeStorageDead<'a>>,
1886+
}
1887+
1888+
impl<'a, 'tcx> Visitor<'tcx> for StorageChecker<'a, 'tcx> {
1889+
fn visit_local(&mut self, local: Local, context: PlaceContext, location: Location) {
1890+
if !context.is_use() {
1891+
return;
1892+
}
1893+
1894+
if self.storage_to_check.contains(local) {
1895+
self.maybe_storage_dead.seek_after_primary_effect(location);
1896+
1897+
if self.maybe_storage_dead.get().contains(local) {
1898+
debug!(?location, ?local, "local is maybe dead in this location, removing storage");
1899+
self.storage_to_remove.insert(local);
1900+
self.storage_to_check.remove(local);
1901+
}
1902+
}
1903+
}
1904+
}

‎compiler/rustc_mir_transform/src/ssa.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,13 @@ impl<'tcx> Visitor<'tcx> for SsaVisitor<'_, 'tcx> {
245245
self.check_dominates(local, loc);
246246
self.direct_uses[local] += 1;
247247
}
248+
PlaceContext::NonUse(NonUseContext::StorageLive) => {
249+
// If the local is already assigned, than marking it _again_ as storage live
250+
// deinitializes it, so need to treat if as a mutating use.
251+
if let Set1::One(_) = self.assignments[local] {
252+
self.assignments[local] = Set1::Many;
253+
}
254+
}
248255
PlaceContext::NonUse(_) => {}
249256
}
250257
}

‎tests/mir-opt/const_debuginfo.main.SingleUseConsts.diff‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,14 @@
5555
}
5656

5757
bb0: {
58-
nop;
58+
StorageLive(_1);
5959
- _1 = const 1_u8;
60-
nop;
61-
- _2 = const 2_u8;
62-
nop;
63-
- _3 = const 3_u8;
6460
+ nop;
61+
StorageLive(_2);
62+
- _2 = const 2_u8;
6563
+ nop;
64+
StorageLive(_3);
65+
- _3 = const 3_u8;
6666
+ nop;
6767
StorageLive(_4);
6868
StorageLive(_5);
@@ -95,7 +95,7 @@
9595
- _12 = const Point {{ x: 32_u32, y: 32_u32 }};
9696
+ nop;
9797
StorageLive(_13);
98-
nop;
98+
StorageLive(_14);
9999
- _14 = const 32_u32;
100100
+ nop;
101101
StorageLive(_15);
@@ -104,17 +104,17 @@
104104
+ nop;
105105
+ nop;
106106
StorageDead(_15);
107-
nop;
107+
StorageDead(_14);
108108
_0 = const ();
109109
StorageDead(_13);
110110
StorageDead(_12);
111111
StorageDead(_11);
112112
StorageDead(_10);
113113
StorageDead(_9);
114114
StorageDead(_4);
115-
nop;
116-
nop;
117-
nop;
115+
StorageDead(_3);
116+
StorageDead(_2);
117+
StorageDead(_1);
118118
return;
119119
}
120120
}

‎tests/mir-opt/const_prop/aggregate.main.GVN.panic-abort.diff‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
}
1414

1515
bb0: {
16-
- StorageLive(_1);
17-
+ nop;
16+
StorageLive(_1);
1817
StorageLive(_2);
1918
StorageLive(_3);
2019
_3 = (const 0_i32, const 1_u8, const 2_i32);
@@ -36,8 +35,7 @@
3635
StorageDead(_5);
3736
StorageDead(_4);
3837
_0 = const ();
39-
- StorageDead(_1);
40-
+ nop;
38+
StorageDead(_1);
4139
return;
4240
}
4341
}

‎tests/mir-opt/const_prop/aggregate.main.GVN.panic-unwind.diff‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
}
1414

1515
bb0: {
16-
- StorageLive(_1);
17-
+ nop;
16+
StorageLive(_1);
1817
StorageLive(_2);
1918
StorageLive(_3);
2019
_3 = (const 0_i32, const 1_u8, const 2_i32);
@@ -36,8 +35,7 @@
3635
StorageDead(_5);
3736
StorageDead(_4);
3837
_0 = const ();
39-
- StorageDead(_1);
40-
+ nop;
38+
StorageDead(_1);
4139
return;
4240
}
4341
}

‎tests/mir-opt/const_prop/bad_op_div_by_zero.main.GVN.panic-abort.diff‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
}
1919

2020
bb0: {
21-
- StorageLive(_1);
22-
+ nop;
21+
StorageLive(_1);
2322
_1 = const 0_i32;
2423
StorageLive(_2);
2524
StorageLive(_3);
@@ -48,8 +47,7 @@
4847
StorageDead(_3);
4948
_0 = const ();
5049
StorageDead(_2);
51-
- StorageDead(_1);
52-
+ nop;
50+
StorageDead(_1);
5351
return;
5452
}
5553
}

‎tests/mir-opt/const_prop/bad_op_div_by_zero.main.GVN.panic-unwind.diff‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
}
1919

2020
bb0: {
21-
- StorageLive(_1);
22-
+ nop;
21+
StorageLive(_1);
2322
_1 = const 0_i32;
2423
StorageLive(_2);
2524
StorageLive(_3);
@@ -48,8 +47,7 @@
4847
StorageDead(_3);
4948
_0 = const ();
5049
StorageDead(_2);
51-
- StorageDead(_1);
52-
+ nop;
50+
StorageDead(_1);
5351
return;
5452
}
5553
}

‎tests/mir-opt/const_prop/bad_op_mod_by_zero.main.GVN.panic-abort.diff‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
}
1919

2020
bb0: {
21-
- StorageLive(_1);
22-
+ nop;
21+
StorageLive(_1);
2322
_1 = const 0_i32;
2423
StorageLive(_2);
2524
StorageLive(_3);
@@ -48,8 +47,7 @@
4847
StorageDead(_3);
4948
_0 = const ();
5049
StorageDead(_2);
51-
- StorageDead(_1);
52-
+ nop;
50+
StorageDead(_1);
5351
return;
5452
}
5553
}

‎tests/mir-opt/const_prop/bad_op_mod_by_zero.main.GVN.panic-unwind.diff‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
}
1919

2020
bb0: {
21-
- StorageLive(_1);
22-
+ nop;
21+
StorageLive(_1);
2322
_1 = const 0_i32;
2423
StorageLive(_2);
2524
StorageLive(_3);
@@ -48,8 +47,7 @@
4847
StorageDead(_3);
4948
_0 = const ();
5049
StorageDead(_2);
51-
- StorageDead(_1);
52-
+ nop;
50+
StorageDead(_1);
5351
return;
5452
}
5553
}

‎tests/mir-opt/const_prop/boolean_identities.test.GVN.diff‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,13 @@
1919
}
2020

2121
bb0: {
22-
- StorageLive(_3);
23-
+ nop;
22+
StorageLive(_3);
2423
StorageLive(_4);
2524
_4 = copy _2;
2625
- _3 = BitOr(move _4, const true);
2726
+ _3 = const true;
2827
StorageDead(_4);
29-
- StorageLive(_5);
30-
+ nop;
28+
StorageLive(_5);
3129
StorageLive(_6);
3230
_6 = copy _1;
3331
- _5 = BitAnd(move _6, const false);
@@ -43,10 +41,8 @@
4341
+ _0 = const false;
4442
StorageDead(_8);
4543
StorageDead(_7);
46-
- StorageDead(_5);
47-
- StorageDead(_3);
48-
+ nop;
49-
+ nop;
44+
StorageDead(_5);
45+
StorageDead(_3);
5046
return;
5147
}
5248
}

0 commit comments

Comments
 (0)