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.

diff --git a/src/librustc/middle/trans/_match.rs b/src/librustc/middle/trans/_match.rs
index 5b9c89f6250931f4f8a8713263bebcaeb776e1e4..5fae1635bee8e491dc458670c6856a41a0aa6213 100644 (file)
--- a/src/librustc/middle/trans/_match.rs
+++ b/src/librustc/middle/trans/_match.rs
@@ -1200,8 +1200,6 @@ fn score(p: &ast::Pat) -> 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_str<'a>(cx: &'a Block<'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_str<'a>(cx: &'a Block<'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"),
         },

diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs
index 35795cc9f23ee4d9f926ed7d6f0ff1cd9c041374..da49c7764ce535604d2fc38093007a2419ea5a51 100644 (file)
--- a/src/librustc/middle/trans/base.rs
+++ b/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));

diff --git a/src/librustc/middle/trans/cabi_arm.rs b/src/librustc/middle/trans/cabi_arm.rs
index ee2c6454aeedbc6c71f1dc98ac6b4d01d7dcfcbd..01bef64ebba642bf1fa0347de358191b179049eb 100644 (file)
--- a/src/librustc/middle/trans/cabi_arm.rs
+++ b/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);

diff --git a/src/librustc/middle/trans/cabi_mips.rs b/src/librustc/middle/trans/cabi_mips.rs
index 395bc637aada086672c156641fa65d04a158ce2e..60db609e59ed184cc9b7eac69e711be04643296f 100644 (file)
--- a/src/librustc/middle/trans/cabi_mips.rs
+++ b/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)
     };

diff --git a/src/librustc/middle/trans/cabi_x86.rs b/src/librustc/middle/trans/cabi_x86.rs
index d10f6b72820d29c68b77c9e21e29edb009062728..5fffdf08646b9824b9f8b1846fbe9041002bdcd2 100644 (file)
--- a/src/librustc/middle/trans/cabi_x86.rs
+++ b/src/librustc/middle/trans/cabi_x86.rs
@@ -59,7 +59,8 @@ enum Strategy { RetValue(Type), RetPointer }
             }
         }
     } 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 @@ enum Strategy { RetValue(Type), RetPointer }
                     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);
     }

diff --git a/src/librustc/middle/trans/cabi_x86_64.rs b/src/librustc/middle/trans/cabi_x86_64.rs
index 9f98cf57c09b581de1df33fa28d8c529ff66a771..b2cd9d256dd43c4ea9845a3a74cf84d658433166 100644 (file)
--- a/src/librustc/middle/trans/cabi_x86_64.rs
+++ b/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,12 +337,12 @@ 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())),
@@ -350,7 +350,8 @@ fn x86_64_ty(ccx: &CrateContext,
                                 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)
         }
     }
 

diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs
index d7509866e08862687c60b03c0857edd44ba0aba6..a1923022e7b176dad7326b7000356453f8e6b033 100644 (file)
--- a/src/librustc/middle/trans/common.rs
+++ b/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,

diff --git a/src/librustc/middle/trans/consts.rs b/src/librustc/middle/trans/consts.rs
index 338821537e8c6d098a095a6f63120dd6ccdf6c8d..527ce5dfaae4591d62285d229780ae8137279736 100644 (file)
--- a/src/librustc/middle/trans/consts.rs
+++ b/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) }

diff --git a/src/librustc/middle/trans/datum.rs b/src/librustc/middle/trans/datum.rs
index 158a7d6cf7ab91a61a8026a27719e865cad3469d..440aa36b28cbd2e9e8cb864b0bc9aa044fa27150 100644 (file)
--- a/src/librustc/middle/trans/datum.rs
+++ b/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 @@ pub fn to_llscalarish<'a>(self, bcx: &'a Block<'a>) -> ValueRef {
 
     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)
     }
 }
 

diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs
index 9c957eec90acdef6fb9f8aeb814a21307aab627c..9af5c7aa792358e4021ee7ec2c6c8ffbd6260ce2 100644 (file)
--- a/src/librustc/middle/trans/expr.rs
+++ b/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);

diff --git a/src/librustc/middle/trans/intrinsic.rs b/src/librustc/middle/trans/intrinsic.rs
index 7624a71c9dd35ca9a629eed3a1a568414b23ff8c..bc0c88ceee95fc5866159c067f70773851ad51f5 100644 (file)
--- a/src/librustc/middle/trans/intrinsic.rs
+++ b/src/librustc/middle/trans/intrinsic.rs
@@ -96,19 +96,13 @@ fn with_overflow_instrinsic(bcx: &Block, name: &'static str, t: ty::t) {
         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);
         }
     }

diff --git a/src/librustc/middle/trans/reflect.rs b/src/librustc/middle/trans/reflect.rs
index 506817629565e37dc9dad7539219c4e350dfc8f1..59903324e10eaaf2c5c6b2061eb59df0ff6e659d 100644 (file)
--- a/src/librustc/middle/trans/reflect.rs
+++ b/src/librustc/middle/trans/reflect.rs
@@ -107,7 +107,6 @@ pub fn visit(&mut self, ty_name: &str, args: &[ValueRef]) {
                                                          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

diff --git a/src/librustc/middle/trans/type_.rs b/src/librustc/middle/trans/type_.rs
index 5d58500f761a4581044d0af023814b4c5a4391ee..75f884aac34cf28f58e46ac0eaa0d45ca18a26db 100644 (file)
--- a/src/librustc/middle/trans/type_.rs
+++ b/src/librustc/middle/trans/type_.rs
@@ -93,7 +93,7 @@ pub fn f128(ccx: &CrateContext) -> Type {
     }
 
     pub fn bool(ccx: &CrateContext) -> Type {
-        Type::i8(ccx)
+        Type::i1(ccx)
     }
 
     pub fn char(ccx: &CrateContext) -> Type {