From 32c337724dd3a1b651cb7ba9769e60262f9f52f8 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 6 May 2019 16:13:20 +0100 Subject: [PATCH] Avoid unnecessary false edges in MIR match lowering --- src/librustc_mir/build/matches/mod.rs | 118 ++++++++++++++----------- src/librustc_mir/build/matches/util.rs | 33 +++++++ 2 files changed, 98 insertions(+), 53 deletions(-) diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 353842730c7e7..38c9a177c388e 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -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::(); - 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(); @@ -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![ @@ -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(), } }, ) @@ -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 @@ -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( @@ -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; } @@ -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 @@ -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, } #[derive(Clone, Debug)] @@ -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"); } @@ -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); } @@ -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 { @@ -1311,27 +1320,38 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { &mut self, candidate: Candidate<'pat, 'tcx>, guard: Option>, - 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. @@ -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: // @@ -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 } } diff --git a/src/librustc_mir/build/matches/util.rs b/src/librustc_mir/build/matches/util.rs index 3b90ff7884f01..04ae5a87e49e6 100644 --- a/src/librustc_mir/build/matches/util.rs +++ b/src/librustc_mir/build/matches/util.rs @@ -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, + 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> {