Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

match lowering: simplify empty candidate selection #121172

Merged
merged 11 commits into from
Feb 21, 2024
Merged

match lowering: simplify empty candidate selection #121172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c00b38e
Don't bother to save a block
Nadrieril Feb 16, 2024
b2edcc7
Tweak the function boundary
Nadrieril Feb 16, 2024
a0fa287
Merge the two loops
Nadrieril Feb 16, 2024
a11abc6
Simplify return block computation
Nadrieril Feb 16, 2024
105bdf7
Reuse `next_prebinding`
Nadrieril Feb 16, 2024
b4f0e76
It's fine to assign `otherwise_block`s to unreachable candidates
Nadrieril Feb 16, 2024
0896a49
No need for empty special case anymore
Nadrieril Feb 16, 2024
e561dbb
Move everything into the loop
Nadrieril Feb 16, 2024
998d0e9
Move the loop out of the function
Nadrieril Feb 16, 2024
cd0fc78
Replace the loop with recursive calls for clarity
Nadrieril Feb 16, 2024
a45d892
Upgrade a `debug_assert` to `assert`
Nadrieril Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 62 additions & 103 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,53 +1207,43 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&mut self,
span: Span,
scrutinee_span: Span,
start_block: BasicBlock,
mut start_block: BasicBlock,
otherwise_block: BasicBlock,
candidates: &mut [&mut Candidate<'_, 'tcx>],
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
// The candidates are sorted by priority. Check to see whether the
// higher priority candidates (and hence at the front of the slice)
// have satisfied all their match pairs.
let fully_matched = candidates.iter().take_while(|c| c.match_pairs.is_empty()).count();
debug!("match_candidates: {:?} candidates fully matched", fully_matched);
let (matched_candidates, unmatched_candidates) = candidates.split_at_mut(fully_matched);

let block = if !matched_candidates.is_empty() {
let otherwise_block =
self.select_matched_candidates(matched_candidates, start_block, fake_borrows);

if let Some(last_otherwise_block) = otherwise_block {
last_otherwise_block
} else {
// Any remaining candidates are unreachable.
if unmatched_candidates.is_empty() {
return;
}
self.cfg.start_new_block()
match candidates {
[] => {
// If there are no candidates that still need testing, we're done. Since all matches are
// exhaustive, execution should never reach this point.
let source_info = self.source_info(span);
self.cfg.goto(start_block, source_info, otherwise_block);
}
[first, remaining @ ..] if first.match_pairs.is_empty() => {
// 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, fake_borrows);
self.match_simplified_candidates(
span,
scrutinee_span,
start_block,
otherwise_block,
remaining,
fake_borrows,
)
}
candidates => {
// The first candidate has some unsatisfied match pairs; we proceed to do more tests.
self.test_candidates_with_or(
span,
scrutinee_span,
candidates,
start_block,
otherwise_block,
fake_borrows,
);
}
} else {
start_block
};

// If there are no candidates that still need testing, we're
// done. Since all matches are exhaustive, execution should
// never reach this point.
if unmatched_candidates.is_empty() {
let source_info = self.source_info(span);
self.cfg.goto(block, source_info, otherwise_block);
return;
}

// Test for the remaining candidates.
self.test_candidates_with_or(
span,
scrutinee_span,
unmatched_candidates,
block,
otherwise_block,
fake_borrows,
);
}

/// Link up matched candidates.
Expand All @@ -1275,47 +1265,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// * the [otherwise block] of the third pattern to a block with an
/// [`Unreachable` terminator](TerminatorKind::Unreachable).
///
/// In addition, we add fake edges from the otherwise blocks to the
/// In addition, we later add fake edges from the otherwise blocks to the
/// pre-binding block of the next candidate in the original set of
/// candidates.
///
/// [pre-binding block]: Candidate::pre_binding_block
/// [otherwise block]: Candidate::otherwise_block
fn select_matched_candidates(
fn select_matched_candidate(
&mut self,
matched_candidates: &mut [&mut Candidate<'_, 'tcx>],
candidate: &mut Candidate<'_, 'tcx>,
start_block: BasicBlock,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) -> Option<BasicBlock> {
debug_assert!(
!matched_candidates.is_empty(),
"select_matched_candidates called with no candidates",
);
debug_assert!(
matched_candidates.iter().all(|c| c.subcandidates.is_empty()),
"subcandidates should be empty in select_matched_candidates",
);
) -> BasicBlock {
assert!(candidate.otherwise_block.is_none());
assert!(candidate.pre_binding_block.is_none());
assert!(candidate.subcandidates.is_empty());

// Insert a borrows of prefixes of places that are bound and are
// behind a dereference projection.
//
// These borrows are taken to avoid situations like the following:
//
// match x[10] {
// _ if { x = &[0]; false } => (),
// y => (), // Out of bounds array access!
// }
//
// match *x {
// // y is bound by reference in the guard and then by copy in the
// // arm, so y is 2 in the arm!
// y if { y == 1 && (x = &2) == () } => y,
// _ => 3,
// }
if let Some(fake_borrows) = fake_borrows {
for Binding { source, .. } in
matched_candidates.iter().flat_map(|candidate| &candidate.bindings)
{
// Insert a borrows of prefixes of places that are bound and are
// behind a dereference projection.
//
// These borrows are taken to avoid situations like the following:
//
// match x[10] {
// _ if { x = &[0]; false } => (),
// y => (), // Out of bounds array access!
// }
//
// match *x {
// // y is bound by reference in the guard and then by copy in the
// // arm, so y is 2 in the arm!
// y if { y == 1 && (x = &2) == () } => y,
// _ => 3,
// }
for Binding { source, .. } in &candidate.bindings {
if let Some(i) =
source.projection.iter().rposition(|elem| elem == ProjectionElem::Deref)
{
Expand All @@ -1329,38 +1312,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

let fully_matched_with_guard = matched_candidates
.iter()
.position(|c| !c.has_guard)
.unwrap_or(matched_candidates.len() - 1);

let (reachable_candidates, unreachable_candidates) =
matched_candidates.split_at_mut(fully_matched_with_guard + 1);

let mut next_prebinding = start_block;

for candidate in reachable_candidates.iter_mut() {
assert!(candidate.otherwise_block.is_none());
assert!(candidate.pre_binding_block.is_none());
candidate.pre_binding_block = Some(next_prebinding);
if candidate.has_guard {
// Create the otherwise block for this candidate, which is the
// pre-binding block for the next candidate.
next_prebinding = self.cfg.start_new_block();
candidate.otherwise_block = Some(next_prebinding);
}
}

debug!(
"match_candidates: add pre_binding_blocks for unreachable {:?}",
unreachable_candidates,
);
for candidate in unreachable_candidates {
assert!(candidate.pre_binding_block.is_none());
candidate.pre_binding_block = Some(self.cfg.start_new_block());
candidate.pre_binding_block = Some(start_block);
let otherwise_block = self.cfg.start_new_block();
if candidate.has_guard {
// Create the otherwise block for this candidate, which is the
// pre-binding block for the next candidate.
candidate.otherwise_block = Some(otherwise_block);
}

reachable_candidates.last_mut().unwrap().otherwise_block
otherwise_block
}

/// Tests a candidate where there are only or-patterns left to test, or
Expand Down
20 changes: 14 additions & 6 deletions tests/mir-opt/building/issue_101867.main.built.after.mir
View file
Open in desktop
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: bb6, otherwise: bb4];
}

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

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

bb4: {
goto -> bb7;
goto -> bb9;
}

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

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

bb7: {
goto -> bb4;
}

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

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

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

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

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

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

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

bb5: {
_0 = const ();
goto -> bb11;
goto -> bb3;
}

bb6: {
_2 = const 4_i32;
goto -> bb9;
_0 = const ();
goto -> bb13;
}

bb7: {
unreachable;
goto -> bb3;
}

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

bb9: {
unreachable;
}

bb10: {
goto -> bb11;
}

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

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

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

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