Rollup merge of rust-lang#39456 - nagisa:mir-switchint-everywhere, r=…

…nikomatsakis

[MIR] SwitchInt Everywhere

Something I’ve been meaning to do for a very long while. This PR essentially gets rid of 3 kinds of conditional branching and only keeps the most general one – `SwitchInt`. Primary benefits are such that dealing with MIR now does not involve dealing with 3 different ways to do conditional control flow. On the other hand, constructing a `SwitchInt` currently requires more code than what previously was necessary to build an equivalent `If` terminator. Something trivially “fixable” with some constructor methods somewhere (MIR needs stuff like that badly in general).

Some timings (tl;dr: slightly faster¹ (unexpected), but also uses slightly more memory at peak (expected)):

¹: Not sure if the speed benefits are because of LLVM liking the generated code better or the compiler itself getting compiled better. Either way, its a net benefit. The CORE and SYNTAX timings done for compilation without optimisation.

```
AFTER:
Building stage1 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 31.50 secs
    Finished release [optimized] target(s) in 31.42 secs
Building stage1 compiler artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 439.56 secs
    Finished release [optimized] target(s) in 435.15 secs

CORE: 99% (24.81 real, 0.13 kernel, 24.57 user); 358536k resident
CORE: 99% (24.56 real, 0.15 kernel, 24.36 user); 359168k resident
SYNTAX: 99% (49.98 real, 0.48 kernel, 49.42 user); 653416k resident
SYNTAX: 99% (50.07 real, 0.58 kernel, 49.43 user); 653604k resident

BEFORE:
Building stage1 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 31.84 secs
Building stage1 compiler artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 451.17 secs

CORE: 99% (24.66 real, 0.20 kernel, 24.38 user); 351096k resident
CORE: 99% (24.36 real, 0.17 kernel, 24.18 user); 352284k resident
SYNTAX: 99% (52.24 real, 0.56 kernel, 51.66 user); 645544k resident
SYNTAX: 99% (51.55 real, 0.48 kernel, 50.99 user); 646428k resident
```

cc @nikomatsakis @eddyb

src/librustc/middle/const_val.rs

@@ -14,6 +14,7 @@ use std::rc::Rc;
 use hir::def_id::DefId;
 use rustc_const_math::*;
 use self::ConstVal::*;
+pub use rustc_const_math::ConstInt;
 
 use std::collections::BTreeMap;
 
@@ -48,4 +49,14 @@ impl ConstVal {
             Char(..) => "char",
         }
     }
+
+    pub fn to_const_int(&self) -> Option<ConstInt> {
+        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
+        }
+    }
 }

src/librustc/mir/mod.rs

@@ -453,36 +453,30 @@ pub enum TerminatorKind<'tcx> {
         target: BasicBlock,
     },
 
-    /// jump to branch 0 if this lvalue evaluates to true
-    If {
-        cond: Operand<'tcx>,
-        targets: (BasicBlock, BasicBlock),
-    },
-
-    /// lvalue evaluates to some enum; jump depending on the branch
-    Switch {
-        discr: Lvalue<'tcx>,
-        adt_def: &'tcx AdtDef,
-        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>,
+        discr: Operand<'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.
+        values: Cow<'tcx, [ConstInt]>,
+
+        /// Possible branch sites. The last element of this vector is used
+        /// for the otherwise branch, so values.len() == targets.len() + 1
+        /// should hold.
+        // This invariant is quite non-obvious and also could be improved.
+        // One way to make this invariant is to have something like this instead:
+        //
+        // branches: Vec<(ConstInt, BasicBlock)>,
+        // otherwise: Option<BasicBlock> // exhaustive if None
+        //
+        // However we’ve decided to keep this as-is until we figure a case
+        // where some other approach seems to be strictly better than other.
         targets: Vec<BasicBlock>,
     },
 
@@ -546,12 +540,21 @@ impl<'tcx> Terminator<'tcx> {
 }
 
 impl<'tcx> TerminatorKind<'tcx> {
+    pub fn if_<'a, 'gcx>(tcx: ty::TyCtxt<'a, 'gcx, 'tcx>, cond: Operand<'tcx>,
+                         t: BasicBlock, f: BasicBlock) -> TerminatorKind<'tcx> {
+        static BOOL_SWITCH_FALSE: &'static [ConstInt] = &[ConstInt::Infer(0)];
+        TerminatorKind::SwitchInt {
+            discr: cond,
+            switch_ty: tcx.types.bool,
+            values: From::from(BOOL_SWITCH_FALSE),
+            targets: vec![f, t],
+        }
+    }
+
     pub fn successors(&self) -> Cow<[BasicBlock]> {
         use self::TerminatorKind::*;
         match *self {
             Goto { target: ref b } => slice::ref_slice(b).into_cow(),
-            If { targets: (b1, b2), .. } => vec![b1, b2].into_cow(),
-            Switch { targets: ref b, .. } => b[..].into_cow(),
             SwitchInt { targets: ref b, .. } => b[..].into_cow(),
             Resume => (&[]).into_cow(),
             Return => (&[]).into_cow(),
@@ -580,8 +583,6 @@ impl<'tcx> TerminatorKind<'tcx> {
         use self::TerminatorKind::*;
         match *self {
             Goto { target: ref mut b } => vec![b],
-            If { targets: (ref mut b1, ref mut b2), .. } => vec![b1, b2],
-            Switch { targets: ref mut b, .. } => b.iter_mut().collect(),
             SwitchInt { targets: ref mut b, .. } => b.iter_mut().collect(),
             Resume => Vec::new(),
             Return => Vec::new(),
@@ -659,8 +660,6 @@ impl<'tcx> TerminatorKind<'tcx> {
         use self::TerminatorKind::*;
         match *self {
             Goto { .. } => write!(fmt, "goto"),
-            If { cond: ref lv, .. } => write!(fmt, "if({:?})", lv),
-            Switch { discr: ref lv, .. } => write!(fmt, "switch({:?})", lv),
             SwitchInt { discr: ref lv, .. } => write!(fmt, "switchInt({:?})", lv),
             Return => write!(fmt, "return"),
             Resume => write!(fmt, "resume"),
@@ -710,18 +709,11 @@ impl<'tcx> TerminatorKind<'tcx> {
         match *self {
             Return | Resume | Unreachable => vec![],
             Goto { .. } => vec!["".into()],
-            If { .. } => vec!["true".into(), "false".into()],
-            Switch { ref adt_def, .. } => {
-                adt_def.variants
-                       .iter()
-                       .map(|variant| variant.name.to_string().into())
-                       .collect()
-            }
             SwitchInt { ref values, .. } => {
                 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()))
@@ -997,6 +989,12 @@ pub enum Rvalue<'tcx> {
 
     UnaryOp(UnOp, Operand<'tcx>),
 
+    /// Read the discriminant of an ADT.
+    ///
+    /// Undefined (i.e. no effort is made to make it defined, but there’s no reason why it cannot
+    /// be defined to return, say, a 0) if ADT is not an enum.
+    Discriminant(Lvalue<'tcx>),
+
     /// Creates an *uninitialized* Box
     Box(Ty<'tcx>),
 
@@ -1111,6 +1109,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
                 write!(fmt, "Checked{:?}({:?}, {:?})", op, a, b)
             }
             UnaryOp(ref op, ref a) => write!(fmt, "{:?}({:?})", op, a),
+            Discriminant(ref lval) => write!(fmt, "discriminant({:?})", lval),
             Box(ref t) => write!(fmt, "Box({:?})", t),
             InlineAsm { ref asm, ref outputs, ref inputs } => {
                 write!(fmt, "asm!({:?} : {:?} : {:?})", asm, outputs, inputs)

src/librustc/mir/tcx.rs

@@ -18,6 +18,7 @@ use ty::subst::{Subst, Substs};
 use ty::{self, AdtDef, Ty, TyCtxt};
 use ty::fold::{TypeFoldable, TypeFolder, TypeVisitor};
 use hir;
+use ty::util::IntTypeExt;
 
 #[derive(Copy, Clone, Debug)]
 pub enum LvalueTy<'tcx> {
@@ -135,15 +136,15 @@ impl<'tcx> Lvalue<'tcx> {
 impl<'tcx> Rvalue<'tcx> {
     pub fn ty<'a, 'gcx>(&self, mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> Option<Ty<'tcx>>
     {
-        match self {
-            &Rvalue::Use(ref operand) => Some(operand.ty(mir, tcx)),
-            &Rvalue::Repeat(ref operand, ref count) => {
+        match *self {
+            Rvalue::Use(ref operand) => Some(operand.ty(mir, tcx)),
+            Rvalue::Repeat(ref operand, ref count) => {
                 let op_ty = operand.ty(mir, tcx);
                 let count = count.value.as_u64(tcx.sess.target.uint_type);
                 assert_eq!(count as usize as u64, count);
                 Some(tcx.mk_array(op_ty, count as usize))
             }
-            &Rvalue::Ref(reg, bk, ref lv) => {
+            Rvalue::Ref(reg, bk, ref lv) => {
                 let lv_ty = lv.ty(mir, tcx).to_ty(tcx);
                 Some(tcx.mk_ref(reg,
                     ty::TypeAndMut {
@@ -152,27 +153,37 @@ impl<'tcx> Rvalue<'tcx> {
                     }
                 ))
             }
-            &Rvalue::Len(..) => Some(tcx.types.usize),
-            &Rvalue::Cast(.., ty) => Some(ty),
-            &Rvalue::BinaryOp(op, ref lhs, ref rhs) => {
+            Rvalue::Len(..) => Some(tcx.types.usize),
+            Rvalue::Cast(.., ty) => Some(ty),
+            Rvalue::BinaryOp(op, ref lhs, ref rhs) => {
                 let lhs_ty = lhs.ty(mir, tcx);
                 let rhs_ty = rhs.ty(mir, tcx);
                 Some(op.ty(tcx, lhs_ty, rhs_ty))
             }
-            &Rvalue::CheckedBinaryOp(op, ref lhs, ref rhs) => {
+            Rvalue::CheckedBinaryOp(op, ref lhs, ref rhs) => {
                 let lhs_ty = lhs.ty(mir, tcx);
                 let rhs_ty = rhs.ty(mir, tcx);
                 let ty = op.ty(tcx, lhs_ty, rhs_ty);
                 let ty = tcx.intern_tup(&[ty, tcx.types.bool], false);
                 Some(ty)
             }
-            &Rvalue::UnaryOp(_, ref operand) => {
+            Rvalue::UnaryOp(_, ref operand) => {
                 Some(operand.ty(mir, tcx))
             }
-            &Rvalue::Box(t) => {
+            Rvalue::Discriminant(ref lval) => {
+                let ty = lval.ty(mir, tcx).to_ty(tcx);
+                if let ty::TyAdt(adt_def, _) = ty.sty {
+                    Some(adt_def.discr_ty.to_ty(tcx))
+                } else {
+                    // Undefined behaviour, bug for now; may want to return something for
+                    // the `discriminant` intrinsic later.
+                    bug!("Rvalue::Discriminant on Lvalue of type {:?}", ty);
+                }
+            }
+            Rvalue::Box(t) => {
                 Some(tcx.mk_box(t))
             }
-            &Rvalue::Aggregate(ref ak, ref ops) => {
+            Rvalue::Aggregate(ref ak, ref ops) => {
                 match *ak {
                     AggregateKind::Array => {
                         if let Some(operand) = ops.get(0) {
@@ -196,7 +207,7 @@ impl<'tcx> Rvalue<'tcx> {
                     }
                 }
             }
-            &Rvalue::InlineAsm { .. } => None
+            Rvalue::InlineAsm { .. } => None
         }
     }
 }

src/librustc/mir/visit.rs

@@ -14,7 +14,6 @@ use ty::subst::Substs;
 use ty::{ClosureSubsts, Region, Ty};
 use mir::*;
 use rustc_const_math::ConstUsize;
-use rustc_data_structures::tuple_slice::TupleSlice;
 use rustc_data_structures::indexed_vec::Idx;
 use syntax_pos::Span;
 
@@ -224,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) {
@@ -363,31 +368,14 @@ macro_rules! make_mir_visitor {
                         self.visit_branch(block, target);
                     }
 
-                    TerminatorKind::If { ref $($mutability)* cond,
-                                         ref $($mutability)* targets } => {
-                        self.visit_operand(cond, source_location);
-                        for &target in targets.as_slice() {
-                            self.visit_branch(block, target);
-                        }
-                    }
-
-                    TerminatorKind::Switch { ref $($mutability)* discr,
-                                             adt_def: _,
-                                             ref targets } => {
-                        self.visit_lvalue(discr, LvalueContext::Inspect, source_location);
-                        for &target in targets {
-                            self.visit_branch(block, target);
-                        }
-                    }
-
                     TerminatorKind::SwitchInt { ref $($mutability)* discr,
                                                 ref $($mutability)* switch_ty,
-                                                ref $($mutability)* values,
+                                                ref values,
                                                 ref targets } => {
-                        self.visit_lvalue(discr, LvalueContext::Inspect, source_location);
+                        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);
@@ -506,6 +494,10 @@ macro_rules! make_mir_visitor {
                         self.visit_operand(op, location);
                     }
 
+                    Rvalue::Discriminant(ref $($mutability)* lvalue) => {
+                        self.visit_lvalue(lvalue, LvalueContext::Inspect, location);
+                    }
+
                     Rvalue::Box(ref $($mutability)* ty) => {
                         self.visit_ty(ty);
                     }
@@ -712,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_int(&mut self, _const_int: &ConstInt) {
             }
 
-            fn super_const_usize(&mut self, _substs: & $($mutability)* ConstUsize) {
+            fn super_const_usize(&mut self, _const_usize: & $($mutability)* ConstUsize) {
             }
 
             // Convenience methods

src/librustc/ty/context.rs

@@ -672,9 +672,11 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
     pub fn alloc_adt_def(self,
                          did: DefId,
                          kind: AdtKind,
+                         discr_ty: Option<attr::IntType>,
                          variants: Vec<ty::VariantDef>)
                          -> &'gcx ty::AdtDef {
-        let def = ty::AdtDef::new(self, did, kind, variants);
+        let discr_ty = discr_ty.unwrap_or(attr::UnsignedInt(ast::UintTy::U8));
+        let def = ty::AdtDef::new(self, did, kind, discr_ty, variants);
         self.global_arenas.adt_def.alloc(def)
     }
 

src/librustc/ty/layout.rs

@@ -20,7 +20,6 @@ use ty::{self, Ty, TyCtxt, TypeFoldable};
 use syntax::ast::{FloatTy, IntTy, UintTy};
 use syntax::attr;
 use syntax_pos::DUMMY_SP;
-use rustc_const_math::ConstInt;
 
 use std::cmp;
 use std::fmt;
@@ -1207,11 +1206,7 @@ impl<'a, 'gcx, 'tcx> Layout {
                                                             i64::min_value(),
                                                             true);
                     for v in &def.variants {
-                        let x = match v.disr_val.erase_type() {
-                            ConstInt::InferSigned(i) => i as i64,
-                            ConstInt::Infer(i) => i as u64 as i64,
-                            _ => bug!()
-                        };
+                        let x = v.disr_val as i128 as i64;
                         if x == 0 { non_zero = false; }
                         if x < min { min = x; }
                         if x > max { max = x; }
@@ -1271,7 +1266,7 @@ impl<'a, 'gcx, 'tcx> Layout {
                 // non-empty body, explicit discriminants should have
                 // been rejected by a checker before this point.
                 for (i, v) in def.variants.iter().enumerate() {
-                    if i as u128 != v.disr_val.to_u128_unchecked() {
+                    if i as u128 != v.disr_val {
                         bug!("non-C-like enum {} with specified discriminants",
                             tcx.item_path_str(def.did));
                     }