Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve MIR match generation for ranges #56810

Merged
merged 6 commits into from
Dec 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use build::{BlockAnd, BlockAndExtension, Builder};
use build::{GuardFrame, GuardFrameLocal, LocalsForNode};
use hair::*;
use hair::pattern::PatternTypeProjections;
use rustc::hir;
use rustc::mir::*;
use rustc::ty::{self, Ty};
use rustc::ty::layout::VariantIdx;
Expand Down Expand Up @@ -100,7 +99,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
.collect();

// create binding start block for link them by false edges
let candidate_count = arms.iter().fold(0, |ac, c| ac + c.patterns.len());
let candidate_count = arms.iter().map(|c| c.patterns.len()).sum::<usize>();
let pre_binding_blocks: Vec<_> = (0..=candidate_count)
.map(|_| self.cfg.start_new_block())
.collect();
Expand Down Expand Up @@ -337,7 +336,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {

pub fn place_into_pattern(
&mut self,
mut block: BasicBlock,
block: BasicBlock,
irrefutable_pat: Pattern<'tcx>,
initializer: &Place<'tcx>,
set_match_place: bool,
Expand All @@ -359,7 +358,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {

// Simplify the candidate. Since the pattern is irrefutable, this should
// always convert all match-pairs into bindings.
unpack!(block = self.simplify_candidate(block, &mut candidate));
self.simplify_candidate(&mut candidate);

if !candidate.match_pairs.is_empty() {
span_bug!(
Expand Down Expand Up @@ -681,12 +680,7 @@ enum TestKind<'tcx> {
},

// test whether the value falls within an inclusive or exclusive range
Range {
lo: &'tcx ty::Const<'tcx>,
hi: &'tcx ty::Const<'tcx>,
ty: Ty<'tcx>,
end: hir::RangeEnd,
},
Range(PatternRange<'tcx>),

// test length of the slice is equal to len
Len {
Expand Down Expand Up @@ -745,7 +739,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// complete, all the match pairs which remain require some
// form of test, whether it be a switch or pattern comparison.
for candidate in &mut candidates {
unpack!(block = self.simplify_candidate(block, candidate));
self.simplify_candidate(candidate);
}

// The candidates are sorted by priority. Check to see
Expand Down Expand Up @@ -1035,7 +1029,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
test, match_pair
);
let target_blocks = self.perform_test(block, &match_pair.place, &test);
let mut target_candidates: Vec<_> = (0..target_blocks.len()).map(|_| vec![]).collect();
let mut target_candidates = vec![vec![]; target_blocks.len()];

// Sort the candidates into the appropriate vector in
// `target_candidates`. Note that at some point we may
Expand Down
20 changes: 9 additions & 11 deletions src/librustc_mir/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@
//! sort of test: for example, testing which variant an enum is, or
//! testing a value against a constant.

use build::{BlockAnd, BlockAndExtension, Builder};
use build::Builder;
use build::matches::{Ascription, Binding, MatchPair, Candidate};
use hair::*;
use rustc::mir::*;
use rustc::ty;
use rustc::ty::layout::{Integer, IntegerExt, Size};
use syntax::attr::{SignedInt, UnsignedInt};
Expand All @@ -35,24 +34,23 @@ use std::mem;

impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
pub fn simplify_candidate<'pat>(&mut self,
block: BasicBlock,
candidate: &mut Candidate<'pat, 'tcx>)
-> BlockAnd<()> {
candidate: &mut Candidate<'pat, 'tcx>) {
// repeatedly simplify match pairs until fixed point is reached
loop {
let match_pairs = mem::replace(&mut candidate.match_pairs, vec![]);
let mut progress = match_pairs.len(); // count how many were simplified
let mut changed = false;
for match_pair in match_pairs {
match self.simplify_match_pair(match_pair, candidate) {
Ok(()) => {}
Ok(()) => {
changed = true;
}
Err(match_pair) => {
candidate.match_pairs.push(match_pair);
progress -= 1; // this one was not simplified
}
}
}
if progress == 0 {
return block.unit(); // if we were not able to simplify any, done.
if !changed {
return; // if we were not able to simplify any, done.
}
}
}
Expand Down Expand Up @@ -109,7 +107,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
Err(match_pair)
}

PatternKind::Range { lo, hi, ty, end } => {
PatternKind::Range(PatternRange { lo, hi, ty, end }) => {
let range = match ty.sty {
ty::Char => {
Some(('\u{0000}' as u128, '\u{10FFFF}' as u128, Size::from_bits(32)))
Expand Down
152 changes: 128 additions & 24 deletions src/librustc_mir/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use build::Builder;
use build::matches::{Candidate, MatchPair, Test, TestKind};
use hair::*;
use hair::pattern::compare_const_vals;
use rustc_data_structures::bit_set::BitSet;
use rustc_data_structures::fx::FxHashMap;
use rustc::ty::{self, Ty};
Expand Down Expand Up @@ -71,16 +72,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}

PatternKind::Range { lo, hi, ty, end } => {
assert!(ty == match_pair.pattern.ty);
PatternKind::Range(range) => {
assert!(range.ty == match_pair.pattern.ty);
Test {
span: match_pair.pattern.span,
kind: TestKind::Range {
lo,
hi,
ty,
end,
},
kind: TestKind::Range(range),
}
}

Expand Down Expand Up @@ -136,7 +132,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
PatternKind::Variant { .. } => {
panic!("you should have called add_variants_to_switch instead!");
}
PatternKind::Range { .. } |
PatternKind::Range(range) => {
// Check that none of the switch values are in the range.
self.values_not_contained_in_range(range, indices)
.unwrap_or(false)
}
PatternKind::Slice { .. } |
PatternKind::Array { .. } |
PatternKind::Wild |
Expand Down Expand Up @@ -200,20 +200,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
for (idx, discr) in adt_def.discriminants(tcx) {
target_blocks.push(if variants.contains(idx) {
values.push(discr.val);
targets.push(self.cfg.start_new_block());
*targets.last().unwrap()
let block = self.cfg.start_new_block();
targets.push(block);
block
} else {
if otherwise_block.is_none() {
otherwise_block = Some(self.cfg.start_new_block());
}
otherwise_block.unwrap()
*otherwise_block
.get_or_insert_with(|| self.cfg.start_new_block())
});
}
if let Some(otherwise_block) = otherwise_block {
targets.push(otherwise_block);
} else {
targets.push(self.unreachable_block());
}
targets.push(
otherwise_block
.unwrap_or_else(|| self.unreachable_block()),
);
debug!("num_enum_variants: {}, tested variants: {:?}, variants: {:?}",
num_enum_variants, values, variants);
let discr_ty = adt_def.repr.discr_type().to_ty(tcx);
Expand Down Expand Up @@ -378,7 +376,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}

TestKind::Range { ref lo, ref hi, ty, ref end } => {
TestKind::Range(PatternRange { ref lo, ref hi, ty, ref end }) => {
// Test `val` by computing `lo <= val && val <= hi`, using primitive comparisons.
let lo = self.literal_operand(test.span, ty.clone(), lo.clone());
let hi = self.literal_operand(test.span, ty.clone(), hi.clone());
Expand Down Expand Up @@ -490,8 +488,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// away.)
let tested_match_pair = candidate.match_pairs.iter()
.enumerate()
.filter(|&(_, mp)| mp.place == *test_place)
.next();
.find(|&(_, mp)| mp.place == *test_place);
let (match_pair_index, match_pair) = match tested_match_pair {
Some(pair) => pair,
None => {
Expand Down Expand Up @@ -532,6 +529,24 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
resulting_candidates[index].push(new_candidate);
true
}

(&TestKind::SwitchInt { switch_ty: _, ref options, ref indices },
&PatternKind::Range(range)) => {
let not_contained = self
.values_not_contained_in_range(range, indices)
.unwrap_or(false);

if not_contained {
// No switch values are contained in the pattern range,
// so the pattern can be matched only if this test fails.
let otherwise = options.len();
resulting_candidates[otherwise].push(candidate.clone());
true
} else {
false
}
}

(&TestKind::SwitchInt { .. }, _) => false,


Expand Down Expand Up @@ -610,8 +625,63 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}

(&TestKind::Range(test),
&PatternKind::Range(pat)) => {
if test == pat {
resulting_candidates[0]
.push(self.candidate_without_match_pair(
match_pair_index,
candidate,
));
return true;
}

let no_overlap = (|| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can use the try syntax inside the compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try syntax seems to require not only #![feature(try_block)] but also --edition 2018 for try keyword.

use std::cmp::Ordering::*;
use rustc::hir::RangeEnd::*;

let param_env = ty::ParamEnv::empty().and(test.ty);
let tcx = self.hir.tcx();

let lo = compare_const_vals(tcx, test.lo, pat.hi, param_env)?;
let hi = compare_const_vals(tcx, test.hi, pat.lo, param_env)?;

match (test.end, pat.end, lo, hi) {
// pat < test
(_, _, Greater, _) |
(_, Excluded, Equal, _) |
// pat > test
(_, _, _, Less) |
(Excluded, _, _, Equal) => Some(true),
_ => Some(false),
}
})();

if no_overlap == Some(true) {
// Testing range does not overlap with pattern range,
// so the pattern can be matched only if this test fails.
resulting_candidates[1].push(candidate.clone());
true
} else {
false
}
}

(&TestKind::Range(range), &PatternKind::Constant { ref value }) => {
if self.const_range_contains(range, value) == Some(false) {
// `value` is not contained in the testing range,
// so `value` can be matched only if this test fails.
resulting_candidates[1].push(candidate.clone());
true
} else {
false
}
}

(&TestKind::Range { .. }, _) => false,


(&TestKind::Eq { .. }, _) |
(&TestKind::Range { .. }, _) |
(&TestKind::Len { .. }, _) => {
// These are all binary tests.
//
Expand Down Expand Up @@ -722,6 +792,40 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
"simplifyable pattern found: {:?}",
match_pair.pattern)
}

fn const_range_contains(
&self,
range: PatternRange<'tcx>,
value: &'tcx ty::Const<'tcx>,
) -> Option<bool> {
use std::cmp::Ordering::*;

let param_env = ty::ParamEnv::empty().and(range.ty);
let tcx = self.hir.tcx();

let a = compare_const_vals(tcx, range.lo, value, param_env)?;
let b = compare_const_vals(tcx, value, range.hi, param_env)?;

match (b, range.end) {
(Less, _) |
(Equal, RangeEnd::Included) if a != Greater => Some(true),
_ => Some(false),
}
}

fn values_not_contained_in_range(
&self,
range: PatternRange<'tcx>,
indices: &FxHashMap<&'tcx ty::Const<'tcx>, usize>,
) -> Option<bool> {
for val in indices.keys() {
if self.const_range_contains(range, val)? {
return Some(false);
}
}

Some(true)
}
}

fn is_switch_ty<'tcx>(ty: Ty<'tcx>) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/hair/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub mod cx;
mod constant;

pub mod pattern;
pub use self::pattern::{BindingMode, Pattern, PatternKind, FieldPattern};
pub use self::pattern::{BindingMode, Pattern, PatternKind, PatternRange, FieldPattern};
pub(crate) use self::pattern::{PatternTypeProjection, PatternTypeProjections};

mod util;
Expand Down
Loading