bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Auto merge of #110569 - saethlin:mir-pass-cooperation, r=cjgillot

Browse source 
Deduplicate unreachable blocks, for real this time

In https://github.com/rust-lang/rust/pull/106428 (in particular 41eda69516) we noticed that inlining `unreachable_unchecked` can produce duplicate unreachable blocks. So we improved two MIR optimizations: `SimplifyCfg` was given a simplify to deduplicate unreachable blocks, then `InstCombine` was given a combiner to deduplicate switch targets that point at the same block. The problem is that change doesn't actually work.

Our current pass order is
```
SimplifyCfg (does nothing relevant to this situation)
Inline (produces multiple unreachable blocks)
InstCombine (doesn't do anything here, oops)
SimplifyCfg (produces the duplicate SwitchTargets that InstCombine is looking for)
```

So in here, I have factored out the specific function from `InstCombine` and placed it inside the simplify that produces the case it is looking for. This should ensure that it runs in the scenario it was designed for.

Fixes https://github.com/rust-lang/rust/issues/110551
r? `@cjgillot`
This commit is contained in:
bors 2023-04-21 15:08:02 +00:00
commit 4a03f14b09
5 changed files with 61 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

21
compiler/rustc_mir_transform/src/instcombine.rs
View file

@ -1,11 +1,9 @@
//! Performs various peephole optimizations.
use crate::simplify::combine_duplicate_switch_targets;
use crate::MirPass;
use rustc_hir::Mutability;
use rustc_middle::mir::{
BinOp, Body, CastKind, Constant, ConstantKind, LocalDecls, Operand, Place, ProjectionElem,
Rvalue, SourceInfo, Statement, StatementKind, SwitchTargets, Terminator, TerminatorKind, UnOp,
};
use rustc_middle::mir::*;
use rustc_middle::ty::layout::ValidityRequirement;
use rustc_middle::ty::util::IntTypeExt;
use rustc_middle::ty::{self, ParamEnv, SubstsRef, Ty, TyCtxt};
@ -46,7 +44,7 @@ impl<'tcx> MirPass<'tcx> for InstCombine {
&mut block.terminator.as_mut().unwrap(),
&mut block.statements,
);
ctx.combine_duplicate_switch_targets(&mut block.terminator.as_mut().unwrap());
combine_duplicate_switch_targets(block.terminator.as_mut().unwrap());
}
}
}
@ -264,19 +262,6 @@ impl<'tcx> InstCombineContext<'tcx, '_> {
terminator.kind = TerminatorKind::Goto { target: destination_block };
}
fn combine_duplicate_switch_targets(&self, terminator: &mut Terminator<'tcx>) {
let TerminatorKind::SwitchInt { targets, .. } = &mut terminator.kind
else { return };
let otherwise = targets.otherwise();
if targets.iter().any(|t| t.1 == otherwise) {
*targets = SwitchTargets::new(
targets.iter().filter(|t| t.1 != otherwise),
targets.otherwise(),
);
}
}
fn combine_intrinsic_assert(
&self,
terminator: &mut Terminator<'tcx>,

14
compiler/rustc_mir_transform/src/simplify.rs
View file

@ -278,6 +278,18 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
}
}
pub fn combine_duplicate_switch_targets(terminator: &mut Terminator<'_>) {
if let TerminatorKind::SwitchInt { targets, .. } = &mut terminator.kind {
let otherwise = targets.otherwise();
if targets.iter().any(|t| t.1 == otherwise) {
*targets = SwitchTargets::new(
targets.iter().filter(|t| t.1 != otherwise),
targets.otherwise(),
);
}
}
}
pub fn remove_duplicate_unreachable_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
struct OptApplier<'tcx> {
tcx: TyCtxt<'tcx>,
@ -298,6 +310,8 @@ pub fn remove_duplicate_unreachable_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut B
}
}
combine_duplicate_switch_targets(terminator);
self.super_terminator(terminator, location);
}
}