Skip to content

Commit 768b800

Browse files
committed
mir-opt: Do not eliminate dead statements that are being used by other BBs during merging
Since we will not eliminate dead statements in other BBs, they may still use the dead statements within the merged BB.
1 parent 4211838 commit 768b800

File tree

5 files changed

+94
-16
lines changed

5 files changed

+94
-16
lines changed

compiler/rustc_mir_dataflow/src/impls/liveness.rs

+36-14
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceC
33
use rustc_middle::mir::{
44
self, CallReturnPlaces, Local, Location, Place, StatementKind, TerminatorEdges,
55
};
6+
use rustc_middle::ty::TyCtxt;
67

7-
use crate::{Analysis, Backward, GenKill};
8+
use crate::{Analysis, Backward, GenKill, ResultsCursor};
89

910
/// A [live-variable dataflow analysis][liveness].
1011
///
@@ -203,21 +204,42 @@ impl DefUse {
203204
/// This is basically written for dead store elimination and nothing else.
204205
///
205206
/// All of the caveats of `MaybeLiveLocals` apply.
206-
pub struct MaybeTransitiveLiveLocals<'a> {
207+
pub struct MaybeTransitiveLiveLocals<'tcx, 'mir, 'a> {
207208
always_live: &'a DenseBitSet<Local>,
209+
live: Option<ResultsCursor<'mir, 'tcx, MaybeLiveLocals>>,
208210
}
209211

210-
impl<'a> MaybeTransitiveLiveLocals<'a> {
212+
impl<'tcx, 'mir, 'a> MaybeTransitiveLiveLocals<'tcx, 'mir, 'a> {
211213
/// The `always_alive` set is the set of locals to which all stores should unconditionally be
212214
/// considered live.
215+
/// The `may_live_in_other_bbs` indicates that, when analyzing the current statement,
216+
/// statements in other basic blocks are assumed to be live.
213217
///
214218
/// This should include at least all locals that are ever borrowed.
215-
pub fn new(always_live: &'a DenseBitSet<Local>) -> Self {
216-
MaybeTransitiveLiveLocals { always_live }
219+
pub fn new(
220+
always_live: &'a DenseBitSet<Local>,
221+
may_live_in_other_bbs: bool,
222+
tcx: TyCtxt<'tcx>,
223+
body: &'mir mir::Body<'tcx>,
224+
) -> Self {
225+
let live = if may_live_in_other_bbs {
226+
Some(MaybeLiveLocals.iterate_to_fixpoint(tcx, body, None).into_results_cursor(body))
227+
} else {
228+
None
229+
};
230+
MaybeTransitiveLiveLocals { always_live, live }
231+
}
232+
233+
fn live_on(&mut self, place: Place<'_>, location: Location) -> bool {
234+
let Some(live) = &mut self.live else {
235+
return false;
236+
};
237+
live.seek_before_primary_effect(location);
238+
live.get().contains(place.local)
217239
}
218240
}
219241

220-
impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> {
242+
impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'tcx, '_, 'a> {
221243
type Domain = DenseBitSet<Local>;
222244
type Direction = Backward;
223245

@@ -256,14 +278,14 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> {
256278
| StatementKind::BackwardIncompatibleDropHint { .. }
257279
| StatementKind::Nop => None,
258280
};
259-
if let Some(destination) = destination {
260-
if !destination.is_indirect()
261-
&& !state.contains(destination.local)
262-
&& !self.always_live.contains(destination.local)
263-
{
264-
// This store is dead
265-
return;
266-
}
281+
if let Some(destination) = destination
282+
&& !destination.is_indirect()
283+
&& !state.contains(destination.local)
284+
&& !self.always_live.contains(destination.local)
285+
&& !self.live_on(destination, location)
286+
{
287+
// This store is dead
288+
return;
267289
}
268290
TransferFunction(state).visit_statement(statement, location);
269291
}

compiler/rustc_mir_transform/src/dead_store_elimination.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ pub(super) fn eliminate<'tcx>(
5555
mut modify_basic_blocks: ModifyBasicBlocks<'tcx, '_>,
5656
ignore_debuginfo: bool,
5757
arg_copy_to_move: bool,
58+
may_live_in_other_bbs: bool,
5859
) {
5960
let body = modify_basic_blocks.body();
6061
let borrowed_locals = borrowed_locals(body);
@@ -69,7 +70,7 @@ pub(super) fn eliminate<'tcx>(
6970
always_live
7071
};
7172

72-
let mut live = MaybeTransitiveLiveLocals::new(&always_live)
73+
let mut live = MaybeTransitiveLiveLocals::new(&always_live, may_live_in_other_bbs, tcx, body)
7374
.iterate_to_fixpoint(tcx, body, None)
7475
.into_results_cursor(body);
7576

@@ -179,7 +180,7 @@ impl<'tcx> crate::MirPass<'tcx> for DeadStoreElimination {
179180
}
180181

181182
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
182-
eliminate(tcx, ModifyBasicBlocks::Direct(body), false, true);
183+
eliminate(tcx, ModifyBasicBlocks::Direct(body), false, true, false);
183184
}
184185

185186
fn is_required(&self) -> bool {

compiler/rustc_mir_transform/src/match_branches.rs

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ impl<'tcx> crate::MirPass<'tcx> for MatchBranchSimplification {
3131
ModifyBasicBlocks::BasicBlocks(body, bbs),
3232
true,
3333
false,
34+
true,
3435
);
3536
eliminate_unused_storage_mark(body, bbs);
3637
strip_nops(bbs.as_mut_slice());
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
- // MIR for `match_dead_store_failed` before MatchBranchSimplification
2+
+ // MIR for `match_dead_store_failed` after MatchBranchSimplification
3+
4+
fn match_dead_store_failed(_1: bool) -> () {
5+
debug b => _1;
6+
let mut _0: ();
7+
let _2: (&str,);
8+
let mut _3: &str;
9+
let mut _4: bool;
10+
let _5: &str;
11+
scope 1 {
12+
debug _b => _2;
13+
}
14+
15+
bb0: {
16+
StorageLive(_2);
17+
StorageLive(_3);
18+
StorageLive(_4);
19+
_4 = copy _1;
20+
switchInt(move _4) -> [0: bb2, otherwise: bb1];
21+
}
22+
23+
bb1: {
24+
_3 = const " ";
25+
goto -> bb3;
26+
}
27+
28+
bb2: {
29+
StorageLive(_5);
30+
_5 = const "";
31+
_3 = &(*_5);
32+
StorageDead(_5);
33+
goto -> bb3;
34+
}
35+
36+
bb3: {
37+
StorageDead(_4);
38+
_2 = (move _3,);
39+
StorageDead(_3);
40+
_0 = const ();
41+
StorageDead(_2);
42+
return;
43+
}
44+
}
45+

tests/mir-opt/matches_reduce_branches.rs

+9
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,15 @@ fn match_i128_u128(i: EnumAi128) -> u128 {
672672
}
673673
}
674674

675+
// We cannot merge these branches unless all the dead statements are eliminated.
676+
// EMIT_MIR matches_reduce_branches.match_dead_store_failed.MatchBranchSimplification.diff
677+
pub fn match_dead_store_failed(b: bool) {
678+
// CHECK-LABEL: fn match_dead_store_failed(
679+
// CHECK: switchInt
680+
// CHECK: return
681+
let _b = (if b { " " } else { "" },);
682+
}
683+
675684
fn main() {
676685
let _ = foo(None);
677686
let _ = foo(Some(()));

0 commit comments

Comments
 (0)