Skip to content

Commit

Permalink
Avoid unnecessary false edges in MIR match lowering
Browse files Browse the repository at this point in the history
  • Loading branch information
@matthewjasper
matthewjasper committed Jun 12, 2019
1 parent 33bc396 commit 32c3377
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 53 deletions.
118 changes: 65 additions & 53 deletions src/librustc_mir/build/matches/mod.rs
View file
Expand Up @@ -143,19 +143,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {

// create binding start block for link them by false edges
let candidate_count = arms.iter().map(|c| c.patterns.len()).sum::<usize>();
let pre_binding_blocks: Vec<_> = (0..=candidate_count)
let pre_binding_blocks: Vec<_> = (0..candidate_count)
.map(|_| self.cfg.start_new_block())
.collect();

// There's one more pre_binding block than there are candidates so that
// every candidate can have a `next_candidate_pre_binding_block`.
let outer_source_info = self.source_info(span);
self.cfg.terminate(
*pre_binding_blocks.last().unwrap(),
outer_source_info,
TerminatorKind::Unreachable,
);

let mut match_has_guard = false;

let mut candidate_pre_binding_blocks = pre_binding_blocks.iter();
Expand All @@ -171,9 +162,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let arm_candidates: Vec<_> = arm.patterns
.iter()
.zip(candidate_pre_binding_blocks.by_ref())
.zip(next_candidate_pre_binding_blocks.by_ref())
.map(
|((pattern, pre_binding_block), next_candidate_pre_binding_block)| {
|(pattern, pre_binding_block)| {
Candidate {
span: pattern.span,
match_pairs: vec![
Expand All @@ -188,7 +178,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
},
pre_binding_block: *pre_binding_block,
next_candidate_pre_binding_block:
*next_candidate_pre_binding_block,
next_candidate_pre_binding_blocks.next().copied(),
}
},
)
Expand Down Expand Up @@ -225,6 +215,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
&mut fake_borrows,
);

let outer_source_info = self.source_info(span);

if !otherwise.is_empty() {
// All matches are exhaustive. However, because some matches
// only have exponentially-large exhaustive decision trees, we
Expand All @@ -251,12 +243,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
};

// Step 5. Create everything else: the guards and the arms.

let arm_end_blocks: Vec<_> = arm_candidates.into_iter().map(|(arm, candidates)| {
let arm_end_blocks: Vec<_> = arm_candidates.into_iter().map(|(arm, mut candidates)| {
let arm_source_info = self.source_info(arm.span);
let region_scope = (arm.scope, arm_source_info);
self.in_scope(region_scope, arm.lint_level, |this| {
let arm_block = this.cfg.start_new_block();
let mut arm_block = this.cfg.start_new_block();

let body = this.hir.mirror(arm.body.clone());
let scope = this.declare_bindings(
Expand All @@ -267,6 +258,30 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
Some((Some(&scrutinee_place), scrutinee_span)),
);

if candidates.len() == 1 {
arm_block = self.bind_and_guard_matched_candidate(
candidates.pop().unwrap(),
arm.guard.clone(),
&fake_borrow_temps,
scrutinee_span,
);
} else {
arm_block = self.cfg.start_new_block();
for candidate in candidates {
let binding_end = self.bind_and_guard_matched_candidate(
candidate,
arm.guard.clone(),
&fake_borrow_temps,
scrutinee_span,
);
self.cfg.terminate(
binding_end,
source_info,
TerminatorKind::Goto { target: arm_block },
);
}
}

if let Some(source_scope) = scope {
this.source_scope = source_scope;
}
Expand Down Expand Up @@ -434,7 +449,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// since we don't call `match_candidates`, next fields are unused
otherwise_block: None,
pre_binding_block: block,
next_candidate_pre_binding_block: block,
next_candidate_pre_binding_block: None,
};

// Simplify the candidate. Since the pattern is irrefutable, this should
Expand Down Expand Up @@ -689,7 +704,7 @@ pub struct Candidate<'pat, 'tcx: 'pat> {

// ...and the blocks for add false edges between candidates
pre_binding_block: BasicBlock,
next_candidate_pre_binding_block: BasicBlock,
next_candidate_pre_binding_block: Option<BasicBlock>,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -956,14 +971,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
if let [first_candidate, second_candidate] = window {
let source_info = self.source_info(first_candidate.span);
if let Some(otherwise_block) = first_candidate.otherwise_block {
self.cfg.terminate(
self.false_edges(
otherwise_block,
second_candidate.pre_binding_block,
first_candidate.next_candidate_pre_binding_block,
source_info,
TerminatorKind::FalseEdges {
real_target: second_candidate.pre_binding_block,
imaginary_target: first_candidate.next_candidate_pre_binding_block,
}
)
);
} else {
bug!("candidate other than the last has no guard");
}
Expand All @@ -977,13 +990,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
if let Some(otherwise) = candidate.otherwise_block {
let source_info = self.source_info(candidate.span);
let unreachable = self.cfg.start_new_block();
self.cfg.terminate(
self.false_edges(
otherwise,
unreachable,
candidate.next_candidate_pre_binding_block,
source_info,
TerminatorKind::FalseEdges {
real_target: unreachable,
imaginary_targets: candidate.next_candidate_pre_binding_block,
}
);
self.cfg.terminate(unreachable, source_info, TerminatorKind::Unreachable);
}
Expand All @@ -994,13 +1005,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
if let Some(otherwise) = last_candidate.otherwise_block {
let source_info = self.source_info(last_candidate.span);
let block = self.cfg.start_new_block();
self.cfg.terminate(
self.false_edges(
otherwise,
block,
last_candidate.next_candidate_pre_binding_block,
source_info,
TerminatorKind::FalseEdges {
real_target: block,
imaginary_target: last_candidate.next_candidate_pre_binding_block,
}
);
Some(block)
} else {
Expand Down Expand Up @@ -1311,27 +1320,38 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
&mut self,
candidate: Candidate<'pat, 'tcx>,
guard: Option<Guard<'tcx>>,
arm_block: BasicBlock,
fake_borrows: &Vec<(&Place<'tcx>, Local)>,
scrutinee_span: Span,
region_scope: (region::Scope, SourceInfo),
) {
) -> BasicBlock {
debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate);

debug_assert!(candidate.match_pairs.is_empty());

let candidate_source_info = self.source_info(candidate.span);

let mut block = self.cfg.start_new_block();
self.cfg.terminate(
candidate.pre_binding_block,
let mut block = candidate.pre_binding_block;

// If we are adding our own statements, then we need a fresh block.
let create_fresh_block = candidate.next_candidate_pre_binding_block.is_some()
|| !candidate.bindings.is_empty()
|| !candidate.ascriptions.is_empty()
|| guard.is_some();

if create_fresh_block {
let fresh_block = self.cfg.start_new_block();
self.false_edges(
block,
fresh_block,
candidate.next_candidate_pre_binding_block,
candidate_source_info,
TerminatorKind::FalseEdges {
real_target: block,
imaginary_target: candidate.next_candidate_pre_binding_block,
},
);
block = fresh_block;
self.ascribe_types(block, &candidate.ascriptions);
} else {
return block;
}

// rust-lang/rust#27282: The `autoref` business deserves some
// explanation here.
Expand Down Expand Up @@ -1476,7 +1496,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// because that would be before we've checked the result
// from the guard.
//
// But binding them on `arm_block` is *too late*, because
// But binding them on the arm is *too late*, because
// then all of the candidates for a single arm would be
// bound in the same place, that would cause a case like:
//
Expand Down Expand Up @@ -1552,22 +1572,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
by_value_bindings,
);

self.cfg.terminate(
post_guard_block,
source_info,
TerminatorKind::Goto { target: arm_block },
);
post_guard_block
} else {
assert!(candidate.otherwise_block.is_none());
// (Here, it is not too early to bind the matched
// candidate on `block`, because there is no guard result
// that we have to inspect before we bind them.)
self.bind_matched_candidate_for_arm_body(block, &candidate.bindings);
self.cfg.terminate(
block,
candidate_source_info,
TerminatorKind::Goto { target: arm_block },
);
block
}
}

Expand Down
33 changes: 33 additions & 0 deletions src/librustc_mir/build/matches/util.rs
View file
Expand Up @@ -65,6 +65,39 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
})
);
}

/// Creates a false edge to `imaginary_target` and a real edge to
/// real_target. If `imaginary_target` is none, or is the same as the real
/// target, a Goto is generated instead to simplify the generated MIR.
pub fn false_edges(
&mut self,
from_block: BasicBlock,
real_target: BasicBlock,
imaginary_target: Option<BasicBlock>,
source_info: SourceInfo,
) {
match imaginary_target {
Some(target) if target != real_target => {
self.cfg.terminate(
from_block,
source_info,
TerminatorKind::FalseEdges {
real_target,
imaginary_target: target,
},
);
}
_ => {
self.cfg.terminate(
from_block,
source_info,
TerminatorKind::Goto {
target: real_target
}
);
}
}
}
}

impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
Expand Down

0 comments on commit 32c3377

Please sign in to comment.