Skip to content

Commit eafab66

Browse files
committed
UnreachableProp: Preserve unreachable branches for multiple targets
Before, UnreachablePropagation removed all unreachable branches. This was a pessimization, as it removed information about reachability that was used later in the optimization pipeline. For example, this code ```rust pub enum Two { A, B } pub fn identity(x: Two) -> Two { match x { Two::A => Two::A, Two::B => Two::B, } } ``` basically has `switchInt() -> [0: 0, 1: 1, otherwise: unreachable]` for the match. This allows it to be transformed into a simple `x`. If we remove the unreachable branch, the transformation becomes illegal.
1 parent 650bff8 commit eafab66

File tree

1 file changed

+48
-23
lines changed

1 file changed

+48
-23
lines changed

compiler/rustc_mir_transform/src/unreachable_prop.rs

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,19 @@ impl MirPass<'_> for UnreachablePropagation {
3838
}
3939
}
4040

41+
// We do want do keep some unreachable blocks, but make them empty.
42+
for bb in unreachable_blocks {
43+
if !tcx.consider_optimizing(|| {
44+
format!("UnreachablePropagation {:?} ", body.source.def_id())
45+
}) {
46+
break;
47+
}
48+
49+
body.basic_blocks_mut()[bb].statements.clear();
50+
}
51+
4152
let replaced = !replacements.is_empty();
53+
4254
for (bb, terminator_kind) in replacements {
4355
if !tcx.consider_optimizing(|| {
4456
format!("UnreachablePropagation {:?} ", body.source.def_id())
@@ -57,42 +69,55 @@ impl MirPass<'_> for UnreachablePropagation {
5769

5870
fn remove_successors<'tcx, F>(
5971
terminator_kind: &TerminatorKind<'tcx>,
60-
predicate: F,
72+
is_unreachable: F,
6173
) -> Option<TerminatorKind<'tcx>>
6274
where
6375
F: Fn(BasicBlock) -> bool,
6476
{
65-
let terminator = match *terminator_kind {
66-
TerminatorKind::Goto { target } if predicate(target) => TerminatorKind::Unreachable,
67-
TerminatorKind::SwitchInt { ref discr, switch_ty, ref targets } => {
77+
let terminator = match terminator_kind {
78+
// This will unconditionally run into an unreachable and is therefore unreachable as well.
79+
TerminatorKind::Goto { target } if is_unreachable(*target) => TerminatorKind::Unreachable,
80+
TerminatorKind::SwitchInt { targets, discr, switch_ty } => {
6881
let otherwise = targets.otherwise();
6982

70-
let original_targets_len = targets.iter().len() + 1;
71-
let (mut values, mut targets): (Vec<_>, Vec<_>) =
72-
targets.iter().filter(|(_, bb)| !predicate(*bb)).unzip();
83+
// If all targets are unreachable, we can be unreachable as well.
84+
if targets.all_targets().iter().all(|bb| is_unreachable(*bb)) {
85+
TerminatorKind::Unreachable
86+
} else if is_unreachable(otherwise) {
87+
// If there are multiple targets, don't delete unreachable branches (like an unreachable otherwise)
88+
// unless otherwise is unrachable, in which case deleting a normal branch causes it to be merged with
89+
// the otherwise, keeping its unreachable.
90+
// This looses information about reachability causing worse codegen.
91+
// For example (see src/test/codegen/match-optimizes-away.rs)
92+
//
93+
// pub enum Two { A, B }
94+
// pub fn identity(x: Two) -> Two {
95+
// match x {
96+
// Two::A => Two::A,
97+
// Two::B => Two::B,
98+
// }
99+
// }
100+
//
101+
// This generates a `switchInt() -> [0: 0, 1: 1, otherwise: unreachable]`, which allows us or LLVM to
102+
// turn it into just `x` later. Without the unreachable, such a transformation would be illegal.
103+
// If the otherwise branch is unreachable, we can delete all other unreacahble targets, as they will
104+
// still point to the unreachable and therefore not lose reachability information.
105+
let reachable_iter = targets.iter().filter(|(_, bb)| !is_unreachable(*bb));
73106

74-
if !predicate(otherwise) {
75-
targets.push(otherwise);
76-
} else {
77-
values.pop();
78-
}
107+
let new_targets = SwitchTargets::new(reachable_iter, otherwise);
79108

80-
let retained_targets_len = targets.len();
109+
// No unreachable branches were removed.
110+
if new_targets.all_targets().len() == targets.all_targets().len() {
111+
return None;
112+
}
81113

82-
if targets.is_empty() {
83-
TerminatorKind::Unreachable
84-
} else if targets.len() == 1 {
85-
TerminatorKind::Goto { target: targets[0] }
86-
} else if original_targets_len != retained_targets_len {
87114
TerminatorKind::SwitchInt {
88115
discr: discr.clone(),
89-
switch_ty,
90-
targets: SwitchTargets::new(
91-
values.iter().copied().zip(targets.iter().copied()),
92-
*targets.last().unwrap(),
93-
),
116+
switch_ty: *switch_ty,
117+
targets: new_targets,
94118
}
95119
} else {
120+
// If the otherwise branch is reachable, we don't want to delete any unreachable branches.
96121
return None;
97122
}
98123
}

0 commit comments

Comments
 (0)