Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit cb1df69

Browse files
committed
MIR: Stop needing an explicit BB for otherwise:unreachable
So many places to update :D For this PR I tried to keep things doing essentially the same thing as before. No new passes to try to use it more, no change to MIR building for exhaustive matches, etc. That said, `UnreachableProp` still picks it up, and thus there's still a bunch of `otherwise` arms removed and `unreachable` blocks that no longer show.
1 parent fa0068b commit cb1df69

File tree

50 files changed

+340
-257
lines changed

Some content is hidden

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

50 files changed

+340
-257
lines changed

‎compiler/rustc_codegen_cranelift/src/base.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ pub(crate) fn codegen_fn<'tcx>(
9595
bcx,
9696
block_map,
9797
local_map: IndexVec::with_capacity(mir.local_decls.len()),
98+
shared_unreachable: None,
9899
caller_location: None, // set by `codegen_fn_prelude`
99100

100101
clif_comments,
@@ -404,7 +405,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
404405
assert_eq!(targets.iter().count(), 1);
405406
let (then_value, then_block) = targets.iter().next().unwrap();
406407
let then_block = fx.get_block(then_block);
407-
let else_block = fx.get_block(targets.otherwise());
408+
let else_block = fx.get_switch_block(targets.otherwise());
408409
let test_zero = match then_value {
409410
0 => true,
410411
1 => false,
@@ -435,7 +436,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
435436
let block = fx.get_block(block);
436437
switch.set_entry(value, block);
437438
}
438-
let otherwise_block = fx.get_block(targets.otherwise());
439+
let otherwise_block = fx.get_switch_block(targets.otherwise());
439440
switch.emit(&mut fx.bcx, discr, otherwise_block);
440441
}
441442
}
@@ -520,6 +521,11 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
520521
}
521522
};
522523
}
524+
525+
if let Some(unreachable) = fx.shared_unreachable {
526+
fx.bcx.switch_to_block(unreachable);
527+
fx.bcx.ins().trap(TrapCode::UnreachableCodeReached);
528+
}
523529
}
524530

525531
fn codegen_stmt<'tcx>(

‎compiler/rustc_codegen_cranelift/src/common.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ pub(crate) struct FunctionCx<'m, 'clif, 'tcx: 'm> {
296296
pub(crate) bcx: FunctionBuilder<'clif>,
297297
pub(crate) block_map: IndexVec<BasicBlock, Block>,
298298
pub(crate) local_map: IndexVec<Local, CPlace<'tcx>>,
299+
pub(crate) shared_unreachable: Option<Block>,
299300

300301
/// When `#[track_caller]` is used, the implicit caller location is stored in this variable.
301302
pub(crate) caller_location: Option<CValue<'tcx>>,
@@ -377,6 +378,17 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
377378
*self.block_map.get(bb).unwrap()
378379
}
379380

381+
pub(crate) fn get_switch_block(&mut self, sa: SwitchAction) -> Block {
382+
match sa {
383+
SwitchAction::Goto(bb) => self.get_block(bb),
384+
SwitchAction::Unreachable => self.unreachable_block(),
385+
}
386+
}
387+
388+
pub(crate) fn unreachable_block(&mut self) -> Block {
389+
*self.shared_unreachable.get_or_insert_with(|| self.bcx.create_block())
390+
}
391+
380392
pub(crate) fn get_local_place(&mut self, local: Local) -> CPlace<'tcx> {
381393
*self.local_map.get(local).unwrap_or_else(|| {
382394
panic!("Local {:?} doesn't exist", local);

‎compiler/rustc_codegen_ssa/src/mir/block.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,17 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
9595
}
9696
}
9797

98+
fn llbb_with_cleanup_from_switch_action<Bx: BuilderMethods<'a, 'tcx>>(
99+
&self,
100+
fx: &mut FunctionCx<'a, 'tcx, Bx>,
101+
target: mir::SwitchAction,
102+
) -> Bx::BasicBlock {
103+
match target {
104+
mir::SwitchAction::Unreachable => fx.unreachable_block(),
105+
mir::SwitchAction::Goto(bb) => self.llbb_with_cleanup(fx, bb),
106+
}
107+
}
108+
98109
fn llbb_characteristics<Bx: BuilderMethods<'a, 'tcx>>(
99110
&self,
100111
fx: &mut FunctionCx<'a, 'tcx, Bx>,
@@ -367,7 +378,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
367378
// If our discriminant is a constant we can branch directly
368379
if let Some(const_discr) = bx.const_to_opt_u128(discr_value, false) {
369380
let target = targets.target_for_value(const_discr);
370-
bx.br(helper.llbb_with_cleanup(self, target));
381+
bx.br(helper.llbb_with_cleanup_from_switch_action(self, target));
371382
return;
372383
};
373384

@@ -377,7 +388,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
377388
// `switch`.
378389
let (test_value, target) = target_iter.next().unwrap();
379390
let lltrue = helper.llbb_with_cleanup(self, target);
380-
let llfalse = helper.llbb_with_cleanup(self, targets.otherwise());
391+
let llfalse = helper.llbb_with_cleanup_from_switch_action(self, targets.otherwise());
381392
if switch_ty == bx.tcx().types.bool {
382393
// Don't generate trivial icmps when switching on bool.
383394
match test_value {
@@ -393,7 +404,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
393404
}
394405
} else if self.cx.sess().opts.optimize == OptLevel::No
395406
&& target_iter.len() == 2
396-
&& self.mir[targets.otherwise()].is_empty_unreachable()
407+
&& self.mir.basic_blocks.is_empty_unreachable(targets.otherwise())
397408
{
398409
// In unoptimized builds, if there are two normal targets and the `otherwise` target is
399410
// an unreachable BB, emit `br` instead of `switch`. This leaves behind the unreachable
@@ -418,7 +429,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
418429
} else {
419430
bx.switch(
420431
discr_value,
421-
helper.llbb_with_cleanup(self, targets.otherwise()),
432+
helper.llbb_with_cleanup_from_switch_action(self, targets.otherwise()),
422433
target_iter.map(|(value, target)| (value, helper.llbb_with_cleanup(self, target))),
423434
);
424435
}
@@ -1644,11 +1655,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
16441655
}
16451656

16461657
fn unreachable_block(&mut self) -> Bx::BasicBlock {
1647-
self.unreachable_block.unwrap_or_else(|| {
1658+
*self.unreachable_block.get_or_insert_with(|| {
16481659
let llbb = Bx::append_block(self.cx, self.llfn, "unreachable");
16491660
let mut bx = Bx::build(self.cx, llbb);
16501661
bx.unreachable();
1651-
self.unreachable_block = Some(llbb);
16521662
llbb
16531663
})
16541664
}

‎compiler/rustc_const_eval/src/interpret/terminator.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
101101
&ImmTy::from_uint(const_int, discr.layout),
102102
)?;
103103
if res.to_scalar().to_bool()? {
104-
target_block = target;
104+
target_block = mir::SwitchAction::Goto(target);
105105
break;
106106
}
107107
}
108108

109-
self.go_to_block(target_block);
109+
if let mir::SwitchAction::Goto(target_block) = target_block {
110+
self.go_to_block(target_block);
111+
} else {
112+
throw_ub!(Unreachable)
113+
}
110114
}
111115

112116
Call {

‎compiler/rustc_const_eval/src/transform/validate.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,10 +373,9 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
373373
self.check_edge(location, *target, EdgeKind::Normal);
374374
}
375375
TerminatorKind::SwitchInt { targets, discr: _ } => {
376-
for (_,target) in targets.iter() {
376+
for &target in targets.all_targets() {
377377
self.check_edge(location, target, EdgeKind::Normal);
378378
}
379-
self.check_edge(location, targets.otherwise(), EdgeKind::Normal);
380379

381380
self.value_cache.clear();
382381
self.value_cache.extend(targets.iter().map(|(value, _)| value));

‎compiler/rustc_middle/src/mir/basic_blocks.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use crate::mir::traversal::Postorder;
2-
use crate::mir::{BasicBlock, BasicBlockData, Terminator, TerminatorKind, START_BLOCK};
2+
use crate::mir::{
3+
BasicBlock, BasicBlockData, SwitchAction, Terminator, TerminatorKind, START_BLOCK,
4+
};
35

46
use rustc_data_structures::fx::FxHashMap;
57
use rustc_data_structures::graph;
@@ -90,7 +92,9 @@ impl<'tcx> BasicBlocks<'tcx> {
9092
for (value, target) in targets.iter() {
9193
switch_sources.entry((target, bb)).or_default().push(Some(value));
9294
}
93-
switch_sources.entry((targets.otherwise(), bb)).or_default().push(None);
95+
if let SwitchAction::Goto(otherwise) = targets.otherwise() {
96+
switch_sources.entry((otherwise, bb)).or_default().push(None);
97+
}
9498
}
9599
}
96100
switch_sources
@@ -128,6 +132,13 @@ impl<'tcx> BasicBlocks<'tcx> {
128132
pub fn invalidate_cfg_cache(&mut self) {
129133
self.cache = Cache::default();
130134
}
135+
136+
pub fn is_empty_unreachable(&self, action: SwitchAction) -> bool {
137+
match action {
138+
SwitchAction::Goto(bb) => self[bb].is_empty_unreachable(),
139+
SwitchAction::Unreachable => true,
140+
}
141+
}
131142
}
132143

133144
impl<'tcx> std::ops::Deref for BasicBlocks<'tcx> {

‎compiler/rustc_middle/src/mir/pretty.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -875,12 +875,14 @@ impl<'tcx> TerminatorKind<'tcx> {
875875
match *self {
876876
Return | UnwindResume | UnwindTerminate(_) | Unreachable | CoroutineDrop => vec![],
877877
Goto { .. } => vec!["".into()],
878-
SwitchInt { ref targets, .. } => targets
879-
.values
880-
.iter()
881-
.map(|&u| Cow::Owned(u.to_string()))
882-
.chain(iter::once("otherwise".into()))
883-
.collect(),
878+
SwitchInt { ref targets, .. } => {
879+
let mut labels: Vec<_> =
880+
targets.values.iter().map(|&u| Cow::Owned(u.to_string())).collect();
881+
if let SwitchAction::Goto(_) = targets.otherwise() {
882+
labels.push("otherwise".into());
883+
}
884+
labels
885+
}
884886
Call { target: Some(_), unwind: UnwindAction::Cleanup(_), .. } => {
885887
vec!["return".into(), "unwind".into()]
886888
}

‎compiler/rustc_middle/src/mir/syntax.rs

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -828,21 +828,51 @@ pub struct SwitchTargets {
828828
/// are found in the corresponding indices from the `targets` vector.
829829
pub(super) values: SmallVec<[Pu128; 1]>,
830830

831-
/// Possible branch sites. The last element of this vector is used
832-
/// for the otherwise branch, so targets.len() == values.len() + 1
833-
/// should hold.
831+
/// Possible branch sites.
832+
///
833+
/// If `targets.len() == values.len() + 1`, the last element of this vector
834+
/// is used for the otherwise branch in [`SwitchAction::Goto`].
835+
///
836+
/// If `targets.len() == values.len()`, the otherwise branch is
837+
/// [`SwitchAction::Unreachable`].
838+
///
839+
/// You must ensure that we're always in at one of those cases.
834840
//
835841
// This invariant is quite non-obvious and also could be improved.
836842
// One way to make this invariant is to have something like this instead:
837843
//
838844
// branches: Vec<(ConstInt, BasicBlock)>,
839-
// otherwise: Option<BasicBlock> // exhaustive if None
845+
// otherwise: SwitchAction,
840846
//
841847
// However we’ve decided to keep this as-is until we figure a case
842848
// where some other approach seems to be strictly better than other.
843849
pub(super) targets: SmallVec<[BasicBlock; 2]>,
844850
}
845851

852+
/// The action to be taken for a particular input to the [`TerminatorKind::SwitchInt`]
853+
#[derive(Copy, Clone, Debug, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable)]
854+
#[derive(TypeFoldable, TypeVisitable)]
855+
pub enum SwitchAction {
856+
/// Jump to the specified block
857+
Goto(BasicBlock),
858+
/// Triggers undefined behavior
859+
///
860+
/// This is equivalent to jumping to a block with [`TerminatorKind::Unreachable`],
861+
/// but lets us not have to store such a block in the body.
862+
/// It also makes it easier in analysis to know this action is unreachable
863+
/// without needing to have all the basic blocks around to look at.
864+
Unreachable,
865+
}
866+
867+
impl SwitchAction {
868+
pub fn into_terminator<'tcx>(self) -> TerminatorKind<'tcx> {
869+
match self {
870+
SwitchAction::Goto(bb) => TerminatorKind::Goto { target: bb },
871+
SwitchAction::Unreachable => TerminatorKind::Unreachable,
872+
}
873+
}
874+
}
875+
846876
/// Action to be taken when a stack unwind happens.
847877
#[derive(Copy, Clone, Debug, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable)]
848878
#[derive(TypeFoldable, TypeVisitable)]

‎compiler/rustc_middle/src/mir/terminator.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ impl SwitchTargets {
1414
///
1515
/// The iterator may be empty, in which case the `SwitchInt` instruction is equivalent to
1616
/// `goto otherwise;`.
17-
pub fn new(targets: impl Iterator<Item = (u128, BasicBlock)>, otherwise: BasicBlock) -> Self {
17+
pub fn new(targets: impl Iterator<Item = (u128, BasicBlock)>, otherwise: SwitchAction) -> Self {
1818
let (values, mut targets): (SmallVec<_>, SmallVec<_>) =
1919
targets.map(|(v, t)| (Pu128(v), t)).unzip();
20-
targets.push(otherwise);
20+
if let SwitchAction::Goto(otherwise) = otherwise {
21+
targets.push(otherwise);
22+
}
2123
Self { values, targets }
2224
}
2325

@@ -39,10 +41,17 @@ impl SwitchTargets {
3941
}
4042
}
4143

42-
/// Returns the fallback target that is jumped to when none of the values match the operand.
44+
/// Returns the fallback action when none of the values match the operand.
4345
#[inline]
44-
pub fn otherwise(&self) -> BasicBlock {
45-
*self.targets.last().unwrap()
46+
pub fn otherwise(&self) -> SwitchAction {
47+
debug_assert!(
48+
self.targets.len() == self.values.len() || self.targets.len() == self.values.len() + 1
49+
);
50+
if self.targets.len() > self.values.len() {
51+
SwitchAction::Goto(*self.targets.last().unwrap())
52+
} else {
53+
SwitchAction::Unreachable
54+
}
4655
}
4756

4857
/// Returns an iterator over the switch targets.
@@ -71,8 +80,9 @@ impl SwitchTargets {
7180
/// specific value. This cannot fail, as it'll return the `otherwise`
7281
/// branch if there's not a specific match for the value.
7382
#[inline]
74-
pub fn target_for_value(&self, value: u128) -> BasicBlock {
75-
self.iter().find_map(|(v, t)| (v == value).then_some(t)).unwrap_or_else(|| self.otherwise())
83+
pub fn target_for_value(&self, value: u128) -> SwitchAction {
84+
let specific = self.iter().find_map(|(v, t)| (v == value).then_some(t));
85+
if let Some(specific) = specific { SwitchAction::Goto(specific) } else { self.otherwise() }
7686
}
7787

7888
/// Adds a new target to the switch. But You cannot add an already present value.
@@ -82,8 +92,12 @@ impl SwitchTargets {
8292
if self.values.contains(&value) {
8393
bug!("target value {:?} already present", value);
8494
}
95+
96+
// There may or may not be a fallback in `targets`, so make sure to
97+
// insert the new target at the same index as its value.
98+
let insert_point = self.values.len();
8599
self.values.push(value);
86-
self.targets.insert(self.targets.len() - 1, bb);
100+
self.targets.insert(insert_point, bb);
87101
}
88102

89103
/// Returns true if all targets (including the fallback target) are distinct.

‎compiler/rustc_middle/src/mir/traversal.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,8 +361,9 @@ impl<'a, 'tcx> Iterator for MonoReachable<'a, 'tcx> {
361361
if let Some((bits, targets)) =
362362
Body::try_const_mono_switchint(self.tcx, self.instance, data)
363363
{
364-
let target = targets.target_for_value(bits);
365-
self.add_work([target]);
364+
if let SwitchAction::Goto(target) = targets.target_for_value(bits) {
365+
self.add_work([target]);
366+
}
366367
} else {
367368
self.add_work(data.terminator().successors());
368369
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /