From 4e91b2d2b03935741a088c64b8554f592ab5bb81 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 20 Mar 2024 15:57:51 +0100 Subject: [PATCH] Expand or-candidates mixed with other candidates We can't mix them with candidates below them, but we can mix them with candidates above. --- .../rustc_mir_build/src/build/matches/mod.rs | 175 ++++++++---------- ...le_switchint.SimplifyCfg-initial.after.mir | 40 ++-- 2 files changed, 96 insertions(+), 119 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 70ba83c55d7c5..aac779b9dd00e 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1364,61 +1364,104 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { otherwise_block: BasicBlock, candidates: &mut [&mut Candidate<'pat, 'tcx>], ) { - let expand_or_pats = candidates.iter().any(|candidate| { - matches!(&*candidate.match_pairs, [MatchPair { test_case: TestCase::Or { .. }, .. }]) - }); + // We process or-patterns here. If any candidate starts with an or-pattern, we have to + // expand the or-pattern before we can proceed further. + // + // We can't expand them freely however. The rule is: if the candidate has an or-pattern as + // its only remaining match pair, we can expand it freely. If it has other match pairs, we + // can expand it but we can't process more candidates after it. + // + // If we didn't stop, the `otherwise` cases could get mixed up. E.g. in the following, the + // `otherwise` of the first `2` would be set to the second arm, so when simplifying the + // or-pattern (in `merge_trivial_subcandidates`) the `otherwise` cases would get mixed up + // and the `(1, true)` case would execute the second arm. + // ```ignore(illustrative) + // match (1, true) { + // (1 | 2, false) => {}, + // (2, _) => {}, + // _ => {} + // } + // ``` + // + // We therefore split the `candidates` slice in two, and expand or-patterns in the first + // half. + let mut expand_until = 0; + for (i, candidate) in candidates.iter().enumerate() { + if matches!( + &*candidate.match_pairs, + [MatchPair { test_case: TestCase::Or { .. }, .. }, ..] + ) { + expand_until = i + 1; + if candidate.match_pairs.len() > 1 { + break; + } + } + } + let (candidates_to_expand, remaining_candidates) = candidates.split_at_mut(expand_until); ensure_sufficient_stack(|| { - if expand_or_pats { - // Split a candidate in which the only match-pair is an or-pattern into multiple - // candidates. This is so that - // - // match x { - // 0 | 1 => { ... }, - // 2 | 3 => { ... }, - // } - // - // only generates a single switch. - let mut new_candidates = Vec::new(); - for candidate in candidates.iter_mut() { - if let [MatchPair { test_case: TestCase::Or { .. }, .. }] = + if candidates_to_expand.is_empty() { + // No candidates start with an or-pattern, we can continue. + self.match_expanded_candidates( + span, + scrutinee_span, + start_block, + otherwise_block, + remaining_candidates, + ); + } else { + // Expand one level of or-patterns for each candidate in `candidates_to_expand`. + let mut expanded_candidates = Vec::new(); + for candidate in candidates_to_expand.iter_mut() { + if let [MatchPair { test_case: TestCase::Or { .. }, .. }, ..] = &*candidate.match_pairs { - let match_pair = candidate.match_pairs.pop().unwrap(); - self.create_or_subcandidates(candidate, match_pair); + let or_match_pair = candidate.match_pairs.remove(0); + // Expand the or-pattern into subcandidates. + self.create_or_subcandidates(candidate, or_match_pair); + // Collect the newly created subcandidates. for subcandidate in candidate.subcandidates.iter_mut() { - new_candidates.push(subcandidate); + expanded_candidates.push(subcandidate); } } else { - new_candidates.push(candidate); + expanded_candidates.push(candidate); } } + + // Process the expanded candidates. + let remainder_start = self.cfg.start_new_block(); + // There might be new or-patterns obtained from expanding the old ones, so we call + // `match_candidates` again. self.match_candidates( span, scrutinee_span, start_block, - otherwise_block, - new_candidates.as_mut_slice(), + remainder_start, + expanded_candidates.as_mut_slice(), ); - for candidate in candidates { + // Simplify subcandidates and process any leftover match pairs. + for candidate in candidates_to_expand { if !candidate.subcandidates.is_empty() { - self.merge_trivial_subcandidates(candidate); + self.finalize_or_candidate(span, scrutinee_span, candidate); } } - } else { - self.match_simplified_candidates( + + // Process the remaining candidates. + self.match_candidates( span, scrutinee_span, - start_block, + remainder_start, otherwise_block, - candidates, + remaining_candidates, ); } }); } - fn match_simplified_candidates( + /// Construct the decision tree for `candidates`. Caller must ensure that no candidate in + /// `candidates` starts with an or-pattern. + fn match_expanded_candidates( &mut self, span: Span, scrutinee_span: Span, @@ -1443,7 +1486,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // The first candidate has satisfied all its match pairs; we link it up and continue // with the remaining candidates. start_block = self.select_matched_candidate(first, start_block); - self.match_simplified_candidates( + self.match_expanded_candidates( span, scrutinee_span, start_block, @@ -1453,7 +1496,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } candidates => { // The first candidate has some unsatisfied match pairs; we proceed to do more tests. - self.test_candidates_with_or( + self.test_candidates( span, scrutinee_span, candidates, @@ -1506,8 +1549,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { otherwise_block } - /// Tests a candidate where there are only or-patterns left to test, or - /// forwards to [Builder::test_candidates]. + /// Simplify subcandidates and process any leftover match pairs. The candidate should have been + /// expanded with `create_or_subcandidates`. /// /// Given a pattern `(P | Q, R | S)` we (in principle) generate a CFG like /// so: @@ -1559,45 +1602,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// | /// ... /// ``` - fn test_candidates_with_or( - &mut self, - span: Span, - scrutinee_span: Span, - candidates: &mut [&mut Candidate<'_, 'tcx>], - start_block: BasicBlock, - otherwise_block: BasicBlock, - ) { - let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap(); - assert!(first_candidate.subcandidates.is_empty()); - if !matches!(first_candidate.match_pairs[0].test_case, TestCase::Or { .. }) { - self.test_candidates(span, scrutinee_span, candidates, start_block, otherwise_block); - return; - } - - let first_match_pair = first_candidate.match_pairs.remove(0); - let remainder_start = self.cfg.start_new_block(); - // Test the alternatives of this or-pattern. - self.test_or_pattern( - span, - scrutinee_span, - first_candidate, - start_block, - remainder_start, - first_match_pair, - ); - - // Test the remaining candidates. - self.match_candidates( - span, - scrutinee_span, - remainder_start, - otherwise_block, - remaining_candidates, - ); - } - - /// Simplify subcandidates and process any leftover match pairs. The candidate should have been - /// expanded with `create_or_subcandidates`. fn finalize_or_candidate( &mut self, span: Span, @@ -1634,40 +1638,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } else { last_otherwise.unwrap() }; - self.test_candidates_with_or( + self.match_candidates( span, scrutinee_span, - &mut [leaf_candidate], or_start, or_otherwise, + &mut [leaf_candidate], ); }); } } - #[instrument(skip(self, start_block, otherwise_block, candidate, match_pair), level = "debug")] - fn test_or_pattern<'pat>( - &mut self, - span: Span, - scrutinee_span: Span, - candidate: &mut Candidate<'pat, 'tcx>, - start_block: BasicBlock, - otherwise_block: BasicBlock, - match_pair: MatchPair<'pat, 'tcx>, - ) { - let or_span = match_pair.pattern.span; - self.create_or_subcandidates(candidate, match_pair); - let mut or_candidate_refs: Vec<_> = candidate.subcandidates.iter_mut().collect(); - self.match_candidates( - or_span, - or_span, - start_block, - otherwise_block, - &mut or_candidate_refs, - ); - self.finalize_or_candidate(span, scrutinee_span, candidate); - } - /// Given a match-pair that corresponds to an or-pattern, expand each subpattern into a new /// subcandidate. Any candidate that has been expanded that way should be passed to /// `finalize_or_candidate` after its subcandidates have been processed. diff --git a/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir b/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir index 7e8901d15dab2..bff7b7a3d6b00 100644 --- a/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir @@ -10,67 +10,63 @@ fn single_switchint() -> () { StorageLive(_2); _2 = (const 1_i32, const true); PlaceMention(_2); - switchInt((_2.0: i32)) -> [1: bb6, 2: bb8, otherwise: bb1]; + switchInt((_2.0: i32)) -> [1: bb2, 2: bb4, otherwise: bb1]; } bb1: { - switchInt((_2.0: i32)) -> [1: bb3, 2: bb3, otherwise: bb2]; + switchInt((_2.0: i32)) -> [3: bb7, 4: bb7, otherwise: bb6]; } bb2: { - switchInt((_2.0: i32)) -> [3: bb5, 4: bb5, otherwise: bb4]; + switchInt((_2.1: bool)) -> [0: bb8, otherwise: bb3]; } bb3: { - falseEdge -> [real: bb12, imaginary: bb5]; + falseEdge -> [real: bb9, imaginary: bb5]; } bb4: { - _1 = const 5_i32; - goto -> bb14; + switchInt((_2.1: bool)) -> [0: bb5, otherwise: bb8]; } bb5: { - falseEdge -> [real: bb13, imaginary: bb4]; + falseEdge -> [real: bb10, imaginary: bb8]; } bb6: { - switchInt((_2.1: bool)) -> [0: bb1, otherwise: bb7]; + _1 = const 5_i32; + goto -> bb13; } bb7: { - falseEdge -> [real: bb10, imaginary: bb9]; + falseEdge -> [real: bb12, imaginary: bb6]; } bb8: { - switchInt((_2.1: bool)) -> [0: bb9, otherwise: bb1]; + falseEdge -> [real: bb11, imaginary: bb7]; } bb9: { - falseEdge -> [real: bb11, imaginary: bb3]; - } - - bb10: { _1 = const 1_i32; - goto -> bb14; + goto -> bb13; } - bb11: { + bb10: { _1 = const 2_i32; - goto -> bb14; + goto -> bb13; } - bb12: { + bb11: { _1 = const 3_i32; - goto -> bb14; + goto -> bb13; } - bb13: { + bb12: { _1 = const 4_i32; - goto -> bb14; + goto -> bb13; } - bb14: { + bb13: { StorageDead(_2); StorageDead(_1); _0 = const ();