Store booleans as i8 in memory to improve optimizations by LLVM

LLVM doesn't really like types with a bit-width that isn't a multiple of
8 and disable various optimizations if it encounters such types used
with loads/stores. OTOH, booleans must be represented as i1 when used as
SSA values. To get the best results, we must use i1 for SSA values, and
i8 when storing the value to memory.

By using range asserts on loads, LLVM can eliminate the required
zero-extend and truncate operations.

Fixes #15203

Author: dotdash

src/librustc/middle/trans/adt.rs

@@ -641,7 +641,7 @@ pub fn trans_start_init(bcx: &Block, r: &Repr, val: ValueRef, discr: Disr) {
         }
         Univariant(ref st, true) => {
             assert_eq!(discr, 0);
-            Store(bcx, C_bool(bcx.ccx(), true),
+            Store(bcx, C_u8(bcx.ccx(), 1),
                   GEPi(bcx, val, [0, st.fields.len() - 1]))
         }
         Univariant(..) => {

src/librustc/middle/trans/base.rs

@@ -959,10 +959,42 @@ pub fn need_invoke(bcx: &Block) -> bool {
 
 pub fn load_if_immediate(cx: &Block, v: ValueRef, t: ty::t) -> ValueRef {
     let _icx = push_ctxt("load_if_immediate");
-    if type_is_immediate(cx.ccx(), t) { return Load(cx, v); }
+    if type_is_immediate(cx.ccx(), t) { return load_ty(cx, v, t); }
     return v;
 }
 
+pub fn load_ty(cx: &Block, ptr: ValueRef, t: ty::t) -> ValueRef {
+    /*!
+     * Helper for loading values from memory. Does the necessary conversion if
+     * the in-memory type differs from the type used for SSA values. Also
+     * handles various special cases where the type gives us better information
+     * about what we are loading.
+     */
+    if type_is_zero_size(cx.ccx(), t) {
+        C_undef(type_of::type_of(cx.ccx(), t))
+    } else if ty::type_is_bool(t) {
+        Trunc(cx, LoadRangeAssert(cx, ptr, 0, 2, lib::llvm::False), Type::i1(cx.ccx()))
+    } else if ty::type_is_char(t) {
+        // a char is a unicode codepoint, and so takes values from 0
+        // to 0x10FFFF inclusive only.
+        LoadRangeAssert(cx, ptr, 0, 0x10FFFF + 1, lib::llvm::False)
+    } else {
+        Load(cx, ptr)
+    }
+}
+
+pub fn store_ty(cx: &Block, v: ValueRef, dst: ValueRef, t: ty::t) {
+    /*!
+     * Helper for storing values in memory. Does the necessary conversion if
+     * the in-memory type differs from the type used for SSA values.
+     */
+    if ty::type_is_bool(t) {
+        Store(cx, ZExt(cx, v, Type::i8(cx.ccx())), dst);
+    } else {
+        Store(cx, v, dst);
+    };
+}
+
 pub fn ignore_lhs(_bcx: &Block, local: &ast::Local) -> bool {
     match local.pat.node {
         ast::PatWild => true, _ => false
@@ -1285,9 +1317,14 @@ fn copy_args_to_allocas<'a>(fcx: &FunctionContext<'a>,
 // Ties up the llstaticallocas -> llloadenv -> lltop edges,
 // and builds the return block.
 pub fn finish_fn<'a>(fcx: &'a FunctionContext<'a>,
-                     last_bcx: &'a Block<'a>) {
+                     last_bcx: &'a Block<'a>,
+                     retty: ty::t) {
     let _icx = push_ctxt("finish_fn");
 
+    // This shouldn't need to recompute the return type,
+    // as new_fn_ctxt did it already.
+    let substd_retty = retty.substp(fcx.ccx.tcx(), fcx.param_substs);
+
     let ret_cx = match fcx.llreturn.get() {
         Some(llreturn) => {
             if !last_bcx.terminated.get() {
@@ -1297,13 +1334,13 @@ pub fn finish_fn<'a>(fcx: &'a FunctionContext<'a>,
         }
         None => last_bcx
     };
-    build_return_block(fcx, ret_cx);
+    build_return_block(fcx, ret_cx, substd_retty);
     debuginfo::clear_source_location(fcx);
     fcx.cleanup();
 }
 
 // Builds the return block for a function.
-pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block) {
+pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block, retty: ty::t) {
     // Return the value if this function immediate; otherwise, return void.
     if fcx.llretptr.get().is_none() || fcx.caller_expects_out_pointer {
         return RetVoid(ret_cx);
@@ -1321,13 +1358,16 @@ pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block) {
                 retptr.erase_from_parent();
             }
 
-            retval
+            if ty::type_is_bool(retty) {
+                Trunc(ret_cx, retval, Type::i1(fcx.ccx))
+            } else {
+                retval
+            }
         }
         // Otherwise, load the return value from the ret slot
-        None => Load(ret_cx, fcx.llretptr.get().unwrap())
+        None => load_ty(ret_cx, fcx.llretptr.get().unwrap(), retty)
     };
 
-
     Ret(ret_cx, retval);
 }
 
@@ -1429,7 +1469,7 @@ pub fn trans_closure(ccx: &CrateContext,
     }
 
     // Insert the mandatory first few basic blocks before lltop.
-    finish_fn(&fcx, bcx);
+    finish_fn(&fcx, bcx, output_type);
 }
 
 // trans_fn: creates an LLVM function corresponding to a source language
@@ -1521,7 +1561,7 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext,
         }
     }
 
-    finish_fn(&fcx, bcx);
+    finish_fn(&fcx, bcx, result_ty);
 }
 
 fn trans_enum_def(ccx: &CrateContext, enum_definition: &ast::EnumDef,

src/librustc/middle/trans/cabi_arm.rs

@@ -85,7 +85,7 @@ fn ty_size(ty: Type) -> uint {
 
 fn classify_ret_ty(ccx: &CrateContext, ty: Type) -> ArgType {
     if is_reg_ty(ty) {
-        let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
         return ArgType::direct(ty, None, None, attr);
     }
     let size = ty_size(ty);
@@ -104,7 +104,7 @@ fn classify_ret_ty(ccx: &CrateContext, ty: Type) -> ArgType {
 
 fn classify_arg_ty(ccx: &CrateContext, ty: Type) -> ArgType {
     if is_reg_ty(ty) {
-        let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
         return ArgType::direct(ty, None, None, attr);
     }
     let align = ty_align(ty);

src/librustc/middle/trans/cabi_mips.rs

@@ -85,7 +85,7 @@ fn ty_size(ty: Type) -> uint {
 
 fn classify_ret_ty(ccx: &CrateContext, ty: Type) -> ArgType {
     if is_reg_ty(ty) {
-        let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
         ArgType::direct(ty, None, None, attr)
     } else {
         ArgType::indirect(ty, Some(StructRetAttribute))
@@ -102,7 +102,7 @@ fn classify_arg_ty(ccx: &CrateContext, ty: Type, offset: &mut uint) -> ArgType {
     *offset += align_up_to(size, align * 8) / 8;
 
     if is_reg_ty(ty) {
-        let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
         ArgType::direct(ty, None, None, attr)
     } else {
         ArgType::direct(

src/librustc/middle/trans/cabi_x86.rs

@@ -59,7 +59,7 @@ pub fn compute_abi_info(ccx: &CrateContext,
             }
         }
     } else {
-        let attr = if rty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        let attr = if rty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
         ret_ty = ArgType::direct(rty, None, None, attr);
     }
 
@@ -74,7 +74,7 @@ pub fn compute_abi_info(ccx: &CrateContext,
                 }
             }
             _ => {
-                let attr = if t == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+                let attr = if t == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
                 ArgType::direct(t, None, None, attr)
             }
         };

src/librustc/middle/trans/cabi_x86_64.rs

@@ -350,7 +350,7 @@ pub fn compute_abi_info(ccx: &CrateContext,
                                 None)
             }
         } else {
-            let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+            let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
             ArgType::direct(ty, None, None, attr)
         }
     }

src/librustc/middle/trans/callee.rs

@@ -346,7 +346,7 @@ pub fn trans_unboxing_shim(bcx: &Block,
                            }).bcx;
 
     bcx = fcx.pop_and_trans_custom_cleanup_scope(bcx, arg_scope);
-    finish_fn(&fcx, bcx);
+    finish_fn(&fcx, bcx, return_type);
 
     llfn
 }
@@ -758,7 +758,7 @@ pub fn trans_call_inner<'a>(
                 if !type_of::return_uses_outptr(bcx.ccx(), ret_ty) &&
                     !type_is_zero_size(bcx.ccx(), ret_ty)
                 {
-                    Store(bcx, llret, llretslot);
+                    store_ty(bcx, llret, llretslot, ret_ty)
                 }
             }
             None => {}

src/librustc/middle/trans/datum.rs

@@ -13,10 +13,8 @@
  * Datums are and how they are intended to be used.
  */
 
-use lib;
 use lib::llvm::ValueRef;
 use middle::trans::base::*;
-use middle::trans::build::*;
 use middle::trans::common::*;
 use middle::trans::cleanup;
 use middle::trans::cleanup::CleanupMethods;
@@ -344,7 +342,7 @@ impl Datum<Rvalue> {
                 match self.kind.mode {
                     ByValue => DatumBlock::new(bcx, self),
                     ByRef => {
-                        let llval = load(bcx, self.val, self.ty);
+                        let llval = load_ty(bcx, self.val, self.ty);
                         DatumBlock::new(bcx, Datum::new(llval, self.ty, Rvalue::new(ByValue)))
                     }
                 }
@@ -471,7 +469,7 @@ impl Datum<Expr> {
                         DatumBlock::new(bcx, scratch)
                     }
                     ByValue => {
-                        let v = load(bcx, l.val, l.ty);
+                        let v = load_ty(bcx, l.val, l.ty);
                         bcx = l.kind.post_store(bcx, l.val, l.ty);
                         DatumBlock::new(bcx, Datum::new(v, l.ty, Rvalue::new(ByValue)))
                     }
@@ -516,24 +514,6 @@ impl Datum<Lvalue> {
     }
 }
 
-fn load<'a>(bcx: &'a Block<'a>, llptr: ValueRef, ty: ty::t) -> ValueRef {
-    /*!
-     * Private helper for loading from a by-ref datum. Handles various
-     * special cases where the type gives us better information about
-     * what we are loading.
-     */
-
-    if type_is_zero_size(bcx.ccx(), ty) {
-        C_undef(type_of::type_of(bcx.ccx(), ty))
-    } else if ty::type_is_char(ty) {
-        // a char is a unicode codepoint, and so takes values from 0
-        // to 0x10FFFF inclusive only.
-        LoadRangeAssert(bcx, llptr, 0, 0x10FFFF + 1, lib::llvm::False)
-    } else {
-        Load(bcx, llptr)
-    }
-}
-
 /**
  * Generic methods applicable to any sort of datum.
  */
@@ -591,7 +571,7 @@ impl<K:KindOps> Datum<K> {
         if self.kind.is_by_ref() {
             memcpy_ty(bcx, dst, self.val, self.ty);
         } else {
-            Store(bcx, self.val, dst);
+            store_ty(bcx, self.val, dst, self.ty);
         }
 
         return bcx;
@@ -642,7 +622,7 @@ impl<K:KindOps> Datum<K> {
         assert!(!ty::type_needs_drop(bcx.tcx(), self.ty));
         assert!(self.appropriate_rvalue_mode(bcx.ccx()) == ByValue);
         if self.kind.is_by_ref() {
-            load(bcx, self.val, self.ty)
+            load_ty(bcx, self.val, self.ty)
         } else {
             self.val
         }

src/librustc/middle/trans/expr.rs

@@ -1374,7 +1374,7 @@ fn trans_lazy_binop<'a>(
     }
 
     Br(past_rhs, join.llbb);
-    let phi = Phi(join, Type::bool(bcx.ccx()), [lhs, rhs],
+    let phi = Phi(join, Type::i1(bcx.ccx()), [lhs, rhs],
                   [past_lhs.llbb, past_rhs.llbb]);
 
     return immediate_rvalue_bcx(join, phi, binop_ty).to_expr_datumblock();
@@ -1591,8 +1591,8 @@ fn trans_imm_cast<'a>(bcx: &'a Block<'a>,
     let k_in = cast_type_kind(t_in);
     let k_out = cast_type_kind(t_out);
     let s_in = k_in == cast_integral && ty::type_is_signed(t_in);
-    let ll_t_in = type_of::type_of(ccx, t_in);
-    let ll_t_out = type_of::type_of(ccx, t_out);
+    let ll_t_in = type_of::arg_type_of(ccx, t_in);
+    let ll_t_out = type_of::arg_type_of(ccx, t_out);
 
     // Convert the value to be cast into a ValueRef, either by-ref or
     // by-value as appropriate given its type:
@@ -1683,7 +1683,7 @@ fn trans_assign_op<'a>(
     let dst_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, dst, "assign_op"));
     assert!(!ty::type_needs_drop(bcx.tcx(), dst_datum.ty));
     let dst_ty = dst_datum.ty;
-    let dst = Load(bcx, dst_datum.val);
+    let dst = load_ty(bcx, dst_datum.val, dst_datum.ty);
 
     // Evaluate RHS
     let rhs_datum = unpack_datum!(bcx, trans(bcx, &*src));

src/librustc/middle/trans/foreign.rs

@@ -325,7 +325,7 @@ pub fn trans_native_call<'a>(
                 base::alloca(bcx,
                              type_of::type_of(ccx, *passed_arg_tys.get(i)),
                              "__arg");
-            Store(bcx, llarg_rust, scratch);
+            base::store_ty(bcx, llarg_rust, scratch, *passed_arg_tys.get(i));
             llarg_rust = scratch;
         }
 
@@ -346,7 +346,12 @@ pub fn trans_native_call<'a>(
         let llarg_foreign = if foreign_indirect {
             llarg_rust
         } else {
-            Load(bcx, llarg_rust)
+            if ty::type_is_bool(*passed_arg_tys.get(i)) {
+                let val = LoadRangeAssert(bcx, llarg_rust, 0, 2, lib::llvm::False);
+                Trunc(bcx, val, Type::i1(bcx.ccx()))
+            } else {
+                Load(bcx, llarg_rust)
+            }
         };
 
         debug!("argument {}, llarg_foreign={}",
@@ -431,7 +436,7 @@ pub fn trans_native_call<'a>(
         debug!("llforeign_ret_ty={}", ccx.tn.type_to_str(llforeign_ret_ty));
 
         if llrust_ret_ty == llforeign_ret_ty {
-            Store(bcx, llforeign_retval, llretptr);
+            base::store_ty(bcx, llforeign_retval, llretptr, fn_sig.output)
         } else {
             // The actual return type is a struct, but the ABI
             // adaptation code has cast it into some scalar type.  The
@@ -715,9 +720,15 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: &CrateContext,
             // pointer).  It makes adapting types easier, since we can
             // always just bitcast pointers.
             if !foreign_indirect {
-                let lltemp = builder.alloca(val_ty(llforeign_arg), "");
-                builder.store(llforeign_arg, lltemp);
-                llforeign_arg = lltemp;
+                llforeign_arg = if ty::type_is_bool(rust_ty) {
+                    let lltemp = builder.alloca(Type::bool(ccx), "");
+                    builder.store(builder.zext(llforeign_arg, Type::bool(ccx)), lltemp);
+                    lltemp
+                } else {
+                    let lltemp = builder.alloca(val_ty(llforeign_arg), "");
+                    builder.store(llforeign_arg, lltemp);
+                    lltemp
+                }
             }
 
             // If the types in the ABI and the Rust types don't match,
@@ -731,7 +742,12 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: &CrateContext,
             let llrust_arg = if rust_indirect {
                 llforeign_arg
             } else {
-                builder.load(llforeign_arg)
+                if ty::type_is_bool(rust_ty) {
+                    let tmp = builder.load_range_assert(llforeign_arg, 0, 2, lib::llvm::False);
+                    builder.trunc(tmp, Type::i1(ccx))
+                } else {
+                    builder.load(llforeign_arg)
+                }
             };
 
             debug!("llrust_arg {}{}: {}", "#",
@@ -828,8 +844,8 @@ fn foreign_signature(ccx: &CrateContext, fn_sig: &ty::FnSig, arg_tys: &[ty::t])
      * values by pointer like we do.
      */
 
-    let llarg_tys = arg_tys.iter().map(|&arg| type_of(ccx, arg)).collect();
-    let llret_ty = type_of::type_of(ccx, fn_sig.output);
+    let llarg_tys = arg_tys.iter().map(|&arg| arg_type_of(ccx, arg)).collect();
+    let llret_ty = type_of::arg_type_of(ccx, fn_sig.output);
     LlvmSignature {
         llarg_tys: llarg_tys,
         llret_ty: llret_ty