Skip to content

Commit

Permalink
Return the otherwise_block instead of passing it as argument
Browse files Browse the repository at this point in the history 
This saves a few blocks and matches the common `unpack!` paradigm.
  • Loading branch information
@Nadrieril
Nadrieril committed Jun 30, 2024
1 parent eff1a86 commit d90c959
Show file tree
Hide file tree
Showing 19 changed files with 333 additions and 461 deletions.
86 changes: 29 additions & 57 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1250,11 +1250,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
candidates: &mut [&mut Candidate<'pat, 'tcx>],
refutable: bool,
) -> BasicBlock {
// This will generate code to test scrutinee_place and branch to the appropriate arm block.
// See the doc comment on `match_candidates` for why we have an otherwise block.
let otherwise_block = self.cfg.start_new_block();

// This will generate code to test scrutinee_place and branch to the appropriate arm block
self.match_candidates(match_start_span, scrutinee_span, block, otherwise_block, candidates);
let otherwise_block =
self.match_candidates(match_start_span, scrutinee_span, block, candidates);

// Link each leaf candidate to the `false_edge_start_block` of the next one.
let mut previous_candidate: Option<&mut Candidate<'_, '_>> = None;
Expand Down Expand Up @@ -1305,27 +1304,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
otherwise_block
}

/// The main match algorithm. It begins with a set of candidates
/// `candidates` and has the job of generating code to determine
/// which of these candidates, if any, is the correct one. The
/// The main match algorithm. It begins with a set of candidates `candidates` and has the job of
/// generating code that branches to an appropriate block if the scrutinee matches one of these
/// candidates. The
/// candidates are sorted such that the first item in the list
/// has the highest priority. When a candidate is found to match
/// the value, we will set and generate a branch to the appropriate
/// pre-binding block.
///
/// If we find that *NONE* of the candidates apply, we branch to `otherwise_block`.
/// If none of the candidates apply, we continue to the returned `otherwise_block`.
///
/// It might be surprising that the input can be non-exhaustive.
/// Indeed, initially, it is not, because all matches are
/// Indeed, for matches, initially, it is not, because all matches are
/// exhaustive in Rust. But during processing we sometimes divide
/// up the list of candidates and recurse with a non-exhaustive
/// list. This is how our lowering approach (called "backtracking
/// automaton" in the literature) works.
/// See [`Builder::test_candidates`] for more details.
///
/// If `fake_borrows` is `Some`, then places which need fake borrows
/// will be added to it.
///
/// For an example of how we use `otherwise_block`, consider:
/// ```
/// # fn foo((x, y): (bool, bool)) -> u32 {
Expand All @@ -1350,7 +1346,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// }
/// if y {
/// if x {
/// // This is actually unreachable because the `(true, true)` case was handled above.
/// // This is actually unreachable because the `(true, true)` case was handled above,
/// // but we don't know that from within the lowering algorithm.
/// // continue
/// } else {
/// return 3
Expand All @@ -1367,25 +1364,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// the algorithm. For more details on why we lower like this, see [`Builder::test_candidates`].
///
/// Note how we test `x` twice. This is the tradeoff of backtracking automata: we prefer smaller
/// code size at the expense of non-optimal code paths.
/// code size so we accept non-optimal code paths.
#[instrument(skip(self), level = "debug")]
fn match_candidates(
&mut self,
span: Span,
scrutinee_span: Span,
start_block: BasicBlock,
otherwise_block: BasicBlock,
candidates: &mut [&mut Candidate<'_, 'tcx>],
) {
) -> BasicBlock {
ensure_sufficient_stack(|| {
self.match_candidates_with_enough_stack(
span,
scrutinee_span,
start_block,
otherwise_block,
candidates,
)
});
self.match_candidates_with_enough_stack(span, scrutinee_span, start_block, candidates)
})
}

/// Construct the decision tree for `candidates`. Don't call this, call `match_candidates`
Expand All @@ -1395,9 +1385,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: Span,
scrutinee_span: Span,
start_block: BasicBlock,
otherwise_block: BasicBlock,
candidates: &mut [&mut Candidate<'_, 'tcx>],
) {
) -> BasicBlock {
if let [first, ..] = candidates {
if first.false_edge_start_block.is_none() {
first.false_edge_start_block = Some(start_block);
Expand All @@ -1408,9 +1397,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let rest = match candidates {
[] => {
// If there are no candidates that still need testing, we're done.
let source_info = self.source_info(span);
self.cfg.goto(start_block, source_info, otherwise_block);
return;
return start_block;
}
[first, remaining @ ..] if first.match_pairs.is_empty() => {
// The first candidate has satisfied all its match pairs; we link it up and continue
Expand All @@ -1431,13 +1418,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

// Process any candidates that remain.
let BlockAnd(start_block, remaining_candidates) = rest;
self.match_candidates(
span,
scrutinee_span,
start_block,
otherwise_block,
remaining_candidates,
);
self.match_candidates(span, scrutinee_span, start_block, remaining_candidates)
}

/// Link up matched candidates.
Expand Down Expand Up @@ -1542,14 +1523,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}

// 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(
let remainder_start = self.match_candidates(
span,
scrutinee_span,
start_block,
remainder_start,
expanded_candidates.as_mut_slice(),
);

Expand Down Expand Up @@ -1648,6 +1625,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.merge_trivial_subcandidates(candidate);

if !candidate.match_pairs.is_empty() {
let or_span = candidate.or_span.unwrap_or(candidate.extra_data.span);
let source_info = self.source_info(or_span);
// If more match pairs remain, test them after each subcandidate.
// We could add them to the or-candidates before the call to `test_or_pattern` but this
// would make it impossible to detect simplifiable or-patterns. That would guarantee
Expand All @@ -1661,6 +1640,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
assert!(leaf_candidate.match_pairs.is_empty());
leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned());
let or_start = leaf_candidate.pre_binding_block.unwrap();
let otherwise =
self.match_candidates(span, scrutinee_span, or_start, &mut [leaf_candidate]);
// In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q,
// R | S)` will fail too. If there is no guard, we skip testing of `Q` by branching
// directly to `last_otherwise`. If there is a guard,
Expand All @@ -1671,13 +1652,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} else {
last_otherwise.unwrap()
};
self.match_candidates(
span,
scrutinee_span,
or_start,
or_otherwise,
&mut [leaf_candidate],
);
self.cfg.goto(otherwise, source_info, or_otherwise);
});
}
}
Expand Down Expand Up @@ -1956,15 +1931,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let target_blocks: FxIndexMap<_, _> = target_candidates
.into_iter()
.map(|(branch, mut candidates)| {
let candidate_start = self.cfg.start_new_block();
self.match_candidates(
span,
scrutinee_span,
candidate_start,
remainder_start,
&mut *candidates,
);
(branch, candidate_start)
let branch_start = self.cfg.start_new_block();
let branch_otherwise =
self.match_candidates(span, scrutinee_span, branch_start, &mut *candidates);
let source_info = self.source_info(span);
self.cfg.goto(branch_otherwise, source_info, remainder_start);
(branch, branch_start)
})
.collect();

Expand Down
18 changes: 7 additions & 11 deletions tests/mir-opt/building/issue_101867.main.built.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ fn main() -> () {
StorageLive(_5);
PlaceMention(_1);
_6 = discriminant(_1);
switchInt(move _6) -> [1: bb5, otherwise: bb4];
switchInt(move _6) -> [1: bb4, otherwise: bb3];
}

bb1: {
StorageLive(_3);
StorageLive(_4);
_4 = begin_panic::<&str>(const "explicit panic") -> bb9;
_4 = begin_panic::<&str>(const "explicit panic") -> bb8;
}

bb2: {
Expand All @@ -43,35 +43,31 @@ fn main() -> () {
}

bb3: {
goto -> bb8;
goto -> bb7;
}

bb4: {
goto -> bb3;
falseEdge -> [real: bb6, imaginary: bb3];
}

bb5: {
falseEdge -> [real: bb7, imaginary: bb3];
goto -> bb3;
}

bb6: {
goto -> bb4;
}

bb7: {
_5 = ((_1 as Some).0: u8);
_0 = const ();
StorageDead(_5);
StorageDead(_1);
return;
}

bb8: {
bb7: {
StorageDead(_5);
goto -> bb1;
}

bb9 (cleanup): {
bb8 (cleanup): {
resume;
}
}
34 changes: 15 additions & 19 deletions tests/mir-opt/building/issue_49232.main.built.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ fn main() -> () {
}

bb1: {
falseUnwind -> [real: bb2, unwind: bb15];
falseUnwind -> [real: bb2, unwind: bb14];
}

bb2: {
StorageLive(_2);
StorageLive(_3);
_3 = const true;
PlaceMention(_3);
switchInt(_3) -> [0: bb5, otherwise: bb7];
switchInt(_3) -> [0: bb4, otherwise: bb6];
}

bb3: {
Expand All @@ -34,63 +34,59 @@ fn main() -> () {
}

bb4: {
goto -> bb3;
falseEdge -> [real: bb8, imaginary: bb6];
}

bb5: {
falseEdge -> [real: bb9, imaginary: bb7];
goto -> bb3;
}

bb6: {
goto -> bb4;
_0 = const ();
goto -> bb13;
}

bb7: {
_0 = const ();
goto -> bb14;
goto -> bb3;
}

bb8: {
goto -> bb4;
_2 = const 4_i32;
goto -> bb11;
}

bb9: {
_2 = const 4_i32;
goto -> bb12;
unreachable;
}

bb10: {
unreachable;
goto -> bb11;
}

bb11: {
goto -> bb12;
}

bb12: {
FakeRead(ForLet(None), _2);
StorageDead(_3);
StorageLive(_5);
StorageLive(_6);
_6 = &_2;
_5 = std::mem::drop::<&i32>(move _6) -> [return: bb13, unwind: bb15];
_5 = std::mem::drop::<&i32>(move _6) -> [return: bb12, unwind: bb14];
}

bb13: {
bb12: {
StorageDead(_6);
StorageDead(_5);
_1 = const ();
StorageDead(_2);
goto -> bb1;
}

bb14: {
bb13: {
StorageDead(_3);
StorageDead(_2);
return;
}

bb15 (cleanup): {
bb14 (cleanup): {
resume;
}
}
Loading

0 comments on commit d90c959

Please sign in to comment.