mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

auto merge of : dotdash/rust/i1_bool, r=alexcrichton

Browse Source 
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.

First, we have to fix some bugs concerning the handling of
attributes in foreign function declarations and calls. These
are required because the i1 type needs the ZExt attribute when
used as a function parameter or return type.

Then we have to update LLVM to get a bugfix without which LLVM
sometimes generates broken code when using i1.

And then, finally, we can switch bools over to i1.
This commit is contained in:
bors 2014-06-22 00:01:34 +00:00
commit 4c39962d32
19 changed files with 155 additions and 99 deletions

14
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"),
},

11
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));

8
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);

12
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)
};

8
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);
}

9
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 @@ pub fn compute_abi_info(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)
}
}

5
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,

13
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) }

5
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)
}
}

15
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);

63
src/librustc/middle/trans/foreign.rs

@ -11,7 +11,7 @@
use back::{link};
use lib::llvm::llvm;
use lib::llvm::{ValueRef, CallConv, StructRetAttribute, Linkage};
use lib::llvm::{ValueRef, CallConv, Linkage};
use lib;
use middle::weak_lang_items;
use middle::trans::base::push_ctxt;
@ -373,18 +373,41 @@ pub fn trans_native_call<'a>(
};
// A function pointer is called without the declaration available, so we have to apply
// any attributes with ABI implications directly to the call instruction. Right now, the
// only attribute we need to worry about is `sret`.
// any attributes with ABI implications directly to the call instruction.
let mut attrs = Vec::new();
if fn_type.ret_ty.is_indirect() {
attrs.push((1, lib::llvm::StructRetAttribute as u64));
// Add attributes that are always applicable, independent of the concrete foreign ABI
if fn_type.ret_ty.is_indirect() {
// The outptr can be noalias and nocapture because it's entirely
// invisible to the program. We can also mark it as nonnull
attrs.push((1, lib::llvm::NoAliasAttribute as u64));
attrs.push((1, lib::llvm::NoCaptureAttribute as u64));
attrs.push((1, lib::llvm::NonNullAttribute as u64));
};
// Add attributes that depend on the concrete foreign ABI
let mut arg_idx = if fn_type.ret_ty.is_indirect() { 1 } else { 0 };
match fn_type.ret_ty.attr {
Some(attr) => attrs.push((arg_idx, attr as u64)),
_ => ()
}
arg_idx += 1;
for arg_ty in fn_type.arg_tys.iter() {
if arg_ty.is_ignore() {
continue;
}
// skip padding
if arg_ty.pad.is_some() { arg_idx += 1; }
match arg_ty.attr {
Some(attr) => attrs.push((arg_idx, attr as u64)),
_ => {}
}
arg_idx += 1;
}
let llforeign_retval = CallWithConv(bcx,
llfn,
llargs_foreign.as_slice(),
@ -934,22 +957,17 @@ pub fn lltype_for_foreign_fn(ccx: &CrateContext, ty: ty::t) -> Type {
fn add_argument_attributes(tys: &ForeignTypes,
llfn: ValueRef) {
let mut i = 0;
let mut i = if tys.fn_ty.ret_ty.is_indirect() { 1 } else { 0 };
if tys.fn_ty.ret_ty.is_indirect() {
match tys.fn_ty.ret_ty.attr {
Some(attr) => {
let llarg = get_param(llfn, i);
unsafe {
llvm::LLVMAddAttribute(llarg, attr as c_uint);
}
}
None => {}
}
i += 1;
match tys.fn_ty.ret_ty.attr {
Some(attr) => unsafe {
llvm::LLVMAddFunctionAttribute(llfn, i as c_uint, attr as u64);
},
None => {}
}
i += 1;
for &arg_ty in tys.fn_ty.arg_tys.iter() {
if arg_ty.is_ignore() {
continue;
@ -958,12 +976,9 @@ fn add_argument_attributes(tys: &ForeignTypes,
if arg_ty.pad.is_some() { i += 1; }
match arg_ty.attr {
Some(attr) => {
let llarg = get_param(llfn, i);
unsafe {
llvm::LLVMAddAttribute(llarg, attr as c_uint);
}
}
Some(attr) => unsafe {
llvm::LLVMAddFunctionAttribute(llfn, i as c_uint, attr as u64);
},
None => ()
}

18
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);
}
}
@ -235,11 +229,15 @@ pub fn trans_intrinsic(ccx: &CrateContext,
lib::llvm::SequentiallyConsistent =>
lib::llvm::SequentiallyConsistent,
};
let old = AtomicCmpXchg(bcx, get_param(decl, first_real_arg),
let res = AtomicCmpXchg(bcx, get_param(decl, first_real_arg),
get_param(decl, first_real_arg + 1u),
get_param(decl, first_real_arg + 2u),
order, strongest_failure_ordering);
Ret(bcx, old);
if unsafe { lib::llvm::llvm::LLVMVersionMinor() >= 5 } {
Ret(bcx, ExtractValue(bcx, res, 0));
} else {
Ret(bcx, res);
}
}
"load" => {
let old = AtomicLoad(bcx, get_param(decl, first_real_arg),

1
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

2
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 {

2
src/llvm

@ -1 +1 @@
Subproject commit 0a894645cf120539876e9eb4eb0d7b572dfa9d14
Subproject commit 1bba09755d95892bc826c558630e93803b0a4ee6

18
src/rt/rust_test_helpers.c

@ -199,3 +199,21 @@ void
rust_dbg_static_mut_check_four() {
assert(rust_dbg_static_mut == 4);
}
struct S {
uint64_t x;
uint64_t y;
uint64_t z;
};
uint64_t get_x(struct S s) {
return s.x;
}
uint64_t get_y(struct S s) {
return s.y;
}
uint64_t get_z(struct S s) {
return s.z;
}

12
src/rustllvm/RustWrapper.cpp

@ -659,7 +659,7 @@ LLVMRustLinkInExternalBitcode(LLVMModuleRef dst, char *bc, size_t len) {
extern "C" void*
LLVMRustOpenArchive(char *path) {
std::unique_ptr<MemoryBuffer> buf;
error_code err = MemoryBuffer::getFile(path, buf);
std::error_code err = MemoryBuffer::getFile(path, buf);
if (err) {
LLVMRustSetLastError(err.message().c_str());
return NULL;
@ -694,14 +694,18 @@ LLVMRustArchiveReadSection(Archive *ar, char *name, size_t *size) {
#if LLVM_VERSION_MINOR >= 5
Archive::child_iterator child = ar->child_begin(),
end = ar->child_end();
for (; child != end; ++child) {
ErrorOr<StringRef> name_or_err = child->getName();
if (name_or_err.getError()) continue;
StringRef sect_name = name_or_err.get();
#else
Archive::child_iterator child = ar->begin_children(),
end = ar->end_children();
#endif
for (; child != end; ++child) {
StringRef sect_name;
error_code err = child->getName(sect_name);
if (err) continue;
#endif
if (sect_name.trim(" ") == name) {
StringRef buf = child->getBuffer();
*size = buf.size();
@ -757,7 +761,11 @@ inline section_iterator *unwrap(LLVMSectionIteratorRef SI) {
extern "C" int
LLVMRustGetSectionName(LLVMSectionIteratorRef SI, const char **ptr) {
StringRef ret;
#if LLVM_VERSION_MINOR >= 5
if (std::error_code ec = (*unwrap(SI))->getName(ret))
#else
if (error_code ec = (*unwrap(SI))->getName(ret))
#endif
report_fatal_error(ec.message());
*ptr = ret.data();
return ret.size();

2
src/rustllvm/llvm-auto-clean-trigger

@ -1,4 +1,4 @@
# If this file is modified, then llvm will be forcibly cleaned and then rebuilt.
# The actual contents of this file do not matter, but to trigger a change on the
# build bots then the contents should be changed so git updates the mtime.
2014-05-20
2014-06-20.2

36
src/test/run-pass/foreign-fn-with-byval.rs Normal file

@ -0,0 +1,36 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
pub struct S {
x: u64,
y: u64,
z: u64,
}
#[link(name = "rust_test_helpers")]
extern {
pub fn get_x(x: S) -> u64;
pub fn get_y(x: S) -> u64;
pub fn get_z(x: S) -> u64;
}
#[inline(never)]
fn indirect_call(func: unsafe extern fn(s: S) -> u64, s: S) -> u64 {
unsafe {
func(s)
}
}
fn main() {
let s = S { x: 1, y: 2, z: 3 };
assert_eq!(s.x, indirect_call(get_x, s));
assert_eq!(s.y, indirect_call(get_y, s));
assert_eq!(s.z, indirect_call(get_z, s));
}