Skip to content

Commit 432535d

Browse files
Refactor how SwitchInt stores jump targets
1 parent cae8bc1 commit 432535d

File tree

22 files changed

+247
-206
lines changed

22 files changed

+247
-206
lines changed

compiler/rustc_codegen_ssa/src/mir/block.rs

+19-26
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ use crate::MemFlags;
1212
use rustc_ast as ast;
1313
use rustc_hir::lang_items::LangItem;
1414
use rustc_index::vec::Idx;
15-
use rustc_middle::mir;
1615
use rustc_middle::mir::interpret::ConstValue;
1716
use rustc_middle::mir::AssertKind;
17+
use rustc_middle::mir::{self, SwitchTargets};
1818
use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt};
1919
use rustc_middle::ty::print::with_no_trimmed_paths;
2020
use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
@@ -24,8 +24,6 @@ use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
2424
use rustc_target::abi::{self, LayoutOf};
2525
use rustc_target::spec::abi::Abi;
2626

27-
use std::borrow::Cow;
28-
2927
/// Used by `FunctionCx::codegen_terminator` for emitting common patterns
3028
/// e.g., creating a basic block, calling a function, etc.
3129
struct TerminatorCodegenHelper<'tcx> {
@@ -198,42 +196,37 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
198196
mut bx: Bx,
199197
discr: &mir::Operand<'tcx>,
200198
switch_ty: Ty<'tcx>,
201-
values: &Cow<'tcx, [u128]>,
202-
targets: &Vec<mir::BasicBlock>,
199+
targets: &SwitchTargets<'tcx>,
203200
) {
204201
let discr = self.codegen_operand(&mut bx, &discr);
205202
// `switch_ty` is redundant, sanity-check that.
206203
assert_eq!(discr.layout.ty, switch_ty);
207-
if targets.len() == 2 {
208-
// If there are two targets, emit br instead of switch
209-
let lltrue = helper.llblock(self, targets[0]);
210-
let llfalse = helper.llblock(self, targets[1]);
204+
helper.maybe_sideeffect(self.mir, &mut bx, targets.all_targets());
205+
206+
let mut target_iter = targets.iter();
207+
if target_iter.len() == 1 {
208+
// If there are two targets (one conditional, one fallback), emit br instead of switch
209+
let (test_value, target) = target_iter.next().unwrap();
210+
let lltrue = helper.llblock(self, target);
211+
let llfalse = helper.llblock(self, targets.otherwise());
211212
if switch_ty == bx.tcx().types.bool {
212-
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
213213
// Don't generate trivial icmps when switching on bool
214-
if let [0] = values[..] {
215-
bx.cond_br(discr.immediate(), llfalse, lltrue);
216-
} else {
217-
assert_eq!(&values[..], &[1]);
218-
bx.cond_br(discr.immediate(), lltrue, llfalse);
214+
match test_value {
215+
0 => bx.cond_br(discr.immediate(), llfalse, lltrue),
216+
1 => bx.cond_br(discr.immediate(), lltrue, llfalse),
217+
_ => bug!(),
219218
}
220219
} else {
221220
let switch_llty = bx.immediate_backend_type(bx.layout_of(switch_ty));
222-
let llval = bx.const_uint_big(switch_llty, values[0]);
221+
let llval = bx.const_uint_big(switch_llty, test_value);
223222
let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval);
224-
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
225223
bx.cond_br(cmp, lltrue, llfalse);
226224
}
227225
} else {
228-
helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
229-
let (otherwise, targets) = targets.split_last().unwrap();
230226
bx.switch(
231227
discr.immediate(),
232-
helper.llblock(self, *otherwise),
233-
values
234-
.iter()
235-
.zip(targets)
236-
.map(|(&value, target)| (value, helper.llblock(self, *target))),
228+
helper.llblock(self, targets.otherwise()),
229+
target_iter.map(|(value, target)| (value, helper.llblock(self, target))),
237230
);
238231
}
239232
}
@@ -975,8 +968,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
975968
helper.funclet_br(self, &mut bx, target);
976969
}
977970

978-
mir::TerminatorKind::SwitchInt { ref discr, switch_ty, ref values, ref targets } => {
979-
self.codegen_switchint_terminator(helper, bx, discr, switch_ty, values, targets);
971+
mir::TerminatorKind::SwitchInt { ref discr, switch_ty, ref targets } => {
972+
self.codegen_switchint_terminator(helper, bx, discr, switch_ty, targets);
980973
}
981974

982975
mir::TerminatorKind::Return => {

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

+88-24
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,87 @@ use std::slice;
1616

1717
pub use super::query::*;
1818

19+
#[derive(Debug, Clone, TyEncodable, TyDecodable, HashStable, PartialEq)]
20+
pub struct SwitchTargets<'tcx> {
21+
/// Possible values. The locations to branch to in each case
22+
/// are found in the corresponding indices from the `targets` vector.
23+
values: Cow<'tcx, [u128]>,
24+
25+
/// Possible branch sites. The last element of this vector is used
26+
/// for the otherwise branch, so targets.len() == values.len() + 1
27+
/// should hold.
28+
//
29+
// This invariant is quite non-obvious and also could be improved.
30+
// One way to make this invariant is to have something like this instead:
31+
//
32+
// branches: Vec<(ConstInt, BasicBlock)>,
33+
// otherwise: Option<BasicBlock> // exhaustive if None
34+
//
35+
// However we’ve decided to keep this as-is until we figure a case
36+
// where some other approach seems to be strictly better than other.
37+
targets: Vec<BasicBlock>,
38+
}
39+
40+
impl<'tcx> SwitchTargets<'tcx> {
41+
/// Creates switch targets from an iterator of values and target blocks.
42+
///
43+
/// The iterator may be empty, in which case the `SwitchInt` instruction is equivalent to
44+
/// `goto otherwise;`.
45+
pub fn new(targets: impl Iterator<Item = (u128, BasicBlock)>, otherwise: BasicBlock) -> Self {
46+
let (values, mut targets): (Vec<_>, Vec<_>) = targets.unzip();
47+
targets.push(otherwise);
48+
Self { values: values.into(), targets }
49+
}
50+
51+
/// Builds a switch targets definition that jumps to `then` if the tested value equals `value`,
52+
/// and to `else_` if not.
53+
pub fn static_if(value: &'static [u128; 1], then: BasicBlock, else_: BasicBlock) -> Self {
54+
Self { values: Cow::Borrowed(value), targets: vec![then, else_] }
55+
}
56+
57+
/// Returns the fallback target that is jumped to when none of the values match the operand.
58+
pub fn otherwise(&self) -> BasicBlock {
59+
*self.targets.last().unwrap()
60+
}
61+
62+
/// Returns an iterator over the switch targets.
63+
///
64+
/// The iterator will yield tuples containing the value and corresponding target to jump to, not
65+
/// including the `otherwise` fallback target.
66+
///
67+
/// Note that this may yield 0 elements. Only the `otherwise` branch is mandatory.
68+
pub fn iter(&self) -> SwitchTargetsIter<'_> {
69+
SwitchTargetsIter { inner: self.values.iter().zip(self.targets.iter()) }
70+
}
71+
72+
/// Returns a slice with all possible jump targets (including the fallback target).
73+
pub fn all_targets(&self) -> &[BasicBlock] {
74+
&self.targets
75+
}
76+
77+
pub fn all_targets_mut(&mut self) -> &mut [BasicBlock] {
78+
&mut self.targets
79+
}
80+
}
81+
82+
pub struct SwitchTargetsIter<'a> {
83+
inner: iter::Zip<slice::Iter<'a, u128>, slice::Iter<'a, BasicBlock>>,
84+
}
85+
86+
impl<'a> Iterator for SwitchTargetsIter<'a> {
87+
type Item = (u128, BasicBlock);
88+
89+
fn next(&mut self) -> Option<Self::Item> {
90+
self.inner.next().map(|(val, bb)| (*val, *bb))
91+
}
92+
93+
fn size_hint(&self) -> (usize, Option<usize>) {
94+
self.inner.size_hint()
95+
}
96+
}
97+
98+
impl<'a> ExactSizeIterator for SwitchTargetsIter<'a> {}
99+
19100
#[derive(Clone, TyEncodable, TyDecodable, HashStable, PartialEq)]
20101
pub enum TerminatorKind<'tcx> {
21102
/// Block should have one successor in the graph; we jump there.
@@ -32,23 +113,7 @@ pub enum TerminatorKind<'tcx> {
32113
/// FIXME: remove this redundant information. Currently, it is relied on by pretty-printing.
33114
switch_ty: Ty<'tcx>,
34115

35-
/// Possible values. The locations to branch to in each case
36-
/// are found in the corresponding indices from the `targets` vector.
37-
values: Cow<'tcx, [u128]>,
38-
39-
/// Possible branch sites. The last element of this vector is used
40-
/// for the otherwise branch, so targets.len() == values.len() + 1
41-
/// should hold.
42-
//
43-
// This invariant is quite non-obvious and also could be improved.
44-
// One way to make this invariant is to have something like this instead:
45-
//
46-
// branches: Vec<(ConstInt, BasicBlock)>,
47-
// otherwise: Option<BasicBlock> // exhaustive if None
48-
//
49-
// However we’ve decided to keep this as-is until we figure a case
50-
// where some other approach seems to be strictly better than other.
51-
targets: Vec<BasicBlock>,
116+
targets: SwitchTargets<'tcx>,
52117
},
53118

54119
/// Indicates that the landing pad is finished and unwinding should
@@ -227,12 +292,10 @@ impl<'tcx> TerminatorKind<'tcx> {
227292
t: BasicBlock,
228293
f: BasicBlock,
229294
) -> TerminatorKind<'tcx> {
230-
static BOOL_SWITCH_FALSE: &[u128] = &[0];
231295
TerminatorKind::SwitchInt {
232296
discr: cond,
233297
switch_ty: tcx.types.bool,
234-
values: From::from(BOOL_SWITCH_FALSE),
235-
targets: vec![f, t],
298+
targets: SwitchTargets::static_if(&[0], f, t),
236299
}
237300
}
238301

@@ -263,7 +326,7 @@ impl<'tcx> TerminatorKind<'tcx> {
263326
| FalseUnwind { real_target: ref t, unwind: Some(ref u) } => {
264327
Some(t).into_iter().chain(slice::from_ref(u))
265328
}
266-
SwitchInt { ref targets, .. } => None.into_iter().chain(&targets[..]),
329+
SwitchInt { ref targets, .. } => None.into_iter().chain(&targets.targets[..]),
267330
FalseEdge { ref real_target, ref imaginary_target } => {
268331
Some(real_target).into_iter().chain(slice::from_ref(imaginary_target))
269332
}
@@ -297,7 +360,7 @@ impl<'tcx> TerminatorKind<'tcx> {
297360
| FalseUnwind { real_target: ref mut t, unwind: Some(ref mut u) } => {
298361
Some(t).into_iter().chain(slice::from_mut(u))
299362
}
300-
SwitchInt { ref mut targets, .. } => None.into_iter().chain(&mut targets[..]),
363+
SwitchInt { ref mut targets, .. } => None.into_iter().chain(&mut targets.targets[..]),
301364
FalseEdge { ref mut real_target, ref mut imaginary_target } => {
302365
Some(real_target).into_iter().chain(slice::from_mut(imaginary_target))
303366
}
@@ -469,11 +532,12 @@ impl<'tcx> TerminatorKind<'tcx> {
469532
match *self {
470533
Return | Resume | Abort | Unreachable | GeneratorDrop => vec![],
471534
Goto { .. } => vec!["".into()],
472-
SwitchInt { ref values, switch_ty, .. } => ty::tls::with(|tcx| {
535+
SwitchInt { ref targets, switch_ty, .. } => ty::tls::with(|tcx| {
473536
let param_env = ty::ParamEnv::empty();
474537
let switch_ty = tcx.lift(&switch_ty).unwrap();
475538
let size = tcx.layout_of(param_env.and(switch_ty)).unwrap().size;
476-
values
539+
targets
540+
.values
477541
.iter()
478542
.map(|&u| {
479543
ty::Const::from_scalar(tcx, Scalar::from_uint(u, size), switch_ty)

compiler/rustc_middle/src/mir/type_foldable.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,9 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
2121

2222
let kind = match self.kind {
2323
Goto { target } => Goto { target },
24-
SwitchInt { ref discr, switch_ty, ref values, ref targets } => SwitchInt {
24+
SwitchInt { ref discr, switch_ty, ref targets } => SwitchInt {
2525
discr: discr.fold_with(folder),
2626
switch_ty: switch_ty.fold_with(folder),
27-
values: values.clone(),
2827
targets: targets.clone(),
2928
},
3029
Drop { ref place, target, unwind } => {

compiler/rustc_middle/src/mir/visit.rs

-1
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,6 @@ macro_rules! make_mir_visitor {
453453
TerminatorKind::SwitchInt {
454454
discr,
455455
switch_ty,
456-
values: _,
457456
targets: _
458457
} => {
459458
self.visit_operand(discr, location);

compiler/rustc_mir/src/borrow_check/invalidation.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
117117
self.check_activations(location);
118118

119119
match &terminator.kind {
120-
TerminatorKind::SwitchInt { ref discr, switch_ty: _, values: _, targets: _ } => {
120+
TerminatorKind::SwitchInt { ref discr, switch_ty: _, targets: _ } => {
121121
self.consume_operand(location, discr);
122122
}
123123
TerminatorKind::Drop { place: drop_place, target: _, unwind: _ } => {

compiler/rustc_mir/src/borrow_check/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc
671671
self.check_activations(loc, span, flow_state);
672672

673673
match term.kind {
674-
TerminatorKind::SwitchInt { ref discr, switch_ty: _, values: _, targets: _ } => {
674+
TerminatorKind::SwitchInt { ref discr, switch_ty: _, targets: _ } => {
675675
self.consume_operand(loc, (discr, span), flow_state);
676676
}
677677
TerminatorKind::Drop { place: ref drop_place, target: _, unwind: _ } => {

compiler/rustc_mir/src/borrow_check/type_check/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1777,7 +1777,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
17771777
self.assert_iscleanup(body, block_data, target, is_cleanup)
17781778
}
17791779
TerminatorKind::SwitchInt { ref targets, .. } => {
1780-
for target in targets {
1780+
for target in targets.all_targets() {
17811781
self.assert_iscleanup(body, block_data, *target, is_cleanup);
17821782
}
17831783
}

compiler/rustc_mir/src/dataflow/framework/direction.rs

+8-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use rustc_index::bit_set::BitSet;
2-
use rustc_middle::mir::{self, BasicBlock, Location};
2+
use rustc_middle::mir::{self, BasicBlock, Location, SwitchTargets};
33
use rustc_middle::ty::TyCtxt;
44
use std::ops::RangeInclusive;
55

@@ -488,11 +488,10 @@ impl Direction for Forward {
488488
}
489489
}
490490

491-
SwitchInt { ref targets, ref values, ref discr, switch_ty: _ } => {
491+
SwitchInt { ref targets, ref discr, switch_ty: _ } => {
492492
let mut applier = SwitchIntEdgeEffectApplier {
493493
exit_state,
494-
targets: targets.as_ref(),
495-
values: values.as_ref(),
494+
targets,
496495
propagate,
497496
effects_applied: false,
498497
};
@@ -504,8 +503,8 @@ impl Direction for Forward {
504503
} = applier;
505504

506505
if !effects_applied {
507-
for &target in targets.iter() {
508-
propagate(target, exit_state);
506+
for target in targets.all_targets() {
507+
propagate(*target, exit_state);
509508
}
510509
}
511510
}
@@ -515,8 +514,7 @@ impl Direction for Forward {
515514

516515
struct SwitchIntEdgeEffectApplier<'a, D, F> {
517516
exit_state: &'a mut D,
518-
values: &'a [u128],
519-
targets: &'a [BasicBlock],
517+
targets: &'a SwitchTargets<'a>,
520518
propagate: F,
521519

522520
effects_applied: bool,
@@ -531,15 +529,15 @@ where
531529
assert!(!self.effects_applied);
532530

533531
let mut tmp = None;
534-
for (&value, &target) in self.values.iter().zip(self.targets.iter()) {
532+
for (value, target) in self.targets.iter() {
535533
let tmp = opt_clone_from_or_clone(&mut tmp, self.exit_state);
536534
apply_edge_effect(tmp, SwitchIntTarget { value: Some(value), target });
537535
(self.propagate)(target, tmp);
538536
}
539537

540538
// Once we get to the final, "otherwise" branch, there is no need to preserve `exit_state`,
541539
// so pass it directly to `apply_edge_effect` to save a clone of the dataflow state.
542-
let otherwise = self.targets.last().copied().unwrap();
540+
let otherwise = self.targets.otherwise();
543541
apply_edge_effect(self.exit_state, SwitchIntTarget { value: None, target: otherwise });
544542
(self.propagate)(otherwise, self.exit_state);
545543

compiler/rustc_mir/src/interpret/terminator.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
2424

2525
Goto { target } => self.go_to_block(target),
2626

27-
SwitchInt { ref discr, ref values, ref targets, switch_ty } => {
27+
SwitchInt { ref discr, ref targets, switch_ty } => {
2828
let discr = self.read_immediate(self.eval_operand(discr, None)?)?;
2929
trace!("SwitchInt({:?})", *discr);
3030
assert_eq!(discr.layout.ty, switch_ty);
3131

3232
// Branch to the `otherwise` case by default, if no match is found.
33-
assert!(!targets.is_empty());
34-
let mut target_block = targets[targets.len() - 1];
33+
assert!(!targets.iter().is_empty());
34+
let mut target_block = targets.otherwise();
3535

36-
for (index, &const_int) in values.iter().enumerate() {
36+
for (const_int, target) in targets.iter() {
3737
// Compare using binary_op, to also support pointer values
3838
let res = self
3939
.overflowing_binary_op(
@@ -43,7 +43,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
4343
)?
4444
.0;
4545
if res.to_bool()? {
46-
target_block = targets[index];
46+
target_block = target;
4747
break;
4848
}
4949
}

0 commit comments

Comments
 (0)