Skip to content

Commit 94e8c6c

Browse files
authored
Rollup merge of #123067 - Nadrieril:always-simplify-or, r=oli-obk
match lowering: consistently merge simple or-patterns There are two places where we expand or-patterns in match lowering: the main one is `test_candidates_with_or`, and there's one in `match_candidates` that's an optimization for the simple case where the whole pattern is just one or-pattern. To reduce duplication, we merge or-pattern alternatives into a single block when possible, but we only to that in `test_candidates_with_or`. This PR fixes this oversight and merges them in `match_candidates` too. This is a part of splitting up #122046 into smaller bits.
2 parents 1fd3ee0 + d1d9aa3 commit 94e8c6c

File tree

3 files changed

+28
-29
lines changed

3 files changed

+28
-29
lines changed

compiler/rustc_mir_build/src/build/matches/mod.rs

+18-10
Original file line numberDiff line numberDiff line change
@@ -1006,6 +1006,10 @@ struct Candidate<'pat, 'tcx> {
10061006
/// If the candidate matches, bindings and ascriptions must be established.
10071007
extra_data: PatternExtraData<'tcx>,
10081008

1009+
/// If we filled `self.subcandidate`, we store here the span of the or-pattern they came from.
1010+
// Invariant: it is `None` iff `subcandidates.is_empty()`.
1011+
or_span: Option<Span>,
1012+
10091013
/// The block before the `bindings` have been established.
10101014
pre_binding_block: Option<BasicBlock>,
10111015
/// The pre-binding block of the next candidate.
@@ -1028,6 +1032,7 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
10281032
extra_data: flat_pat.extra_data,
10291033
has_guard,
10301034
subcandidates: Vec::new(),
1035+
or_span: None,
10311036
otherwise_block: None,
10321037
pre_binding_block: None,
10331038
next_candidate_pre_binding_block: None,
@@ -1277,7 +1282,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
12771282
//
12781283
// only generates a single switch.
12791284
candidate.subcandidates = self.create_or_subcandidates(pats, candidate.has_guard);
1280-
candidate.match_pairs.pop();
1285+
let first_match_pair = candidate.match_pairs.pop().unwrap();
1286+
candidate.or_span = Some(first_match_pair.pattern.span);
12811287
split_or_candidate = true;
12821288
}
12831289
}
@@ -1287,8 +1293,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
12871293
// At least one of the candidates has been split into subcandidates.
12881294
// We need to change the candidate list to include those.
12891295
let mut new_candidates = Vec::new();
1290-
1291-
for candidate in candidates {
1296+
for candidate in candidates.iter_mut() {
12921297
candidate.visit_leaves(|leaf_candidate| new_candidates.push(leaf_candidate));
12931298
}
12941299
self.match_simplified_candidates(
@@ -1298,6 +1303,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
12981303
otherwise_block,
12991304
&mut *new_candidates,
13001305
);
1306+
1307+
for candidate in candidates {
1308+
self.merge_trivial_subcandidates(candidate);
1309+
}
13011310
} else {
13021311
self.match_simplified_candidates(
13031312
span,
@@ -1531,16 +1540,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15311540
&mut or_candidate_refs,
15321541
);
15331542
candidate.subcandidates = or_candidates;
1534-
self.merge_trivial_subcandidates(candidate, self.source_info(or_span));
1543+
candidate.or_span = Some(or_span);
1544+
self.merge_trivial_subcandidates(candidate);
15351545
}
15361546

15371547
/// Try to merge all of the subcandidates of the given candidate into one.
15381548
/// This avoids exponentially large CFGs in cases like `(1 | 2, 3 | 4, ...)`.
1539-
fn merge_trivial_subcandidates(
1540-
&mut self,
1541-
candidate: &mut Candidate<'_, 'tcx>,
1542-
source_info: SourceInfo,
1543-
) {
1549+
fn merge_trivial_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) {
15441550
if candidate.subcandidates.is_empty() || candidate.has_guard {
15451551
// FIXME(or_patterns; matthewjasper) Don't give up if we have a guard.
15461552
return;
@@ -1550,7 +1556,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15501556

15511557
// Not `Iterator::all` because we don't want to short-circuit.
15521558
for subcandidate in &mut candidate.subcandidates {
1553-
self.merge_trivial_subcandidates(subcandidate, source_info);
1559+
self.merge_trivial_subcandidates(subcandidate);
15541560

15551561
// FIXME(or_patterns; matthewjasper) Try to be more aggressive here.
15561562
can_merge &=
@@ -1559,6 +1565,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15591565

15601566
if can_merge {
15611567
let any_matches = self.cfg.start_new_block();
1568+
let or_span = candidate.or_span.take().unwrap();
1569+
let source_info = self.source_info(or_span);
15621570
for subcandidate in mem::take(&mut candidate.subcandidates) {
15631571
let or_block = subcandidate.pre_binding_block.unwrap();
15641572
self.cfg.goto(or_block, source_info, any_matches);

compiler/rustc_mir_build/src/build/matches/simplify.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
8282
&*candidate.match_pairs
8383
{
8484
candidate.subcandidates = self.create_or_subcandidates(pats, has_guard);
85-
candidate.match_pairs.pop();
85+
let first_match_pair = candidate.match_pairs.pop().unwrap();
86+
candidate.or_span = Some(first_match_pair.pattern.span);
8687
}
8788
candidate
8889
})

tests/mir-opt/issues/issue_75439.foo.MatchBranchSimplification.diff

+8-18
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,20 @@
2626
_3 = _1;
2727
_2 = move _3 as [u32; 4] (Transmute);
2828
StorageDead(_3);
29-
switchInt(_2[0 of 4]) -> [0: bb1, otherwise: bb6];
29+
switchInt(_2[0 of 4]) -> [0: bb1, otherwise: bb4];
3030
}
3131

3232
bb1: {
33-
switchInt(_2[1 of 4]) -> [0: bb2, otherwise: bb6];
33+
switchInt(_2[1 of 4]) -> [0: bb2, otherwise: bb4];
3434
}
3535

3636
bb2: {
37-
switchInt(_2[2 of 4]) -> [0: bb4, 4294901760: bb5, otherwise: bb6];
37+
switchInt(_2[2 of 4]) -> [0: bb3, 4294901760: bb3, otherwise: bb4];
3838
}
3939

4040
bb3: {
41+
StorageLive(_4);
42+
_4 = _2[3 of 4];
4143
StorageLive(_5);
4244
StorageLive(_6);
4345
_6 = _4;
@@ -46,27 +48,15 @@
4648
_0 = Option::<[u8; 4]>::Some(move _5);
4749
StorageDead(_5);
4850
StorageDead(_4);
49-
goto -> bb7;
51+
goto -> bb5;
5052
}
5153

5254
bb4: {
53-
StorageLive(_4);
54-
_4 = _2[3 of 4];
55-
goto -> bb3;
56-
}
57-
58-
bb5: {
59-
StorageLive(_4);
60-
_4 = _2[3 of 4];
61-
goto -> bb3;
62-
}
63-
64-
bb6: {
6555
_0 = Option::<[u8; 4]>::None;
66-
goto -> bb7;
56+
goto -> bb5;
6757
}
6858

69-
bb7: {
59+
bb5: {
7060
StorageDead(_2);
7161
return;
7262
}

0 commit comments

Comments
 (0)