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 2 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
10 changes: 5 additions & 5 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,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 +337,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 +359,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 @@ -745,7 +745,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 +1035,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
18 changes: 8 additions & 10 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
143 changes: 128 additions & 15 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 @@ -136,7 +137,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
PatternKind::Variant { .. } => {
panic!("you should have called add_variants_to_switch instead!");
}
PatternKind::Range { .. } |
PatternKind::Range { ty, lo, hi, end } => {
indices
.keys()
.all(|value| {
!self
.const_range_contains(ty, lo, hi, end, value)
.unwrap_or(true)
})
}
PatternKind::Slice { .. } |
PatternKind::Array { .. } |
PatternKind::Wild |
Expand Down Expand Up @@ -200,20 +209,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 @@ -490,8 +497,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 +538,28 @@ 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 { ty, lo, hi, end }) => {
let not_contained = indices
.keys()
.all(|value| {
!self
.const_range_contains(ty, lo, hi, end, value)
.unwrap_or(true)
});

if not_contained {
// No 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 +638,70 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}

(&TestKind::Range {
lo: test_lo, hi: test_hi, ty: test_ty, end: test_end,
}, &PatternKind::Range {
lo: pat_lo, hi: pat_hi, ty: _, end: pat_end,
}) => {
if (test_lo, test_hi, test_end) == (pat_lo, pat_hi, pat_end) {
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 {
lo, hi, ty, end
}, &PatternKind::Constant {
ref value
}) => {
if self.const_range_contains(ty, lo, hi, end, 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 +812,29 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
"simplifyable pattern found: {:?}",
match_pair.pattern)
}

fn const_range_contains(
&self,
ty: Ty<'tcx>,
lo: &'tcx ty::Const<'tcx>,
hi: &'tcx ty::Const<'tcx>,
end: RangeEnd,
value: &'tcx ty::Const<'tcx>,
) -> Option<bool> {
use std::cmp::Ordering::*;

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

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

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

fn is_switch_ty<'tcx>(ty: Ty<'tcx>) -> bool {
Expand Down
9 changes: 6 additions & 3 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use hair::constant::*;
use rustc::mir::{fmt_const_val, Field, BorrowKind, Mutability};
use rustc::mir::{ProjectionElem, UserTypeAnnotation, UserTypeProjection, UserTypeProjections};
use rustc::mir::interpret::{Scalar, GlobalId, ConstValue, sign_extend};
use rustc::ty::{self, Region, TyCtxt, AdtDef, Ty};
use rustc::ty::{self, Region, TyCtxt, AdtDef, Ty, Lift};
use rustc::ty::subst::{Substs, Kind};
use rustc::ty::layout::VariantIdx;
use rustc::hir::{self, PatKind, RangeEnd};
Expand Down Expand Up @@ -1210,8 +1210,8 @@ impl<'tcx> PatternFoldable<'tcx> for PatternKind<'tcx> {
}
}

pub fn compare_const_vals<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
pub fn compare_const_vals<'a, 'gcx, 'tcx>(
tcx: TyCtxt<'a, 'gcx, 'tcx>,
a: &'tcx ty::Const<'tcx>,
b: &'tcx ty::Const<'tcx>,
ty: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
Expand All @@ -1233,6 +1233,9 @@ pub fn compare_const_vals<'a, 'tcx>(
return fallback();
}

let tcx = tcx.global_tcx();
let (a, b, ty) = (a, b, ty).lift_to_tcx(tcx).unwrap();

// FIXME: This should use assert_bits(ty) instead of use_bits
// but triggers possibly bugs due to mismatching of arrays and slices
if let (Some(a), Some(b)) = (a.to_bits(tcx, ty), b.to_bits(tcx, ty)) {
Expand Down
83 changes: 83 additions & 0 deletions src/test/run-pass/mir/mir_match_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#![feature(exclusive_range_pattern)]

// run-pass

fn main() {
let incl_range = |x, b| {
match x {
0..=5 if b => 0,
5..=10 if b => 1,
1..=4 if !b => 2,
_ => 3,
}
};
assert_eq!(incl_range(3, false), 2);
assert_eq!(incl_range(3, true), 0);
assert_eq!(incl_range(5, false), 3);
assert_eq!(incl_range(5, true), 0);

let excl_range = |x, b| {
match x {
0..5 if b => 0,
5..10 if b => 1,
1..4 if !b => 2,
_ => 3,
}
};
assert_eq!(excl_range(3, false), 2);
assert_eq!(excl_range(3, true), 0);
assert_eq!(excl_range(5, false), 3);
assert_eq!(excl_range(5, true), 1);

let incl_range_vs_const = |x, b| {
match x {
0..=5 if b => 0,
7 => 1,
3 => 2,
_ => 3,
}
};
assert_eq!(incl_range_vs_const(5, false), 3);
assert_eq!(incl_range_vs_const(5, true), 0);
assert_eq!(incl_range_vs_const(3, false), 2);
assert_eq!(incl_range_vs_const(3, true), 0);
assert_eq!(incl_range_vs_const(7, false), 1);
assert_eq!(incl_range_vs_const(7, true), 1);

let excl_range_vs_const = |x, b| {
match x {
0..5 if b => 0,
7 => 1,
3 => 2,
_ => 3,
}
};
assert_eq!(excl_range_vs_const(5, false), 3);
assert_eq!(excl_range_vs_const(5, true), 3);
assert_eq!(excl_range_vs_const(3, false), 2);
assert_eq!(excl_range_vs_const(3, true), 0);
assert_eq!(excl_range_vs_const(7, false), 1);
assert_eq!(excl_range_vs_const(7, true), 1);

let const_vs_incl_range = |x, b| {
match x {
3 if b => 0,
5..=7 => 2,
1..=4 => 1,
_ => 3,
}
};
assert_eq!(const_vs_incl_range(3, false), 1);
assert_eq!(const_vs_incl_range(3, true), 0);

let const_vs_excl_range = |x, b| {
match x {
3 if b => 0,
5..7 => 2,
1..4 => 1,
_ => 3,
}
};
assert_eq!(const_vs_excl_range(3, false), 1);
assert_eq!(const_vs_excl_range(3, true), 0);
}