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 block creation #120978

Merged
merged 1 commit into from
Feb 13, 2024
Merged

match lowering: simplify block creation #120978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
132 changes: 61 additions & 71 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 @@ -319,54 +319,52 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// them.
let mut fake_borrows = match_has_guard.then(FxIndexSet::default);

let mut otherwise = None;
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,
&mut otherwise,
otherwise_block,
candidates,
&mut fake_borrows,
);

if let Some(otherwise_block) = otherwise {
// See the doc comment on `match_candidates` for why we may have an
// otherwise block. Match checking will ensure this is actually
// unreachable.
let source_info = self.source_info(scrutinee_span);

// Matching on a `scrutinee_place` with an uninhabited type doesn't
// generate any memory reads by itself, and so if the place "expression"
// contains unsafe operations like raw pointer dereferences or union
// field projections, we wouldn't know to require an `unsafe` block
// around a `match` equivalent to `std::intrinsics::unreachable()`.
// See issue #47412 for this hole being discovered in the wild.
//
// HACK(eddyb) Work around the above issue by adding a dummy inspection
// of `scrutinee_place`, specifically by applying `ReadForMatch`.
//
// NOTE: ReadForMatch also checks that the scrutinee is initialized.
// This is currently needed to not allow matching on an uninitialized,
// uninhabited value. If we get never patterns, those will check that
// the place is initialized, and so this read would only be used to
// check safety.
let cause_matched_place = FakeReadCause::ForMatchedPlace(None);

if let Some(scrutinee_place) = scrutinee_place_builder.try_to_place(self) {
self.cfg.push_fake_read(
otherwise_block,
source_info,
cause_matched_place,
scrutinee_place,
);
}
// See the doc comment on `match_candidates` for why we may have an
// otherwise block. Match checking will ensure this is actually
// unreachable.
let source_info = self.source_info(scrutinee_span);

// Matching on a `scrutinee_place` with an uninhabited type doesn't
// generate any memory reads by itself, and so if the place "expression"
// contains unsafe operations like raw pointer dereferences or union
// field projections, we wouldn't know to require an `unsafe` block
// around a `match` equivalent to `std::intrinsics::unreachable()`.
// See issue #47412 for this hole being discovered in the wild.
//
// HACK(eddyb) Work around the above issue by adding a dummy inspection
// of `scrutinee_place`, specifically by applying `ReadForMatch`.
//
// NOTE: ReadForMatch also checks that the scrutinee is initialized.
// This is currently needed to not allow matching on an uninitialized,
// uninhabited value. If we get never patterns, those will check that
// the place is initialized, and so this read would only be used to
// check safety.
let cause_matched_place = FakeReadCause::ForMatchedPlace(None);

self.cfg.terminate(otherwise_block, source_info, TerminatorKind::Unreachable);
if let Some(scrutinee_place) = scrutinee_place_builder.try_to_place(self) {
self.cfg.push_fake_read(
otherwise_block,
source_info,
cause_matched_place,
scrutinee_place,
);
}

self.cfg.terminate(otherwise_block, source_info, TerminatorKind::Unreachable);

// Link each leaf candidate to the `pre_binding_block` of the next one.
let mut previous_candidate: Option<&mut Candidate<'_, '_>> = None;

Expand Down Expand Up @@ -1163,7 +1161,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: Span,
scrutinee_span: Span,
start_block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
otherwise_block: BasicBlock,
candidates: &mut [&mut Candidate<'pat, 'tcx>],
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
Expand Down Expand Up @@ -1210,7 +1208,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: Span,
scrutinee_span: Span,
start_block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
otherwise_block: BasicBlock,
candidates: &mut [&mut Candidate<'_, 'tcx>],
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
Expand Down Expand Up @@ -1243,11 +1241,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// never reach this point.
if unmatched_candidates.is_empty() {
let source_info = self.source_info(span);
if let Some(otherwise) = *otherwise_block {
self.cfg.goto(block, source_info, otherwise);
} else {
*otherwise_block = Some(block);
}
self.cfg.goto(block, source_info, otherwise_block);
return;
}

Expand Down Expand Up @@ -1428,7 +1422,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
scrutinee_span: Span,
candidates: &mut [&mut Candidate<'_, 'tcx>],
block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
otherwise_block: BasicBlock,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap();
Expand All @@ -1453,7 +1447,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let match_pairs = mem::take(&mut first_candidate.match_pairs);
first_candidate.pre_binding_block = Some(block);

let mut otherwise = None;
let remainder_start = self.cfg.start_new_block();
for match_pair in match_pairs {
let PatKind::Or { ref pats } = &match_pair.pattern.kind else {
bug!("Or-patterns should have been sorted to the end");
Expand All @@ -1463,7 +1457,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
first_candidate.visit_leaves(|leaf_candidate| {
self.test_or_pattern(
leaf_candidate,
&mut otherwise,
remainder_start,
pats,
or_span,
&match_pair.place,
Expand All @@ -1472,8 +1466,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
});
}

let remainder_start = otherwise.unwrap_or_else(|| self.cfg.start_new_block());

self.match_candidates(
span,
scrutinee_span,
Expand All @@ -1491,7 +1483,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn test_or_pattern<'pat>(
&mut self,
candidate: &mut Candidate<'pat, 'tcx>,
otherwise: &mut Option<BasicBlock>,
otherwise: BasicBlock,
pats: &'pat [Box<Pat<'tcx>>],
or_span: Span,
place: &PlaceBuilder<'tcx>,
Expand All @@ -1503,8 +1495,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.map(|pat| Candidate::new(place.clone(), pat, candidate.has_guard, self))
.collect();
let mut or_candidate_refs: Vec<_> = or_candidates.iter_mut().collect();
let otherwise = if candidate.otherwise_block.is_some() {
&mut candidate.otherwise_block
let otherwise = if let Some(otherwise_block) = candidate.otherwise_block {
otherwise_block
} else {
otherwise
};
Expand Down Expand Up @@ -1680,8 +1672,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: Span,
scrutinee_span: Span,
mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>],
block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
start_block: BasicBlock,
otherwise_block: BasicBlock,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
// extract the match-pair from the highest priority candidate
Expand Down Expand Up @@ -1749,12 +1741,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
debug!("untested_candidates: {}", candidates.len());

// The block that we should branch to if none of the
// `target_candidates` match. This is either the block where we
// start matching the untested candidates if there are any,
// otherwise it's the `otherwise_block`.
let remainder_start = &mut None;
let remainder_start =
if candidates.is_empty() { &mut *otherwise_block } else { remainder_start };
// `target_candidates` match.
let remainder_start = if !candidates.is_empty() {
let remainder_start = self.cfg.start_new_block();
self.match_candidates(
span,
scrutinee_span,
remainder_start,
otherwise_block,
candidates,
fake_borrows,
);
remainder_start
} else {
otherwise_block
};

// For each outcome of test, process the candidates that still
// apply. Collect a list of blocks where control flow will
Expand All @@ -1775,24 +1776,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
);
candidate_start
} else {
*remainder_start.get_or_insert_with(|| self.cfg.start_new_block())
remainder_start
}
})
.collect();

if !candidates.is_empty() {
let remainder_start = remainder_start.unwrap_or_else(|| self.cfg.start_new_block());
self.match_candidates(
span,
scrutinee_span,
remainder_start,
otherwise_block,
candidates,
fake_borrows,
);
}

self.perform_test(span, scrutinee_span, block, &match_place, &test, target_blocks);
// Perform the test, branching to one of N blocks.
self.perform_test(span, scrutinee_span, start_block, &match_place, &test, target_blocks);
}

/// Determine the fake borrows that are needed from a set of places that
Expand Down
14 changes: 7 additions & 7 deletions tests/mir-opt/building/async_await.b-{closure#0}.coroutine_resume.0.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,

bb0: {
_39 = discriminant((*(_1.0: &mut {async fn body@$DIR/async_await.rs:15:18: 18:2})));
switchInt(move _39) -> [0: bb1, 1: bb29, 3: bb27, 4: bb28, otherwise: bb9];
switchInt(move _39) -> [0: bb1, 1: bb29, 3: bb27, 4: bb28, otherwise: bb8];
}

bb1: {
Expand Down Expand Up @@ -165,10 +165,14 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
StorageDead(_10);
PlaceMention(_9);
_16 = discriminant(_9);
switchInt(move _16) -> [0: bb10, 1: bb8, otherwise: bb9];
switchInt(move _16) -> [0: bb10, 1: bb9, otherwise: bb8];
}

bb8: {
unreachable;
}

bb9: {
_8 = const ();
StorageDead(_14);
StorageDead(_12);
Expand All @@ -186,10 +190,6 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
return;
}

bb9: {
unreachable;
}

bb10: {
StorageLive(_17);
_17 = ((_9 as Ready).0: ());
Expand Down Expand Up @@ -267,7 +267,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
StorageDead(_26);
PlaceMention(_25);
_32 = discriminant(_25);
switchInt(move _32) -> [0: bb21, 1: bb20, otherwise: bb9];
switchInt(move _32) -> [0: bb21, 1: bb20, otherwise: bb8];
}

bb20: {
Expand Down
17 changes: 11 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: bb4, otherwise: bb3];
switchInt(move _6) -> [1: bb5, otherwise: bb4];
}

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

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

bb3: {
goto -> bb6;
FakeRead(ForMatchedPlace(None), _1);
unreachable;
}

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

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

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

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

bb7 (cleanup): {
bb8 (cleanup): {
resume;
}
}
Loading
Loading