OpenE2K
/
rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #29588 - nikomatsakis:mir-switch, r=aatch 

Introduce a `SwitchInt` and restructure pattern matching to collect integers and characters into one master switch. This is aimed at #29227, but is not a complete fix. Whereas before we generated an if-else-if chain and, at least on my machine, just failed to compile, we now spend ~9sec compiling `rustc_abuse`. AFAICT this is basically just due to a need for more micro-optimization of the matching process: perf shows a fair amount of time just spent iterating over the candidate list. Still, it seemed worth opening a PR with this step alone, since it's a big step forward.
This commit is contained in:
bors 2015-11-06 06:13:59 +00:00
parent 7512808ac9 dcf323e586
commit 98fa2ac1bc
6 changed files with 319 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/librustc/middle/const_eval.rs
View File

@ -39,6 +39,7 @@ use std::borrow::{Cow, IntoCow};
use std::num::wrapping::OverflowingOps;
use std::cmp::Ordering;
use std::collections::hash_map::Entry::Vacant;
use std::hash;
use std::mem::transmute;
use std::{i8, i16, i32, i64, u8, u16, u32, u64};
use std::rc::Rc;
@ -257,6 +258,22 @@ pub enum ConstVal {
Function(DefId),
}
impl hash::Hash for ConstVal {
fn hash<H: hash::Hasher>(&self, state: &mut H) {
match *self {
Float(a) => unsafe { transmute::<_,u64>(a) }.hash(state),
Int(a) => a.hash(state),
Uint(a) => a.hash(state),
Str(ref a) => a.hash(state),
ByteStr(ref a) => a.hash(state),
Bool(a) => a.hash(state),
Struct(a) => a.hash(state),
Tuple(a) => a.hash(state),
Function(a) => a.hash(state),
}
}
}
/// Note that equality for `ConstVal` means that the it is the same
/// constant, not that the rust values are equal. In particular, `NaN
/// == NaN` (at least if it's the same NaN; distinct encodings for NaN
@ -278,6 +295,8 @@ impl PartialEq for ConstVal {
}
}
impl Eq for ConstVal { }
impl ConstVal {
pub fn description(&self) -> &'static str {
match *self {

41
src/librustc_mir/build/matches/mod.rs
View File

@ -15,6 +15,8 @@
use build::{BlockAnd, Builder};
use repr::*;
use rustc_data_structures::fnv::FnvHashMap;
use rustc::middle::const_eval::ConstVal;
use rustc::middle::region::CodeExtent;
use rustc::middle::ty::{AdtDef, Ty};
use hair::*;
@ -241,6 +243,13 @@ enum TestKind<'tcx> {
adt_def: AdtDef<'tcx>,
},
// test the branches of enum
SwitchInt {
switch_ty: Ty<'tcx>,
options: Vec<ConstVal>,
indices: FnvHashMap<ConstVal, usize>,
},
// test for equality
Eq {
value: Literal<'tcx>,
@ -315,20 +324,36 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// otherwise, extract the next match pair and construct tests
let match_pair = &candidates.last().unwrap().match_pairs[0];
let test = self.test(match_pair);
let mut test = self.test(match_pair);
// most of the time, the test to perform is simply a function
// of the main candidate; but for a test like SwitchInt, we
// may want to add cases based on the candidates that are
// available
match test.kind {
TestKind::SwitchInt { switch_ty, ref mut options, ref mut indices } => {
for candidate in &candidates {
self.add_cases_to_switch(&match_pair.lvalue,
candidate,
switch_ty,
options,
indices);
}
}
_ => { }
}
debug!("match_candidates: test={:?} match_pair={:?}", test, match_pair);
let target_blocks = self.perform_test(block, &match_pair.lvalue, &test);
for (outcome, mut target_block) in target_blocks.into_iter().enumerate() {
for (outcome, target_block) in target_blocks.into_iter().enumerate() {
let applicable_candidates: Vec<Candidate<'tcx>> =
candidates.iter()
.filter_map(|candidate| {
unpack!(target_block =
self.candidate_under_assumption(target_block,
&match_pair.lvalue,
&test.kind,
outcome,
candidate))
self.candidate_under_assumption(&match_pair.lvalue,
&test.kind,
outcome,
candidate)
})
.collect();
self.match_candidates(span, arm_blocks, applicable_candidates, target_block);

297
src/librustc_mir/build/matches/test.rs
View File

@ -15,10 +15,13 @@
// identify what tests are needed, perform the tests, and then filter
// the candidates based on the result.
use build::{BlockAnd, Builder};
use build::Builder;
use build::matches::{Candidate, MatchPair, Test, TestKind};
use hair::*;
use repr::*;
use rustc_data_structures::fnv::FnvHashMap;
use rustc::middle::const_eval::ConstVal;
use rustc::middle::ty::Ty;
use syntax::codemap::Span;
impl<'a,'tcx> Builder<'a,'tcx> {
@ -34,13 +37,31 @@ impl<'a,'tcx> Builder<'a,'tcx> {
}
}
PatternKind::Constant { value: Literal::Value { .. } }
if is_switch_ty(match_pair.pattern.ty) => {
// for integers, we use a SwitchInt match, which allows
// us to handle more cases
Test {
span: match_pair.pattern.span,
kind: TestKind::SwitchInt {
switch_ty: match_pair.pattern.ty,
// these maps are empty to start; cases are
// added below in add_cases_to_switch
options: vec![],
indices: FnvHashMap(),
}
}
}
PatternKind::Constant { ref value } => {
// for other types, we use an equality comparison
Test {
span: match_pair.pattern.span,
kind: TestKind::Eq {
value: value.clone(),
ty: match_pair.pattern.ty.clone(),
},
}
}
}
@ -78,13 +99,52 @@ impl<'a,'tcx> Builder<'a,'tcx> {
}
}
pub fn add_cases_to_switch(&mut self,
test_lvalue: &Lvalue<'tcx>,
candidate: &Candidate<'tcx>,
switch_ty: Ty<'tcx>,
options: &mut Vec<ConstVal>,
indices: &mut FnvHashMap<ConstVal, usize>)
{
let match_pair = match candidate.match_pairs.iter().find(|mp| mp.lvalue == *test_lvalue) {
Some(match_pair) => match_pair,
_ => { return; }
};
match match_pair.pattern.kind {
PatternKind::Constant { value: Literal::Value { ref value } } => {
// if the lvalues match, the type should match
assert_eq!(match_pair.pattern.ty, switch_ty);
indices.entry(value.clone())
.or_insert_with(|| {
options.push(value.clone());
options.len() - 1
});
}
PatternKind::Range { .. } => {
}
PatternKind::Constant { .. } |
PatternKind::Variant { .. } |
PatternKind::Slice { .. } |
PatternKind::Array { .. } |
PatternKind::Wild |
PatternKind::Binding { .. } |
PatternKind::Leaf { .. } |
PatternKind::Deref { .. } => {
}
}
}
/// Generates the code to perform a test.
pub fn perform_test(&mut self,
block: BasicBlock,
lvalue: &Lvalue<'tcx>,
test: &Test<'tcx>)
-> Vec<BasicBlock> {
match test.kind.clone() {
match test.kind {
TestKind::Switch { adt_def } => {
let num_enum_variants = self.hir.num_variants(adt_def);
let target_blocks: Vec<_> =
@ -98,18 +158,34 @@ impl<'a,'tcx> Builder<'a,'tcx> {
target_blocks
}
TestKind::Eq { value, ty } => {
TestKind::SwitchInt { switch_ty, ref options, indices: _ } => {
let otherwise = self.cfg.start_new_block();
let targets: Vec<_> =
options.iter()
.map(|_| self.cfg.start_new_block())
.chain(Some(otherwise))
.collect();
self.cfg.terminate(block, Terminator::SwitchInt {
discr: lvalue.clone(),
switch_ty: switch_ty,
values: options.clone(),
targets: targets.clone(),
});
targets
}
TestKind::Eq { ref value, ty } => {
// call PartialEq::eq(discrim, constant)
let constant = self.literal_operand(test.span, ty.clone(), value);
let constant = self.literal_operand(test.span, ty.clone(), value.clone());
let item_ref = self.hir.partial_eq(ty);
self.call_comparison_fn(block, test.span, item_ref,
Operand::Consume(lvalue.clone()), constant)
}
TestKind::Range { lo, hi, ty } => {
TestKind::Range { ref lo, ref hi, ty } => {
// Test `v` by computing `PartialOrd::le(lo, v) && PartialOrd::le(v, hi)`.
let lo = self.literal_operand(test.span, ty.clone(), lo);
let hi = self.literal_operand(test.span, ty.clone(), hi);
let lo = self.literal_operand(test.span, ty.clone(), lo.clone());
let hi = self.literal_operand(test.span, ty.clone(), hi.clone());
let item_ref = self.hir.partial_le(ty);
let lo_blocks = self.call_comparison_fn(block,
@ -207,34 +283,31 @@ impl<'a,'tcx> Builder<'a,'tcx> {
/// were `Ok`, we would return `Some([x.0.downcast<Ok>.0 @ P1, x.1
/// @ 22])`.
pub fn candidate_under_assumption(&mut self,
mut block: BasicBlock,
test_lvalue: &Lvalue<'tcx>,
test_kind: &TestKind<'tcx>,
test_outcome: usize,
candidate: &Candidate<'tcx>)
-> BlockAnd<Option<Candidate<'tcx>>> {
-> Option<Candidate<'tcx>> {
let candidate = candidate.clone();
let match_pairs = candidate.match_pairs;
let result = unpack!(block = self.match_pairs_under_assumption(block,
test_lvalue,
test_kind,
test_outcome,
match_pairs));
block.and(match result {
let result = self.match_pairs_under_assumption(test_lvalue,
test_kind,
test_outcome,
match_pairs);
match result {
Some(match_pairs) => Some(Candidate { match_pairs: match_pairs, ..candidate }),
None => None,
})
}
}
/// Helper for candidate_under_assumption that does the actual
/// work of transforming the list of match pairs.
fn match_pairs_under_assumption(&mut self,
mut block: BasicBlock,
test_lvalue: &Lvalue<'tcx>,
test_kind: &TestKind<'tcx>,
test_outcome: usize,
match_pairs: Vec<MatchPair<'tcx>>)
-> BlockAnd<Option<Vec<MatchPair<'tcx>>>> {
-> Option<Vec<MatchPair<'tcx>>> {
let mut result = vec![];
for match_pair in match_pairs {
@ -245,30 +318,17 @@ impl<'a,'tcx> Builder<'a,'tcx> {
continue;
}
let desired_test = self.test(&match_pair);
if *test_kind != desired_test.kind {
// if the match pair wants to (e.g.) test for
// equality against some particular constant, but
// we did a switch, then we can't say whether it
// matches or not, so we still have to include it
// as a possibility.
//
// For example, we have a constant `FOO:
// Option<i32> = Some(22)`, and `match_pair` is `x
// @ FOO`, but we did a switch on the variant
// (`Some` vs `None`). (OK, in principle this
// could tell us something, but we're not that
// smart yet to actually dig into the constant
// itself)
// if this test doesn't tell us anything about this match-pair, then hang onto it.
if !self.test_informs_match_pair(&match_pair, test_kind, test_outcome) {
result.push(match_pair);
continue;
}
// otherwise, build up the consequence match pairs
let opt_consequent_match_pairs =
unpack!(block = self.consequent_match_pairs_under_assumption(block,
match_pair,
test_outcome));
self.consequent_match_pairs_under_assumption(match_pair,
test_kind,
test_outcome);
match opt_consequent_match_pairs {
None => {
// Right kind of test, but wrong outcome. That
@ -276,7 +336,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// inapplicable, since the candidate is only
// applicable if all of its match-pairs apply (and
// this one doesn't).
return block.and(None);
return None;
}
Some(consequent_match_pairs) => {
@ -285,23 +345,122 @@ impl<'a,'tcx> Builder<'a,'tcx> {
}
}
}
block.and(Some(result))
Some(result)
}
/// Identifies what test is needed to decide if `match_pair` is applicable.
/// Given that we executed `test` to `match_pair.lvalue` with
/// outcome `test_outcome`, does that tell us anything about
/// whether `match_pair` applies?
///
/// It is a bug to call this with a simplifyable pattern.
/// Often it does not. For example, if we are testing whether
/// the discriminant equals 4, and we find that it does not,
/// but the `match_pair` is testing if the discriminant equals 5,
/// that does not help us.
fn test_informs_match_pair(&mut self,
match_pair: &MatchPair<'tcx>,
test_kind: &TestKind<'tcx>,
_test_outcome: usize)
-> bool {
match match_pair.pattern.kind {
PatternKind::Variant { .. } => {
match *test_kind {
TestKind::Switch { .. } => true,
_ => false,
}
}
PatternKind::Constant { value: Literal::Value { .. } }
if is_switch_ty(match_pair.pattern.ty) => {
match *test_kind {
TestKind::SwitchInt { .. } => true,
// Did not do an integer equality test (which is always a SwitchInt).
// So we learned nothing relevant to this match-pair.
//
// FIXME(#29623) we could use TestKind::Range to rule
// things out here, in some cases.
_ => false,
}
}
PatternKind::Constant { .. } |
PatternKind::Range { .. } |
PatternKind::Slice { .. } => {
let pattern_test = self.test(&match_pair);
if pattern_test.kind == *test_kind {
true
} else {
// FIXME(#29623) in all 3 cases, we could sometimes do
// better here. For example, if we are checking
// whether the value is equal to X, and we find
// that it is, that (may) imply value is not equal
// to Y. Or, if the range tested is `3..5`, and
// our range is `4..5`, then we know that our
// range also does not apply. Similarly, if we
// test that length is >= 5, and it fails, we also
// know that length is not >= 7. etc.
false
}
}
PatternKind::Array { .. } |
PatternKind::Wild |
PatternKind::Binding { .. } |
PatternKind::Leaf { .. } |
PatternKind::Deref { .. } => {
self.error_simplifyable(&match_pair)
}
}
}
/// Given that we executed `test` with outcome `test_outcome`,
/// what are the resulting match pairs? This can return either:
///
/// - None, meaning that the test indicated that this outcome
/// means that this match-pair is not the current one for the
/// current discriminant (which rules out the enclosing
/// candidate);
/// - Some(...), meaning that either the test didn't tell us whether this
/// match-pair is correct or not, or that we DID match and now have
/// subsequent matches to perform.
///
/// As an example, consider:
///
/// ```
/// match option {
/// Ok(<pattern>) => ...,
/// Err(_) => ...,
/// }
/// ```
///
/// Suppose that the `test` is a `Switch` and the outcome is
/// `Ok`. Then in that case, the first arm will have a match-pair
/// of `option @ Ok(<pattern>)`. In that case we will return
/// `Some(vec![(option as Ok) @ <pattern>])`. The `Some` reuslt
/// indicates that the match-pair still applies, and we now have
/// to test `(option as Ok) @ <pattern>`.
///
/// On the second arm, a `None` will be returned, because if we
/// observed that `option` has the discriminant `Ok`, then the
/// second arm cannot apply.
pub fn consequent_match_pairs_under_assumption(&mut self,
mut block: BasicBlock,
match_pair: MatchPair<'tcx>,
test_kind: &TestKind<'tcx>,
test_outcome: usize)
-> BlockAnd<Option<Vec<MatchPair<'tcx>>>> {
-> Option<Vec<MatchPair<'tcx>>> {
match match_pair.pattern.kind {
PatternKind::Variant { adt_def, variant_index, subpatterns } => {
assert!(match *test_kind { TestKind::Switch { .. } => true,
_ => false });
if test_outcome != variant_index {
return block.and(None);
return None; // Tested, but found the wrong variant.
}
// Correct variant. Extract the subitems and match
// those. The lvalue goes gets downcast, so
// e.g. `foo.bar` becomes `foo.bar as Variant`.
let elem = ProjectionElem::Downcast(adt_def, variant_index);
let downcast_lvalue = match_pair.lvalue.clone().elem(elem);
let consequent_match_pairs =
@ -313,33 +472,37 @@ impl<'a,'tcx> Builder<'a,'tcx> {
self.match_pair(lvalue, subpattern.pattern)
})
.collect();
block.and(Some(consequent_match_pairs))
Some(consequent_match_pairs)
}
PatternKind::Constant { .. } |
PatternKind::Range { .. } => {
// these are boolean tests: if we are on the 0th
// successor, then they passed, and otherwise they
// failed, but there are never any more tests to come.
if test_outcome == 0 {
block.and(Some(vec![]))
} else {
block.and(None)
PatternKind::Constant { value: Literal::Value { ref value } }
if is_switch_ty(match_pair.pattern.ty) => {
match *test_kind {
TestKind::SwitchInt { switch_ty: _, options: _, ref indices } => {
let index = indices[value];
if index == test_outcome {
Some(vec![]) // this value, nothing left to test
} else {
None // some other value, candidate is inapplicable
}
}
_ => {
self.hir.span_bug(
match_pair.pattern.span,
&format!("did a switch-int, but value {:?} not found in cases",
value));
}
}
}
PatternKind::Slice { prefix, slice, suffix } => {
PatternKind::Constant { .. } |
PatternKind::Range { .. } |
PatternKind::Slice { .. } => {
if test_outcome == 0 {
let mut consequent_match_pairs = vec![];
unpack!(block = self.prefix_suffix_slice(&mut consequent_match_pairs,
block,
match_pair.lvalue,
prefix,
slice,
suffix));
block.and(Some(consequent_match_pairs))
Some(vec![])
} else {
block.and(None)
None
}
}
@ -358,3 +521,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
&format!("simplifyable pattern found: {:?}", match_pair.pattern))
}
}
fn is_switch_ty<'tcx>(ty: Ty<'tcx>) -> bool {
ty.is_integral() || ty.is_char()
}

25
src/librustc_mir/repr.rs
View File

@ -251,6 +251,26 @@ pub enum Terminator<'tcx> {
targets: Vec<BasicBlock>,
},
/// operand evaluates to an integer; jump depending on its value
/// to one of the targets, and otherwise fallback to `otherwise`
SwitchInt {
/// discriminant value being tested
discr: Lvalue<'tcx>,
/// type of value being tested
switch_ty: Ty<'tcx>,
/// Possible values. The locations to branch to in each case
/// are found in the corresponding indices from the `targets` vector.
values: Vec<ConstVal>,
/// Possible branch sites. The length of this vector should be
/// equal to the length of the `values` vector plus 1 -- the
/// extra item is the block to branch to if none of the values
/// fit.
targets: Vec<BasicBlock>,
},
/// Indicates that the last statement in the block panics, aborts,
/// etc. No successors. This terminator appears on exactly one
/// basic block which we create in advance. However, during
@ -280,7 +300,8 @@ impl<'tcx> Terminator<'tcx> {
Goto { target: ref b } => slice::ref_slice(b),
Panic { target: ref b } => slice::ref_slice(b),
If { cond: _, targets: ref b } => b,
Switch { discr: _, adt_def: _, targets: ref b } => b,
Switch { targets: ref b, .. } => b,
SwitchInt { targets: ref b, .. } => b,
Diverge => &[],
Return => &[],
Call { data: _, targets: ref b } => b,
@ -321,6 +342,8 @@ impl<'tcx> Debug for Terminator<'tcx> {
write!(fmt, "if({:?}) -> {:?}", lv, targets),
Switch { discr: ref lv, adt_def: _, ref targets } =>
write!(fmt, "switch({:?}) -> {:?}", lv, targets),
SwitchInt { discr: ref lv, switch_ty: _, ref values, ref targets } =>
write!(fmt, "switchInt({:?}, {:?}) -> {:?}", lv, values, targets),
Diverge =>
write!(fmt, "diverge"),
Return =>

7
src/librustc_mir/visit.rs
View File

@ -109,6 +109,13 @@ pub trait Visitor<'tcx> {
}
}
Terminator::SwitchInt { ref discr, switch_ty: _, values: _, ref targets } => {
self.visit_lvalue(discr, LvalueContext::Inspect);
for &target in targets {
self.visit_branch(block, target);
}
}
Terminator::Diverge |
Terminator::Return => {
}

4
src/librustc_trans/trans/mir/block.rs
View File

@ -50,6 +50,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
unimplemented!()
}
mir::Terminator::SwitchInt { .. } => {
unimplemented!()
}
mir::Terminator::Diverge => {
if let Some(llpersonalityslot) = self.llpersonalityslot {
let lp = build::Load(bcx, llpersonalityslot);