Rollup merge of #121750 - Nadrieril:switchkind-if, r=matthewjasper

match lowering: Separate the `bool` case from other integers in `TestKind`

`TestKind::SwitchInt` had a special case for `bool` essentially everywhere it's used, so I made `TestKind::If` to handle the bool case on its own.

r? `@matthewjasper`
This commit is contained in:
Matthias Krüger 2024-03-01 22:38:49 +01:00 committed by GitHub
commit 0d2205f9a6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 59 additions and 65 deletions

View File

@ -1097,21 +1097,18 @@ enum TestKind<'tcx> {
variants: BitSet<VariantIdx>,
},
/// Test what value an integer, `bool`, or `char` has.
/// Test what value an integer or `char` has.
SwitchInt {
/// The type of the value that we're testing.
switch_ty: Ty<'tcx>,
/// The (ordered) set of values that we test for.
///
/// For integers and `char`s we create a branch to each of the values in
/// `options`, as well as an "otherwise" branch for all other values, even
/// in the (rare) case that `options` is exhaustive.
///
/// For `bool` we always generate two edges, one for `true` and one for
/// `false`.
/// We create a branch to each of the values in `options`, as well as an "otherwise" branch
/// for all other values, even in the (rare) case that `options` is exhaustive.
options: FxIndexMap<Const<'tcx>, u128>,
},
/// Test what value a `bool` has.
If,
/// Test for equality with value, possibly after an unsizing coercion to
/// `ty`,
Eq {
@ -1617,7 +1614,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// a test like SwitchInt, we may want to add cases based on the candidates that are
// available
match test.kind {
TestKind::SwitchInt { switch_ty: _, ref mut options } => {
TestKind::SwitchInt { ref mut options } => {
for candidate in candidates.iter() {
if !self.add_cases_to_switch(&match_place, candidate, options) {
break;

View File

@ -34,12 +34,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
TestKind::Switch { adt_def, variants: BitSet::new_empty(adt_def.variants().len()) }
}
TestCase::Constant { .. } if match_pair.pattern.ty.is_bool() => TestKind::If,
TestCase::Constant { .. } if is_switch_ty(match_pair.pattern.ty) => {
// For integers, we use a `SwitchInt` match, which allows
// us to handle more cases.
TestKind::SwitchInt {
switch_ty: match_pair.pattern.ty,
// these maps are empty to start; cases are
// added below in add_cases_to_switch
options: Default::default(),
@ -182,34 +182,29 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
);
}
TestKind::SwitchInt { switch_ty, ref options } => {
let terminator = if *switch_ty.kind() == ty::Bool {
assert!(!options.is_empty() && options.len() <= 2);
let [first_bb, second_bb] = *target_blocks else {
bug!("`TestKind::SwitchInt` on `bool` should have two targets")
};
let (true_bb, false_bb) = match options[0] {
1 => (first_bb, second_bb),
0 => (second_bb, first_bb),
v => span_bug!(test.span, "expected boolean value but got {:?}", v),
};
TerminatorKind::if_(Operand::Copy(place), true_bb, false_bb)
} else {
// The switch may be inexhaustive so we have a catch all block
debug_assert_eq!(options.len() + 1, target_blocks.len());
let otherwise_block = *target_blocks.last().unwrap();
let switch_targets = SwitchTargets::new(
options.values().copied().zip(target_blocks),
otherwise_block,
);
TerminatorKind::SwitchInt {
discr: Operand::Copy(place),
targets: switch_targets,
}
TestKind::SwitchInt { ref options } => {
// The switch may be inexhaustive so we have a catch-all block
debug_assert_eq!(options.len() + 1, target_blocks.len());
let otherwise_block = *target_blocks.last().unwrap();
let switch_targets = SwitchTargets::new(
options.values().copied().zip(target_blocks),
otherwise_block,
);
let terminator = TerminatorKind::SwitchInt {
discr: Operand::Copy(place),
targets: switch_targets,
};
self.cfg.terminate(block, self.source_info(match_start_span), terminator);
}
TestKind::If => {
let [false_bb, true_bb] = *target_blocks else {
bug!("`TestKind::If` should have two targets")
};
let terminator = TerminatorKind::if_(Operand::Copy(place), true_bb, false_bb);
self.cfg.terminate(block, self.source_info(match_start_span), terminator);
}
TestKind::Eq { value, ty } => {
let tcx = self.tcx;
let [success_block, fail_block] = *target_blocks else {
@ -589,14 +584,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
//
// FIXME(#29623) we could use PatKind::Range to rule
// things out here, in some cases.
(TestKind::SwitchInt { switch_ty: _, options }, TestCase::Constant { value })
(TestKind::SwitchInt { options }, TestCase::Constant { value })
if is_switch_ty(match_pair.pattern.ty) =>
{
fully_matched = true;
let index = options.get_index_of(value).unwrap();
Some(index)
}
(TestKind::SwitchInt { switch_ty: _, options }, TestCase::Range(range)) => {
(TestKind::SwitchInt { options }, TestCase::Range(range)) => {
fully_matched = false;
let not_contained =
self.values_not_contained_in_range(&*range, options).unwrap_or(false);
@ -608,6 +603,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
})
}
(&TestKind::If, TestCase::Constant { value }) => {
fully_matched = true;
let value = value.try_eval_bool(self.tcx, self.param_env).unwrap_or_else(|| {
span_bug!(test.span, "expected boolean value but got {value:?}")
});
Some(value as usize)
}
(&TestKind::If, _) => {
fully_matched = false;
None
}
(
&TestKind::Len { len: test_len, op: BinOp::Eq },
&TestCase::Slice { len, variable_length },
@ -746,7 +753,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
impl Test<'_> {
pub(super) fn targets(&self) -> usize {
match self.kind {
TestKind::Eq { .. } | TestKind::Range(_) | TestKind::Len { .. } => 2,
TestKind::Eq { .. } | TestKind::Range(_) | TestKind::Len { .. } | TestKind::If => 2,
TestKind::Switch { adt_def, .. } => {
// While the switch that we generate doesn't test for all
// variants, we have a target for each variant and the
@ -754,21 +761,13 @@ impl Test<'_> {
// specified have the same block.
adt_def.variants().len() + 1
}
TestKind::SwitchInt { switch_ty, ref options, .. } => {
if switch_ty.is_bool() {
// `bool` is special cased in `perform_test` to always
// branch to two blocks.
2
} else {
options.len() + 1
}
}
TestKind::SwitchInt { ref options } => options.len() + 1,
}
}
}
fn is_switch_ty(ty: Ty<'_>) -> bool {
ty.is_integral() || ty.is_char() || ty.is_bool()
ty.is_integral() || ty.is_char()
}
fn trait_method<'tcx>(

View File

@ -42,15 +42,11 @@
}
bb2: {
- switchInt((_2.0: bool)) -> [0: bb4, otherwise: bb3];
- switchInt((_2.0: bool)) -> [0: bb3, otherwise: bb4];
+ switchInt((_2.0: bool)) -> [0: bb3, otherwise: bb17];
}
bb3: {
- falseEdge -> [real: bb20, imaginary: bb4];
- }
-
- bb4: {
StorageLive(_15);
_15 = (_2.1: bool);
StorageLive(_16);
@ -59,8 +55,12 @@
+ goto -> bb16;
}
bb4: {
- falseEdge -> [real: bb20, imaginary: bb3];
- }
-
- bb5: {
- falseEdge -> [real: bb13, imaginary: bb3];
- falseEdge -> [real: bb13, imaginary: bb4];
- }
-
- bb6: {
@ -68,7 +68,6 @@
- }
-
- bb7: {
+ bb4: {
_0 = const 1_i32;
- drop(_7) -> [return: bb18, unwind: bb25];
+ drop(_7) -> [return: bb15, unwind: bb22];
@ -184,7 +183,7 @@
StorageDead(_12);
StorageDead(_8);
StorageDead(_6);
- falseEdge -> [real: bb2, imaginary: bb3];
- falseEdge -> [real: bb2, imaginary: bb4];
+ goto -> bb2;
}

View File

@ -42,15 +42,11 @@
}
bb2: {
- switchInt((_2.0: bool)) -> [0: bb4, otherwise: bb3];
- switchInt((_2.0: bool)) -> [0: bb3, otherwise: bb4];
+ switchInt((_2.0: bool)) -> [0: bb3, otherwise: bb17];
}
bb3: {
- falseEdge -> [real: bb20, imaginary: bb4];
- }
-
- bb4: {
StorageLive(_15);
_15 = (_2.1: bool);
StorageLive(_16);
@ -59,8 +55,12 @@
+ goto -> bb16;
}
bb4: {
- falseEdge -> [real: bb20, imaginary: bb3];
- }
-
- bb5: {
- falseEdge -> [real: bb13, imaginary: bb3];
- falseEdge -> [real: bb13, imaginary: bb4];
- }
-
- bb6: {
@ -68,7 +68,6 @@
- }
-
- bb7: {
+ bb4: {
_0 = const 1_i32;
- drop(_7) -> [return: bb18, unwind: bb25];
+ drop(_7) -> [return: bb15, unwind: bb22];
@ -184,7 +183,7 @@
StorageDead(_12);
StorageDead(_8);
StorageDead(_6);
- falseEdge -> [real: bb2, imaginary: bb3];
- falseEdge -> [real: bb2, imaginary: bb4];
+ goto -> bb2;
}