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.
This commit is contained in:
parent
650bff80a6
commit
eafab66096
1 changed files with 50 additions and 25 deletions
|
@ -38,7 +38,19 @@ impl MirPass<'_> for UnreachablePropagation {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// We do want do keep some unreachable blocks, but make them empty.
|
||||||
|
for bb in unreachable_blocks {
|
||||||
|
if !tcx.consider_optimizing(|| {
|
||||||
|
format!("UnreachablePropagation {:?} ", body.source.def_id())
|
||||||
|
}) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
|
body.basic_blocks_mut()[bb].statements.clear();
|
||||||
|
}
|
||||||
|
|
||||||
let replaced = !replacements.is_empty();
|
let replaced = !replacements.is_empty();
|
||||||
|
|
||||||
for (bb, terminator_kind) in replacements {
|
for (bb, terminator_kind) in replacements {
|
||||||
if !tcx.consider_optimizing(|| {
|
if !tcx.consider_optimizing(|| {
|
||||||
format!("UnreachablePropagation {:?} ", body.source.def_id())
|
format!("UnreachablePropagation {:?} ", body.source.def_id())
|
||||||
|
@ -57,42 +69,55 @@ impl MirPass<'_> for UnreachablePropagation {
|
||||||
|
|
||||||
fn remove_successors<'tcx, F>(
|
fn remove_successors<'tcx, F>(
|
||||||
terminator_kind: &TerminatorKind<'tcx>,
|
terminator_kind: &TerminatorKind<'tcx>,
|
||||||
predicate: F,
|
is_unreachable: F,
|
||||||
) -> Option<TerminatorKind<'tcx>>
|
) -> Option<TerminatorKind<'tcx>>
|
||||||
where
|
where
|
||||||
F: Fn(BasicBlock) -> bool,
|
F: Fn(BasicBlock) -> bool,
|
||||||
{
|
{
|
||||||
let terminator = match *terminator_kind {
|
let terminator = match terminator_kind {
|
||||||
TerminatorKind::Goto { target } if predicate(target) => TerminatorKind::Unreachable,
|
// This will unconditionally run into an unreachable and is therefore unreachable as well.
|
||||||
TerminatorKind::SwitchInt { ref discr, switch_ty, ref targets } => {
|
TerminatorKind::Goto { target } if is_unreachable(*target) => TerminatorKind::Unreachable,
|
||||||
|
TerminatorKind::SwitchInt { targets, discr, switch_ty } => {
|
||||||
let otherwise = targets.otherwise();
|
let otherwise = targets.otherwise();
|
||||||
|
|
||||||
let original_targets_len = targets.iter().len() + 1;
|
// If all targets are unreachable, we can be unreachable as well.
|
||||||
let (mut values, mut targets): (Vec<_>, Vec<_>) =
|
if targets.all_targets().iter().all(|bb| is_unreachable(*bb)) {
|
||||||
targets.iter().filter(|(_, bb)| !predicate(*bb)).unzip();
|
|
||||||
|
|
||||||
if !predicate(otherwise) {
|
|
||||||
targets.push(otherwise);
|
|
||||||
} else {
|
|
||||||
values.pop();
|
|
||||||
}
|
|
||||||
|
|
||||||
let retained_targets_len = targets.len();
|
|
||||||
|
|
||||||
if targets.is_empty() {
|
|
||||||
TerminatorKind::Unreachable
|
TerminatorKind::Unreachable
|
||||||
} else if targets.len() == 1 {
|
} else if is_unreachable(otherwise) {
|
||||||
TerminatorKind::Goto { target: targets[0] }
|
// If there are multiple targets, don't delete unreachable branches (like an unreachable otherwise)
|
||||||
} else if original_targets_len != retained_targets_len {
|
// unless otherwise is unrachable, in which case deleting a normal branch causes it to be merged with
|
||||||
|
// the otherwise, keeping its unreachable.
|
||||||
|
// This looses information about reachability causing worse codegen.
|
||||||
|
// For example (see src/test/codegen/match-optimizes-away.rs)
|
||||||
|
//
|
||||||
|
// pub enum Two { A, B }
|
||||||
|
// pub fn identity(x: Two) -> Two {
|
||||||
|
// match x {
|
||||||
|
// Two::A => Two::A,
|
||||||
|
// Two::B => Two::B,
|
||||||
|
// }
|
||||||
|
// }
|
||||||
|
//
|
||||||
|
// This generates a `switchInt() -> [0: 0, 1: 1, otherwise: unreachable]`, which allows us or LLVM to
|
||||||
|
// turn it into just `x` later. Without the unreachable, such a transformation would be illegal.
|
||||||
|
// If the otherwise branch is unreachable, we can delete all other unreacahble targets, as they will
|
||||||
|
// still point to the unreachable and therefore not lose reachability information.
|
||||||
|
let reachable_iter = targets.iter().filter(|(_, bb)| !is_unreachable(*bb));
|
||||||
|
|
||||||
|
let new_targets = SwitchTargets::new(reachable_iter, otherwise);
|
||||||
|
|
||||||
|
// No unreachable branches were removed.
|
||||||
|
if new_targets.all_targets().len() == targets.all_targets().len() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
TerminatorKind::SwitchInt {
|
TerminatorKind::SwitchInt {
|
||||||
discr: discr.clone(),
|
discr: discr.clone(),
|
||||||
switch_ty,
|
switch_ty: *switch_ty,
|
||||||
targets: SwitchTargets::new(
|
targets: new_targets,
|
||||||
values.iter().copied().zip(targets.iter().copied()),
|
|
||||||
*targets.last().unwrap(),
|
|
||||||
),
|
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
// If the otherwise branch is reachable, we don't want to delete any unreachable branches.
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue