From a00a0adc7913152bff626d6dbebfa2cfdbb93d0a Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Thu, 2 Feb 2017 06:44:30 +0200 Subject: [PATCH] Only SwitchInt over integers, not all consts Also use a Cow to avoid full Vec for all SwitchInts --- src/librustc/middle/const_val.rs | 13 +++ src/librustc/mir/mod.rs | 8 +- src/librustc/mir/visit.rs | 19 +++- .../borrowck/mir/elaborate_drops.rs | 6 +- src/librustc_mir/build/expr/into.rs | 9 +- src/librustc_mir/build/matches/mod.rs | 2 +- src/librustc_mir/build/matches/test.rs | 95 ++++++++----------- src/librustc_trans/mir/block.rs | 27 ++++-- src/librustc_trans/mir/constant.rs | 48 ++++++---- src/libserialize/serialize.rs | 28 ++++++ 10 files changed, 154 insertions(+), 101 deletions(-) diff --git a/src/librustc/middle/const_val.rs b/src/librustc/middle/const_val.rs index c4e3827fef2..f885a6d9569 100644 --- a/src/librustc/middle/const_val.rs +++ b/src/librustc/middle/const_val.rs @@ -11,6 +11,7 @@ use syntax::symbol::InternedString; use syntax::ast; use std::rc::Rc; +use std::borrow::Cow; use hir::def_id::DefId; use rustc_const_math::*; use self::ConstVal::*; @@ -18,6 +19,8 @@ pub use rustc_const_math::ConstInt; use std::collections::BTreeMap; +pub static BOOL_SWITCH_TRUE: Cow<'static, [ConstInt]> = Cow::Borrowed(&[ConstInt::Infer(1)]); + #[derive(Clone, Debug, Hash, RustcEncodable, RustcDecodable, Eq, PartialEq)] pub enum ConstVal { Float(ConstFloat), @@ -49,4 +52,14 @@ impl ConstVal { Char(..) => "char", } } + + pub fn to_const_int(&self) -> Option { + match *self { + ConstVal::Integral(i) => Some(i), + ConstVal::Bool(true) => Some(ConstInt::Infer(1)), + ConstVal::Bool(false) => Some(ConstInt::Infer(0)), + ConstVal::Char(ch) => Some(ConstInt::U32(ch as u32)), + _ => None + } + } } diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index b0a6784c3c3..28990fd323f 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -446,6 +446,9 @@ pub struct Terminator<'tcx> { pub kind: TerminatorKind<'tcx> } +/// For use in SwitchInt, for switching on bools. +pub static BOOL_SWITCH_TRUE: Cow<'static, [ConstInt]> = Cow::Borrowed(&[ConstInt::Infer(1)]); + #[derive(Clone, RustcEncodable, RustcDecodable)] pub enum TerminatorKind<'tcx> { /// block should have one successor in the graph; we jump there @@ -464,8 +467,7 @@ pub enum TerminatorKind<'tcx> { /// Possible values. The locations to branch to in each case /// are found in the corresponding indices from the `targets` vector. - // FIXME: ConstVal doesn’t quite make any sense here? Its a Switch*Int*. - values: Vec, + values: Cow<'tcx, [ConstInt]>, /// Possible branch sites. The length of this vector should be /// equal to the length of the `values` vector plus 1 -- the @@ -696,7 +698,7 @@ impl<'tcx> TerminatorKind<'tcx> { values.iter() .map(|const_val| { let mut buf = String::new(); - fmt_const_val(&mut buf, const_val).unwrap(); + fmt_const_val(&mut buf, &ConstVal::Integral(*const_val)).unwrap(); buf.into() }) .chain(iter::once(String::from("otherwise").into())) diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index ca20cf6236b..be3c43db7ba 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -223,6 +223,12 @@ macro_rules! make_mir_visitor { self.super_const_val(const_val); } + fn visit_const_int(&mut self, + const_int: &ConstInt, + _: Location) { + self.super_const_int(const_int); + } + fn visit_const_usize(&mut self, const_usize: & $($mutability)* ConstUsize, _: Location) { @@ -364,12 +370,12 @@ macro_rules! make_mir_visitor { TerminatorKind::SwitchInt { ref $($mutability)* discr, ref $($mutability)* switch_ty, - ref $($mutability)* values, + ref values, ref targets } => { self.visit_operand(discr, source_location); self.visit_ty(switch_ty); - for value in values { - self.visit_const_val(value, source_location); + for value in &values[..] { + self.visit_const_int(value, source_location); } for &target in targets { self.visit_branch(block, target); @@ -698,10 +704,13 @@ macro_rules! make_mir_visitor { _substs: & $($mutability)* ClosureSubsts<'tcx>) { } - fn super_const_val(&mut self, _substs: & $($mutability)* ConstVal) { + fn super_const_val(&mut self, _const_val: & $($mutability)* ConstVal) { } - fn super_const_usize(&mut self, _substs: & $($mutability)* ConstUsize) { + fn super_const_int(&mut self, _const_int: &ConstInt) { + } + + fn super_const_usize(&mut self, _const_usize: & $($mutability)* ConstUsize) { } // Convenience methods diff --git a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs index 8f3dcd0b8d6..53e84f1fb71 100644 --- a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs +++ b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs @@ -679,7 +679,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { let discr = ConstInt::new_inttype(variant.disr_val, adt.discr_ty, self.tcx.sess.target.uint_type, self.tcx.sess.target.int_type).unwrap(); - values.push(ConstVal::Integral(discr)); + values.push(discr); blocks.push(self.open_drop_for_variant(c, &mut drop_block, adt, substs, idx)); } // If there are multiple variants, then if something @@ -704,7 +704,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { kind: TerminatorKind::SwitchInt { discr: Operand::Consume(discr), switch_ty: discr_ty, - values: values, + values: From::from(values), targets: blocks, // adt_def: adt, // targets: variant_drops @@ -836,7 +836,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { self.new_block(c, is_cleanup, TerminatorKind::SwitchInt { discr: Operand::Consume(flag), switch_ty: boolty, - values: vec![ConstVal::Bool(true)], + values: BOOL_SWITCH_TRUE.clone(), targets: vec![on_set, on_unset], }) } diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 2b4336ba66f..537daa4a15b 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -15,7 +15,6 @@ use build::expr::category::{Category, RvalueFunc}; use hair::*; use rustc::ty; use rustc::mir::*; -use rustc::middle::const_val::ConstVal; impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// Compile `expr`, storing the result into `destination`, which @@ -73,7 +72,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { this.cfg.terminate(block, source_info, TerminatorKind::SwitchInt { discr: operand, switch_ty: this.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], + values: BOOL_SWITCH_TRUE.clone(), targets: vec![then_block, else_block], }); @@ -120,7 +119,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { this.cfg.terminate(block, source_info, TerminatorKind::SwitchInt { discr: lhs, switch_ty: this.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], + values: BOOL_SWITCH_TRUE.clone(), targets: blocks, }); @@ -128,7 +127,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { this.cfg.terminate(else_block, source_info, TerminatorKind::SwitchInt { discr: rhs, switch_ty: this.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], + values: BOOL_SWITCH_TRUE.clone(), targets: vec![true_block, false_block], }); @@ -192,7 +191,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { TerminatorKind::SwitchInt { discr: cond, switch_ty: this.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], + values: BOOL_SWITCH_TRUE.clone(), targets: vec![body_block, exit_block], }); diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 0898d06d2e4..885965da1f9 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -675,7 +675,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.cfg.terminate(block, source_info, TerminatorKind::SwitchInt { discr: cond, switch_ty: self.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], + values: BOOL_SWITCH_TRUE.clone(), targets: vec![arm_block, otherwise], }); Some(otherwise) diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index 528e37e73c4..f2d48f65fef 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -196,7 +196,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let discr = ConstInt::new_inttype(variant.disr_val, adt_def.discr_ty, tcx.sess.target.uint_type, tcx.sess.target.int_type).unwrap(); - values.push(ConstVal::Integral(discr)); + values.push(discr); *(targets.place_back() <- self.cfg.start_new_block()) } else { if otherwise_block.is_none() { @@ -226,59 +226,45 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.cfg.terminate(block, source_info, TerminatorKind::SwitchInt { discr: Operand::Consume(discr), switch_ty: discr_ty, - values: values, + values: From::from(values), targets: targets }); target_blocks } TestKind::SwitchInt { switch_ty, ref options, indices: _ } => { - let (targets, term) = match switch_ty.sty { - // If we're matching on boolean we can - // use the If TerminatorKind instead - ty::TyBool => { - assert!(options.len() > 0 && options.len() <= 2); - - let (true_bb, else_bb) = - (self.cfg.start_new_block(), - self.cfg.start_new_block()); - - let targets = match &options[0] { - &ConstVal::Bool(true) => vec![true_bb, else_bb], - &ConstVal::Bool(false) => vec![else_bb, true_bb], - v => span_bug!(test.span, "expected boolean value but got {:?}", v) - }; - - (targets, TerminatorKind::SwitchInt { - discr: Operand::Consume(lvalue.clone()), - switch_ty: self.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], - targets: vec![true_bb, else_bb] - }) - - } - _ => { - // The switch may be inexhaustive so we - // add a catch all block - let otherwise = self.cfg.start_new_block(); - let targets: Vec<_> = - options.iter() - .map(|_| self.cfg.start_new_block()) - .chain(Some(otherwise)) - .collect(); - - (targets.clone(), - TerminatorKind::SwitchInt { - discr: Operand::Consume(lvalue.clone()), - switch_ty: switch_ty, - values: options.clone(), - targets: targets - }) - } + let (values, targets, ret) = if switch_ty.sty == ty::TyBool { + assert!(options.len() > 0 && options.len() <= 2); + let (true_bb, false_bb) = (self.cfg.start_new_block(), + self.cfg.start_new_block()); + let ret = match &options[0] { + &ConstVal::Bool(true) => vec![true_bb, false_bb], + &ConstVal::Bool(false) => vec![false_bb, true_bb], + v => span_bug!(test.span, "expected boolean value but got {:?}", v) + }; + (BOOL_SWITCH_TRUE.clone(), vec![true_bb, false_bb], ret) + } else { + // The switch may be inexhaustive so we + // add a catch all block + let otherwise = self.cfg.start_new_block(); + let targets: Vec<_> = + options.iter() + .map(|_| self.cfg.start_new_block()) + .chain(Some(otherwise)) + .collect(); + let values: Vec<_> = options.iter().map(|v| + v.to_const_int().expect("switching on integral") + ).collect(); + (From::from(values), targets.clone(), targets) }; - self.cfg.terminate(block, source_info, term); - targets + self.cfg.terminate(block, source_info, TerminatorKind::SwitchInt { + discr: Operand::Consume(lvalue.clone()), + switch_ty: switch_ty, + values: values, + targets: targets.clone(), + }); + ret } TestKind::Eq { ref value, mut ty } => { @@ -346,10 +332,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.cfg.terminate(eq_block, source_info, TerminatorKind::SwitchInt { discr: Operand::Consume(eq_result), switch_ty: self.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], + values: BOOL_SWITCH_TRUE.clone(), targets: vec![block, fail], }); - vec![block, fail] } else { let block = self.compare(block, fail, test.span, BinOp::Eq, expect, val); @@ -391,16 +376,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { Operand::Consume(expected))); // branch based on result - let target_blocks: Vec<_> = vec![self.cfg.start_new_block(), - self.cfg.start_new_block()]; + let (false_bb, true_bb) = (self.cfg.start_new_block(), + self.cfg.start_new_block()); self.cfg.terminate(block, source_info, TerminatorKind::SwitchInt { discr: Operand::Consume(result), switch_ty: self.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], - targets: target_blocks.clone(), + values: BOOL_SWITCH_TRUE.clone(), + targets: vec![true_bb, false_bb], }); - - target_blocks + vec![true_bb, false_bb] } } } @@ -425,10 +409,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.cfg.terminate(block, source_info, TerminatorKind::SwitchInt { discr: Operand::Consume(result), switch_ty: self.hir.bool_ty(), - values: vec![ConstVal::Bool(true)], + values: BOOL_SWITCH_TRUE.clone(), targets: vec![target_block, fail_block] }); - target_block } diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index 2ed2cc61062..651d0066b12 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -11,6 +11,7 @@ use llvm::{self, ValueRef, BasicBlockRef}; use rustc_const_eval::{ErrKind, ConstEvalErr, note_const_eval_err}; use rustc::middle::lang_items; +use rustc::middle::const_val::ConstInt; use rustc::ty::{self, layout, TypeFoldable}; use rustc::mir; use abi::{Abi, FnType, ArgType}; @@ -134,14 +135,24 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> { } mir::TerminatorKind::SwitchInt { ref discr, switch_ty, ref values, ref targets } => { - // TODO: cond_br if only 1 value - let (otherwise, targets) = targets.split_last().unwrap(); - let discr = self.trans_operand(&bcx, discr).immediate(); - let switch = bcx.switch(discr, llblock(self, *otherwise), values.len()); - for (value, target) in values.iter().zip(targets) { - let val = Const::from_constval(bcx.ccx, value.clone(), switch_ty); - let llbb = llblock(self, *target); - bcx.add_case(switch, val.llval, llbb) + let discr = self.trans_operand(&bcx, discr); + if switch_ty == bcx.tcx().types.bool { + let lltrue = llblock(self, targets[0]); + let llfalse = llblock(self, targets[1]); + if let [ConstInt::Infer(0)] = values[..] { + bcx.cond_br(discr.immediate(), llfalse, lltrue); + } else { + bcx.cond_br(discr.immediate(), lltrue, llfalse); + } + } else { + let (otherwise, targets) = targets.split_last().unwrap(); + let switch = bcx.switch(discr.immediate(), + llblock(self, *otherwise), values.len()); + for (value, target) in values.iter().zip(targets) { + let val = Const::from_constint(bcx.ccx, value); + let llbb = llblock(self, *target); + bcx.add_case(switch, val.llval, llbb) + } } } diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index 11668e792e3..19139301bb0 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -61,6 +61,33 @@ impl<'tcx> Const<'tcx> { } } + pub fn from_constint<'a>(ccx: &CrateContext<'a, 'tcx>, ci: &ConstInt) + -> Const<'tcx> { + let tcx = ccx.tcx(); + let (llval, ty) = match *ci { + I8(v) => (C_integral(Type::i8(ccx), v as u64, true), tcx.types.i8), + I16(v) => (C_integral(Type::i16(ccx), v as u64, true), tcx.types.i16), + I32(v) => (C_integral(Type::i32(ccx), v as u64, true), tcx.types.i32), + I64(v) => (C_integral(Type::i64(ccx), v as u64, true), tcx.types.i64), + I128(v) => (C_big_integral(Type::i128(ccx), v as u128), tcx.types.i128), + Isize(v) => { + let i = v.as_i64(ccx.tcx().sess.target.int_type); + (C_integral(Type::int(ccx), i as u64, true), tcx.types.isize) + }, + U8(v) => (C_integral(Type::i8(ccx), v as u64, false), tcx.types.u8), + U16(v) => (C_integral(Type::i16(ccx), v as u64, false), tcx.types.u16), + U32(v) => (C_integral(Type::i32(ccx), v as u64, false), tcx.types.u32), + U64(v) => (C_integral(Type::i64(ccx), v, false), tcx.types.u64), + U128(v) => (C_big_integral(Type::i128(ccx), v), tcx.types.u128), + Usize(v) => { + let u = v.as_u64(ccx.tcx().sess.target.uint_type); + (C_integral(Type::int(ccx), u, false), tcx.types.usize) + }, + Infer(_) | InferSigned(_) => bug!("MIR must not use `{:?}`", ci), + }; + Const { llval: llval, ty: ty } + } + /// Translate ConstVal into a LLVM constant value. pub fn from_constval<'a>(ccx: &CrateContext<'a, 'tcx>, cv: ConstVal, @@ -72,26 +99,7 @@ impl<'tcx> Const<'tcx> { ConstVal::Float(F64(v)) => C_floating_f64(v, llty), ConstVal::Float(FInfer {..}) => bug!("MIR must not use `{:?}`", cv), ConstVal::Bool(v) => C_bool(ccx, v), - ConstVal::Integral(I8(v)) => C_integral(Type::i8(ccx), v as u64, true), - ConstVal::Integral(I16(v)) => C_integral(Type::i16(ccx), v as u64, true), - ConstVal::Integral(I32(v)) => C_integral(Type::i32(ccx), v as u64, true), - ConstVal::Integral(I64(v)) => C_integral(Type::i64(ccx), v as u64, true), - ConstVal::Integral(I128(v)) => C_big_integral(Type::i128(ccx), v as u128), - ConstVal::Integral(Isize(v)) => { - let i = v.as_i64(ccx.tcx().sess.target.int_type); - C_integral(Type::int(ccx), i as u64, true) - }, - ConstVal::Integral(U8(v)) => C_integral(Type::i8(ccx), v as u64, false), - ConstVal::Integral(U16(v)) => C_integral(Type::i16(ccx), v as u64, false), - ConstVal::Integral(U32(v)) => C_integral(Type::i32(ccx), v as u64, false), - ConstVal::Integral(U64(v)) => C_integral(Type::i64(ccx), v, false), - ConstVal::Integral(U128(v)) => C_big_integral(Type::i128(ccx), v), - ConstVal::Integral(Usize(v)) => { - let u = v.as_u64(ccx.tcx().sess.target.uint_type); - C_integral(Type::int(ccx), u, false) - }, - ConstVal::Integral(Infer(_)) | - ConstVal::Integral(InferSigned(_)) => bug!("MIR must not use `{:?}`", cv), + ConstVal::Integral(ref i) => return Const::from_constint(ccx, i), ConstVal::Str(ref v) => C_str_slice(ccx, v.clone()), ConstVal::ByteStr(ref v) => consts::addr_of(ccx, C_bytes(ccx, v), 1, "byte_str"), ConstVal::Struct(_) | ConstVal::Tuple(_) | diff --git a/src/libserialize/serialize.rs b/src/libserialize/serialize.rs index ba39fcdec6f..c6847249803 100644 --- a/src/libserialize/serialize.rs +++ b/src/libserialize/serialize.rs @@ -567,6 +567,34 @@ impl Decodable for Vec { } } +impl<'a, T:Encodable> Encodable for Cow<'a, [T]> +where [T]: ToOwned> +{ + fn encode(&self, s: &mut S) -> Result<(), S::Error> { + s.emit_seq(self.len(), |s| { + for (i, e) in self.iter().enumerate() { + s.emit_seq_elt(i, |s| e.encode(s))? + } + Ok(()) + }) + } +} + +impl Decodable for Cow<'static, [T]> +where [T]: ToOwned> +{ + fn decode(d: &mut D) -> Result, D::Error> { + d.read_seq(|d, len| { + let mut v = Vec::with_capacity(len); + for i in 0..len { + v.push(d.read_seq_elt(i, |d| Decodable::decode(d))?); + } + Ok(Cow::Owned(v)) + }) + } +} + + impl Encodable for Option { fn encode(&self, s: &mut S) -> Result<(), S::Error> { s.emit_option(|s| {