From 4f0dc7b06c72d4b1f5caf2c40c8f7407cdb9c5ab Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 20 Dec 2019 18:05:45 +0100 Subject: [PATCH 1/2] misc cleanup in match MIR building --- src/librustc_mir/build/matches/mod.rs | 42 ++++++++++++-------------- src/librustc_mir/build/matches/test.rs | 2 +- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index bf0b2439c00b5..c0a7439b5ce57 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -26,6 +26,7 @@ mod simplify; mod test; mod util; +use itertools::Itertools; use std::convert::TryFrom; impl<'a, 'tcx> Builder<'a, 'tcx> { @@ -822,9 +823,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); let (matched_candidates, unmatched_candidates) = candidates.split_at_mut(fully_matched); - let block: BasicBlock; - - if !matched_candidates.is_empty() { + let block: BasicBlock = if !matched_candidates.is_empty() { let otherwise_block = self.select_matched_candidates( matched_candidates, start_block, @@ -832,17 +831,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); if let Some(last_otherwise_block) = otherwise_block { - block = last_otherwise_block + last_otherwise_block } else { // Any remaining candidates are unreachable. if unmatched_candidates.is_empty() { return; } - block = self.cfg.start_new_block(); - }; + self.cfg.start_new_block() + } } else { - block = *start_block.get_or_insert_with(|| self.cfg.start_new_block()); - } + *start_block.get_or_insert_with(|| self.cfg.start_new_block()) + }; // If there are no candidates that still need testing, we're // done. Since all matches are exhaustive, execution should @@ -885,7 +884,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// ... /// /// We generate real edges from: - /// * `block` to the prebinding_block of the first pattern, + /// * `start_block` to the `prebinding_block` of the first pattern, /// * the otherwise block of the first pattern to the second pattern, /// * the otherwise block of the third pattern to the a block with an /// Unreachable terminator. @@ -948,6 +947,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let first_candidate = &reachable_candidates[0]; let first_prebinding_block = first_candidate.pre_binding_block; + // `goto -> first_prebinding_block` from the `start_block` if there is one. if let Some(start_block) = *start_block { let source_info = self.source_info(first_candidate.span); self.cfg.terminate( @@ -959,21 +959,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { *start_block = Some(first_prebinding_block); } - for window in reachable_candidates.windows(2) { - 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.false_edges( - otherwise_block, - second_candidate.pre_binding_block, - first_candidate.next_candidate_pre_binding_block, - source_info, - ); - } else { - bug!("candidate other than the last has no guard"); - } + for (first_candidate, second_candidate) in reachable_candidates.iter().tuple_windows() { + let source_info = self.source_info(first_candidate.span); + if let Some(otherwise_block) = first_candidate.otherwise_block { + self.false_edges( + otherwise_block, + second_candidate.pre_binding_block, + first_candidate.next_candidate_pre_binding_block, + source_info, + ); } else { - bug!("<[_]>::windows returned incorrectly sized window"); + bug!("candidate other than the last has no guard"); } } diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index e320811ca0556..bdc1bdd5b9855 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -30,7 +30,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Test { span: match_pair.pattern.span, kind: TestKind::Switch { - adt_def: adt_def.clone(), + adt_def, variants: BitSet::new_empty(adt_def.variants.len()), }, } From f5a8d1ab034c48b69eef8ee3618f7eb97d72810f Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 20 Dec 2019 18:27:05 +0100 Subject: [PATCH 2/2] simplify MIR building with cfg.goto(...) --- src/librustc_mir/build/block.rs | 3 +-- src/librustc_mir/build/cfg.rs | 5 ++++ src/librustc_mir/build/expr/into.rs | 28 +++++---------------- src/librustc_mir/build/matches/mod.rs | 35 ++++++-------------------- src/librustc_mir/build/matches/util.rs | 10 +------- src/librustc_mir/build/mod.rs | 9 +++---- src/librustc_mir/build/scope.rs | 18 +++++-------- 7 files changed, 29 insertions(+), 79 deletions(-) diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 7353ca9285ddb..aceed09757e7e 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -33,8 +33,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode) }); - this.cfg.terminate(unpack!(block_exit), source_info, - TerminatorKind::Goto { target: exit_block }); + this.cfg.goto(unpack!(block_exit), source_info, exit_block); exit_block.unit() } else { this.ast_block_stmts(destination, block, span, stmts, expr, diff --git a/src/librustc_mir/build/cfg.rs b/src/librustc_mir/build/cfg.rs index 6bd8d2f7c0792..0e685486c3f99 100644 --- a/src/librustc_mir/build/cfg.rs +++ b/src/librustc_mir/build/cfg.rs @@ -85,4 +85,9 @@ impl<'tcx> CFG<'tcx> { kind, }); } + + /// In the `origin` block, push a `goto -> target` terminator. + pub fn goto(&mut self, origin: BasicBlock, source_info: SourceInfo, target: BasicBlock) { + self.terminate(origin, source_info, TerminatorKind::Goto { target }) + } } diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 608415408e35c..a9fa7cfa04a89 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -140,17 +140,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, ); - this.cfg.terminate( - true_block, - source_info, - TerminatorKind::Goto { target: join_block }, - ); - this.cfg.terminate( - false_block, - source_info, - TerminatorKind::Goto { target: join_block }, - ); - + // Link up both branches: + this.cfg.goto(true_block, source_info, join_block); + this.cfg.goto(false_block, source_info, join_block); join_block.unit() } ExprKind::Loop { body } => { @@ -167,12 +159,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let loop_block = this.cfg.start_new_block(); let exit_block = this.cfg.start_new_block(); - // start the loop - this.cfg.terminate( - block, - source_info, - TerminatorKind::Goto { target: loop_block }, - ); + // Start the loop. + this.cfg.goto(block, source_info, loop_block); this.in_breakable_scope( Some(loop_block), @@ -196,11 +184,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let tmp = this.get_unit_temp(); // Execute the body, branching back to the test. let body_block_end = unpack!(this.into(&tmp, body_block, body)); - this.cfg.terminate( - body_block_end, - source_info, - TerminatorKind::Goto { target: loop_block }, - ); + this.cfg.goto(body_block_end, source_info, loop_block); }, ); exit_block.unit() diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index c0a7439b5ce57..6869930509cbe 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -259,11 +259,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scrutinee_span, match_scope, ); - this.cfg.terminate( - binding_end, - source_info, - TerminatorKind::Goto { target: arm_block }, - ); + this.cfg.goto(binding_end, source_info, arm_block); } } @@ -279,11 +275,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let end_block = self.cfg.start_new_block(); for arm_block in arm_end_blocks { - self.cfg.terminate( - unpack!(arm_block), - outer_source_info, - TerminatorKind::Goto { target: end_block }, - ); + self.cfg.goto(unpack!(arm_block), outer_source_info, end_block); } self.source_scope = outer_source_info.scope; @@ -848,18 +840,9 @@ 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.terminate( - block, - source_info, - TerminatorKind::Goto { target: otherwise }, - ); - } else { - self.cfg.terminate( - block, - source_info, - TerminatorKind::Unreachable, - ) + match otherwise_block { + Some(otherwise) => self.cfg.goto(block, source_info, otherwise), + None => self.cfg.terminate(block, source_info, TerminatorKind::Unreachable), } return; } @@ -950,11 +933,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // `goto -> first_prebinding_block` from the `start_block` if there is one. if let Some(start_block) = *start_block { let source_info = self.source_info(first_candidate.span); - self.cfg.terminate( - start_block, - source_info, - TerminatorKind::Goto { target: first_prebinding_block }, - ); + self.cfg.goto(start_block, source_info, first_prebinding_block); } else { *start_block = Some(first_prebinding_block); } @@ -988,8 +967,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - let last_candidate = reachable_candidates.last().unwrap(); + let last_candidate = reachable_candidates.last().unwrap(); if let Some(otherwise) = last_candidate.otherwise_block { let source_info = self.source_info(last_candidate.span); let block = self.cfg.start_new_block(); diff --git a/src/librustc_mir/build/matches/util.rs b/src/librustc_mir/build/matches/util.rs index ec8b3c5e24bf2..87481d1d69bc7 100644 --- a/src/librustc_mir/build/matches/util.rs +++ b/src/librustc_mir/build/matches/util.rs @@ -109,15 +109,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, ); } - _ => { - self.cfg.terminate( - from_block, - source_info, - TerminatorKind::Goto { - target: real_target - } - ); - } + _ => self.cfg.goto(from_block, source_info, real_target), } } } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 3479ad6749a90..1ecae105694ed 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -606,14 +606,11 @@ where let fn_end = span.shrink_to_hi(); let source_info = builder.source_info(fn_end); let return_block = builder.return_block(); - builder.cfg.terminate(block, source_info, - TerminatorKind::Goto { target: return_block }); - builder.cfg.terminate(return_block, source_info, - TerminatorKind::Return); + builder.cfg.goto(block, source_info, return_block); + builder.cfg.terminate(return_block, source_info, TerminatorKind::Return); // Attribute any unreachable codepaths to the function's closing brace if let Some(unreachable_block) = builder.cached_unreachable_block { - builder.cfg.terminate(unreachable_block, source_info, - TerminatorKind::Unreachable); + builder.cfg.terminate(unreachable_block, source_info, TerminatorKind::Unreachable); } return_block.unit() })); diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 00a30af806a89..9c5966263dfc0 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -564,14 +564,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let source_info = scope.source_info(span); block = match scope.cached_exits.entry((target, region_scope)) { Entry::Occupied(e) => { - self.cfg.terminate(block, source_info, - TerminatorKind::Goto { target: *e.get() }); + self.cfg.goto(block, source_info, *e.get()); return; } Entry::Vacant(v) => { let b = self.cfg.start_new_block(); - self.cfg.terminate(block, source_info, - TerminatorKind::Goto { target: b }); + self.cfg.goto(block, source_info, b); v.insert(b); b } @@ -596,8 +594,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scope = next_scope; } - let source_info = self.scopes.source_info(scope_count, span); - self.cfg.terminate(block, source_info, TerminatorKind::Goto { target }); + self.cfg.goto(block, self.scopes.source_info(scope_count, span), target); } /// Creates a path that performs all required cleanup for dropping a generator. @@ -616,14 +613,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { while let Some(scope) = scopes.next() { block = if let Some(b) = scope.cached_generator_drop { - self.cfg.terminate(block, src_info, - TerminatorKind::Goto { target: b }); + self.cfg.goto(block, src_info, b); return Some(result); } else { let b = self.cfg.start_new_block(); scope.cached_generator_drop = Some(b); - self.cfg.terminate(block, src_info, - TerminatorKind::Goto { target: b }); + self.cfg.goto(block, src_info, b); b }; @@ -1243,8 +1238,7 @@ fn build_diverge_scope<'tcx>(cfg: &mut CFG<'tcx>, // block for our StorageDead statements. let block = cfg.start_new_cleanup_block(); let source_info = SourceInfo { span: DUMMY_SP, scope: source_scope }; - cfg.terminate(block, source_info, - TerminatorKind::Goto { target: target }); + cfg.goto(block, source_info, target); target = block; target_built_by_us = true; }