Compile bools to i1

We currently compiled bools to i8 values, because there was a bug in
LLVM that sometimes caused miscompilations when using i1 in, for
example, structs.

Using i8 means a lot of unnecessary zero-extend and truncate operations
though, since we have to convert the value from and to i1 when using for
example icmp or br instructions. Besides the unnecessary overhead caused
by this, it also sometimes made LLVM miss some optimizations.

Fixes #8106.

Author: dotdash

src/librustc/middle/trans/_match.rs

@@ -1200,8 +1200,6 @@ fn pick_col(m: &[Match]) -> uint {
 pub enum branch_kind { no_branch, single, switch, compare, compare_vec_len, }
 
 // Compiles a comparison between two things.
-//
-// NB: This must produce an i1, not a Rust bool (i8).
 fn compare_values<'a>(
                   cx: &'a Block<'a>,
                   lhs: ValueRef,
@@ -1218,11 +1216,7 @@ fn compare_values<'a>(
                            format!("comparison of `{}`",
                                    cx.ty_to_str(rhs_t)).as_slice(),
                            StrEqFnLangItem);
-        let result = callee::trans_lang_call(cx, did, [lhs, rhs], None);
-        Result {
-            bcx: result.bcx,
-            val: bool_to_i1(result.bcx, result.val)
-        }
+        callee::trans_lang_call(cx, did, [lhs, rhs], None)
     }
 
     let _icx = push_ctxt("compare_values");
@@ -1243,11 +1237,7 @@ fn compare_values<'a>(
                                    format!("comparison of `{}`",
                                            cx.ty_to_str(rhs_t)).as_slice(),
                                    UniqStrEqFnLangItem);
-                let result = callee::trans_lang_call(cx, did, [scratch_lhs, scratch_rhs], None);
-                Result {
-                    bcx: result.bcx,
-                    val: bool_to_i1(result.bcx, result.val)
-                }
+                callee::trans_lang_call(cx, did, [scratch_lhs, scratch_rhs], None)
             }
             _ => cx.sess().bug("only strings supported in compare_values"),
         },

src/librustc/middle/trans/base.rs

@@ -501,7 +501,6 @@ pub fn maybe_name_value(cx: &CrateContext, v: ValueRef, s: &str) {
 // Used only for creating scalar comparison glue.
 pub enum scalar_type { nil_type, signed_int, unsigned_int, floating_point, }
 
-// NB: This produces an i1, not a Rust bool (i8).
 pub fn compare_scalar_types<'a>(
                             cx: &'a Block<'a>,
                             lhs: ValueRef,
@@ -1815,6 +1814,13 @@ pub fn get_fn_llvm_attributes(ccx: &CrateContext, fn_ty: ty::t) -> Vec<(uint, u6
             }
             _ => {}
         }
+
+        match ty::get(ret_ty).sty {
+            ty::ty_bool => {
+                attrs.push((lib::llvm::ReturnIndex as uint, lib::llvm::ZExtAttribute as u64));
+            }
+            _ => {}
+        }
     }
 
     for (idx, &t) in fn_sig.inputs.iter().enumerate().map(|(i, v)| (i + first_arg_offset, v)) {
@@ -1828,6 +1834,9 @@ pub fn get_fn_llvm_attributes(ccx: &CrateContext, fn_ty: ty::t) -> Vec<(uint, u6
                 attrs.push((idx, lib::llvm::NoCaptureAttribute as u64));
                 attrs.push((idx, lib::llvm::NonNullAttribute as u64));
             }
+            ty::ty_bool => {
+                attrs.push((idx, lib::llvm::ZExtAttribute as u64));
+            }
             // `~` pointer parameters never alias because ownership is transferred
             ty::ty_uniq(_) => {
                 attrs.push((idx, lib::llvm::NoAliasAttribute as u64));

src/librustc/middle/trans/cabi_arm.rs

@@ -11,7 +11,7 @@
 #![allow(non_uppercase_pattern_statics)]
 
 use lib::llvm::{llvm, Integer, Pointer, Float, Double, Struct, Array};
-use lib::llvm::StructRetAttribute;
+use lib::llvm::{StructRetAttribute, ZExtAttribute};
 use middle::trans::cabi::{FnType, ArgType};
 use middle::trans::context::CrateContext;
 use middle::trans::type_::Type;
@@ -85,7 +85,8 @@ fn ty_size(ty: Type) -> uint {
 
 fn classify_ret_ty(ccx: &CrateContext, ty: Type) -> ArgType {
     if is_reg_ty(ty) {
-        return ArgType::direct(ty, None, None, None);
+        let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        return ArgType::direct(ty, None, None, attr);
     }
     let size = ty_size(ty);
     if size <= 4 {
@@ -103,7 +104,8 @@ fn classify_ret_ty(ccx: &CrateContext, ty: Type) -> ArgType {
 
 fn classify_arg_ty(ccx: &CrateContext, ty: Type) -> ArgType {
     if is_reg_ty(ty) {
-        return ArgType::direct(ty, None, None, None);
+        let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        return ArgType::direct(ty, None, None, attr);
     }
     let align = ty_align(ty);
     let size = ty_size(ty);

src/librustc/middle/trans/cabi_mips.rs

@@ -13,7 +13,7 @@
 use libc::c_uint;
 use std::cmp;
 use lib::llvm::{llvm, Integer, Pointer, Float, Double, Struct, Array};
-use lib::llvm::StructRetAttribute;
+use lib::llvm::{StructRetAttribute, ZExtAttribute};
 use middle::trans::context::CrateContext;
 use middle::trans::cabi::*;
 use middle::trans::type_::Type;
@@ -83,9 +83,10 @@ fn ty_size(ty: Type) -> uint {
     }
 }
 
-fn classify_ret_ty(ty: Type) -> ArgType {
+fn classify_ret_ty(ccx: &CrateContext, ty: Type) -> ArgType {
     if is_reg_ty(ty) {
-        ArgType::direct(ty, None, None, None)
+        let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        ArgType::direct(ty, None, None, attr)
     } else {
         ArgType::indirect(ty, Some(StructRetAttribute))
     }
@@ -101,7 +102,8 @@ 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) {
-        ArgType::direct(ty, None, None, None)
+        let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        ArgType::direct(ty, None, None, attr)
     } else {
         ArgType::direct(
             ty,
@@ -160,7 +162,7 @@ pub fn compute_abi_info(ccx: &CrateContext,
                         rty: Type,
                         ret_def: bool) -> FnType {
     let ret_ty = if ret_def {
-        classify_ret_ty(rty)
+        classify_ret_ty(ccx, rty)
     } else {
         ArgType::direct(Type::void(ccx), None, None, None)
     };

src/librustc/middle/trans/cabi_x86.rs

@@ -59,7 +59,8 @@ pub fn compute_abi_info(ccx: &CrateContext,
             }
         }
     } else {
-        ret_ty = ArgType::direct(rty, None, None, None);
+        let attr = if rty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+        ret_ty = ArgType::direct(rty, None, None, attr);
     }
 
     for &t in atys.iter() {
@@ -72,7 +73,10 @@ pub fn compute_abi_info(ccx: &CrateContext,
                     ArgType::indirect(t, Some(ByValAttribute))
                 }
             }
-            _ => ArgType::direct(t, None, None, None),
+            _ => {
+                let attr = if t == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+                ArgType::direct(t, None, None, attr)
+            }
         };
         arg_tys.push(ty);
     }

src/librustc/middle/trans/cabi_x86_64.rs

@@ -15,7 +15,7 @@
 
 use lib::llvm::{llvm, Integer, Pointer, Float, Double};
 use lib::llvm::{Struct, Array, Attribute};
-use lib::llvm::{StructRetAttribute, ByValAttribute};
+use lib::llvm::{StructRetAttribute, ByValAttribute, ZExtAttribute};
 use middle::trans::cabi::*;
 use middle::trans::context::CrateContext;
 use middle::trans::type_::Type;
@@ -337,20 +337,21 @@ pub fn compute_abi_info(ccx: &CrateContext,
     fn x86_64_ty(ccx: &CrateContext,
                  ty: Type,
                  is_mem_cls: |cls: &[RegClass]| -> bool,
-                 attr: Attribute)
+                 ind_attr: Attribute)
                  -> ArgType {
         if !ty.is_reg_ty() {
             let cls = classify_ty(ty);
             if is_mem_cls(cls.as_slice()) {
-                ArgType::indirect(ty, Some(attr))
+                ArgType::indirect(ty, Some(ind_attr))
             } else {
                 ArgType::direct(ty,
                                 Some(llreg_ty(ccx, cls.as_slice())),
                                 None,
                                 None)
             }
         } else {
-            ArgType::direct(ty, None, None, None)
+            let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
+            ArgType::direct(ty, None, None, attr)
         }
     }
 

src/librustc/middle/trans/common.rs

@@ -818,11 +818,6 @@ pub fn find_vtable(tcx: &ty::ctxt,
     param_bounds.get(n_bound).clone()
 }
 
-// Casts a Rust bool value to an i1.
-pub fn bool_to_i1(bcx: &Block, llval: ValueRef) -> ValueRef {
-    build::ICmp(bcx, lib::llvm::IntNE, llval, C_bool(bcx.ccx(), false))
-}
-
 pub fn langcall(bcx: &Block,
                 span: Option<Span>,
                 msg: &str,

src/librustc/middle/trans/consts.rs

@@ -399,18 +399,7 @@ fn const_expr_unadjusted(cx: &CrateContext, e: &ast::Expr,
                 let (dv, _dt) = const_deref(cx, te, ty, true);
                 dv
               }
-              ast::UnNot    => {
-                match ty::get(ty).sty {
-                    ty::ty_bool => {
-                        // Somewhat questionable, but I believe this is
-                        // correct.
-                        let te = llvm::LLVMConstTrunc(te, Type::i1(cx).to_ref());
-                        let te = llvm::LLVMConstNot(te);
-                        llvm::LLVMConstZExt(te, Type::bool(cx).to_ref())
-                    }
-                    _ => llvm::LLVMConstNot(te),
-                }
-              }
+              ast::UnNot    => llvm::LLVMConstNot(te),
               ast::UnNeg    => {
                 if is_float { llvm::LLVMConstFNeg(te) }
                 else        { llvm::LLVMConstNeg(te) }

src/librustc/middle/trans/datum.rs

@@ -525,8 +525,6 @@ fn load<'a>(bcx: &'a Block<'a>, llptr: ValueRef, ty: ty::t) -> ValueRef {
 
     if type_is_zero_size(bcx.ccx(), ty) {
         C_undef(type_of::type_of(bcx.ccx(), ty))
-    } else if ty::type_is_bool(ty) {
-        LoadRangeAssert(bcx, llptr, 0, 2, lib::llvm::False)
     } else if ty::type_is_char(ty) {
         // a char is a unicode codepoint, and so takes values from 0
         // to 0x10FFFF inclusive only.
@@ -652,8 +650,7 @@ impl<K:KindOps> Datum<K> {
 
     pub fn to_llbool<'a>(self, bcx: &'a Block<'a>) -> ValueRef {
         assert!(ty::type_is_bool(self.ty) || ty::type_is_bot(self.ty))
-        let cond_val = self.to_llscalarish(bcx);
-        bool_to_i1(bcx, cond_val)
+        self.to_llscalarish(bcx)
     }
 }
 

src/librustc/middle/trans/expr.rs

@@ -1147,11 +1147,7 @@ fn trans_unary<'a>(bcx: &'a Block<'a>,
             let datum = unpack_datum!(bcx, trans(bcx, sub_expr));
             let llresult = if ty::type_is_bool(un_ty) {
                 let val = datum.to_llscalarish(bcx);
-                let llcond = ICmp(bcx,
-                                  lib::llvm::IntEQ,
-                                  val,
-                                  C_bool(ccx, false));
-                Select(bcx, llcond, C_bool(ccx, true), C_bool(ccx, false))
+                Xor(bcx, val, C_bool(ccx, true))
             } else {
                 // Note: `Not` is bitwise, not suitable for logical not.
                 Not(bcx, datum.to_llscalarish(bcx))
@@ -1325,9 +1321,7 @@ fn trans_eager_binop<'a>(
         if ty::type_is_bot(rhs_t) {
             C_bool(bcx.ccx(), false)
         } else if ty::type_is_scalar(rhs_t) {
-            let cmpr = base::compare_scalar_types(bcx, lhs, rhs, rhs_t, op);
-            bcx = cmpr.bcx;
-            ZExt(bcx, cmpr.val, Type::i8(bcx.ccx()))
+            unpack_result!(bcx, base::compare_scalar_types(bcx, lhs, rhs, rhs_t, op))
         } else if is_simd {
             base::compare_simd_types(bcx, lhs, rhs, intype, ty::simd_size(tcx, lhs_t), op)
         } else {
@@ -1369,10 +1363,9 @@ fn trans_lazy_binop<'a>(
     let join = fcx.new_id_block("join", binop_expr.id);
     let before_rhs = fcx.new_id_block("before_rhs", b.id);
 
-    let lhs_i1 = bool_to_i1(past_lhs, lhs);
     match op {
-      lazy_and => CondBr(past_lhs, lhs_i1, before_rhs.llbb, join.llbb),
-      lazy_or => CondBr(past_lhs, lhs_i1, join.llbb, before_rhs.llbb)
+      lazy_and => CondBr(past_lhs, lhs, before_rhs.llbb, join.llbb),
+      lazy_or => CondBr(past_lhs, lhs, join.llbb, before_rhs.llbb)
     }
 
     let DatumBlock {bcx: past_rhs, datum: rhs} = trans(before_rhs, b);

src/librustc/middle/trans/intrinsic.rs

@@ -96,19 +96,13 @@ pub fn trans_intrinsic(ccx: &CrateContext,
         let b = get_param(bcx.fcx.llfn, first_real_arg + 1);
         let llfn = bcx.ccx().get_intrinsic(&name);
 
-        // convert `i1` to a `bool`, and write to the out parameter
         let val = Call(bcx, llfn, [a, b], []);
-        let result = ExtractValue(bcx, val, 0);
-        let overflow = ZExt(bcx, ExtractValue(bcx, val, 1), Type::bool(bcx.ccx()));
-        let ret = C_undef(type_of::type_of(bcx.ccx(), t));
-        let ret = InsertValue(bcx, ret, result, 0);
-        let ret = InsertValue(bcx, ret, overflow, 1);
 
         if type_is_immediate(bcx.ccx(), t) {
-            Ret(bcx, ret);
+            Ret(bcx, val);
         } else {
             let retptr = get_param(bcx.fcx.llfn, bcx.fcx.out_arg_pos());
-            Store(bcx, ret, retptr);
+            Store(bcx, val, retptr);
             RetVoid(bcx);
         }
     }

src/librustc/middle/trans/reflect.rs

@@ -107,7 +107,6 @@ impl<'a, 'b> Reflector<'a, 'b> {
                                                          mth_idx,
                                                          v),
             ArgVals(args), None));
-        let result = bool_to_i1(bcx, result);
         let next_bcx = fcx.new_temp_block("next");
         CondBr(bcx, result, next_bcx.llbb, self.final_bcx.llbb);
         self.bcx = next_bcx

src/librustc/middle/trans/type_.rs

@@ -93,7 +93,7 @@ impl Type {
     }
 
     pub fn bool(ccx: &CrateContext) -> Type {
-        Type::i8(ccx)
+        Type::i1(ccx)
     }
 
     pub fn char(ccx: &CrateContext) -> Type {