From 9d34c280d92e1bc443c9ae10744e5b73a71e8915 Mon Sep 17 00:00:00 2001 From: Scott A Carr Date: Tue, 31 May 2016 17:08:00 -0700 Subject: [PATCH] generate fewer basic blocks for variant switches --- src/librustc_mir/build/matches/mod.rs | 16 +++++++- src/librustc_mir/build/matches/test.rs | 57 ++++++++++++++++++++++---- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index c1a0e1f9a69..bdaaafc1d8f 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -266,6 +266,7 @@ enum TestKind<'tcx> { // test the branches of enum Switch { adt_def: AdtDef<'tcx>, + variants: Vec, }, // test the branches of enum @@ -391,9 +392,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { fn join_otherwise_blocks(&mut self, span: Span, - otherwise: Vec) + mut otherwise: Vec) -> BasicBlock { + otherwise.sort(); + otherwise.dedup(); // variant switches can introduce duplicate target blocks let scope_id = self.innermost_scope_id(); if otherwise.len() == 1 { otherwise[0] @@ -502,6 +505,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } } + TestKind::Switch { adt_def: _, ref mut variants} => { + for candidate in candidates.iter() { + if !self.add_variants_to_switch(&match_pair.lvalue, + candidate, + variants) { + break; + } + } + } _ => { } } @@ -525,6 +537,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { &mut target_candidates)) .count(); assert!(tested_candidates > 0); // at least the last candidate ought to be tested + debug!("tested_candidates: {}", tested_candidates); + debug!("untested_candidates: {}", candidates.len() - tested_candidates); // For each outcome of test, process the candidates that still // apply. Collect a list of blocks where control flow will diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index e53584a3f8b..307c8ecd832 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -33,7 +33,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { PatternKind::Variant { ref adt_def, variant_index: _, subpatterns: _ } => { Test { span: match_pair.pattern.span, - kind: TestKind::Switch { adt_def: adt_def.clone() }, + kind: TestKind::Switch { + adt_def: adt_def.clone(), + variants: vec![false; self.hir.num_variants(adt_def)], + }, } } @@ -125,9 +128,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }); true } - + PatternKind::Variant { .. } => { + panic!("you should have called add_cases_to_switch_switch instead!"); + } PatternKind::Range { .. } | - PatternKind::Variant { .. } | PatternKind::Slice { .. } | PatternKind::Array { .. } | PatternKind::Wild | @@ -140,6 +144,30 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } + pub fn add_variants_to_switch<'pat>(&mut self, + test_lvalue: &Lvalue<'tcx>, + candidate: &Candidate<'pat, 'tcx>, + variants: &mut Vec) + -> bool + { + let match_pair = match candidate.match_pairs.iter().find(|mp| mp.lvalue == *test_lvalue) { + Some(match_pair) => match_pair, + _ => { return false; } + }; + + match *match_pair.pattern.kind { + PatternKind::Variant { adt_def: _ , variant_index, .. } => { + // Do I need to look at the PatternKind::Variant subpatterns? + variants[variant_index] |= true; + true + } + _ => { + // don't know how to add these patterns to a switch + false + } + } + } + /// Generates the code to perform a test. pub fn perform_test(&mut self, block: BasicBlock, @@ -148,11 +176,26 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { -> Vec { let scope_id = self.innermost_scope_id(); match test.kind { - TestKind::Switch { adt_def } => { + TestKind::Switch { adt_def, ref variants } => { let num_enum_variants = self.hir.num_variants(adt_def); - let target_blocks: Vec<_> = + debug!("num_enum_variants: {}", num_enum_variants); + debug!("variants.len(): {}", variants.len()); + debug!("variants: {:?}", variants); + let target_blocks: Vec<_> = if variants.into_iter().any(|b| {!b}) { + let otherwise_block = self.cfg.start_new_block(); + debug!("basic block: {:?} is an otherwise block!", otherwise_block); + (0..num_enum_variants).map(|i| + if variants[i] { + self.cfg.start_new_block() + } else { + otherwise_block + } + ) + .collect() + } else { (0..num_enum_variants).map(|_| self.cfg.start_new_block()) - .collect(); + .collect() + }; self.cfg.terminate(block, scope_id, test.span, TerminatorKind::Switch { discr: lvalue.clone(), adt_def: adt_def, @@ -383,7 +426,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { match test.kind { // If we are performing a variant switch, then this // informs variant patterns, but nothing else. - TestKind::Switch { adt_def: tested_adt_def } => { + TestKind::Switch { adt_def: tested_adt_def , .. } => { match *match_pair.pattern.kind { PatternKind::Variant { adt_def, variant_index, ref subpatterns } => { assert_eq!(adt_def, tested_adt_def);