Skip to content

Commit d9db7f6

Browse files
borsbors
committed
auto merge of #15464 : dotdash/rust/bool_stores, r=pcwalton
LLVM doesn't handle i1 value in allocas/memory very well and skips a number of optimizations if it hits it. So we have to do the same thing that Clang does, using i1 for SSA values, but storing i8 in memory. Fixes #15203.
2 parents 21ef888 + dd4112b commit d9db7f6

File tree

16 files changed

+126
-85
lines changed

16 files changed

+126
-85
lines changed

‎src/librustc/middle/trans/adt.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,7 @@ pub fn trans_case<'a>(bcx: &'a Block<'a>, r: &Repr, discr: Disr)
618618
RawNullablePointer { .. } |
619619
StructWrappedNullablePointer { .. } => {
620620
assert!(discr == 0 || discr == 1);
621-
_match::single_result(Result::new(bcx, C_i1(bcx.ccx(), discr != 0)))
621+
_match::single_result(Result::new(bcx, C_bool(bcx.ccx(), discr != 0)))
622622
}
623623
}
624624
}
@@ -641,7 +641,7 @@ pub fn trans_start_init(bcx: &Block, r: &Repr, val: ValueRef, discr: Disr) {
641641
}
642642
Univariant(ref st, true) => {
643643
assert_eq!(discr, 0);
644-
Store(bcx, C_bool(bcx.ccx(), true),
644+
Store(bcx, C_u8(bcx.ccx(), 1),
645645
GEPi(bcx, val, [0, st.fields.len() - 1]))
646646
}
647647
Univariant(..) => {

‎src/librustc/middle/trans/base.rs

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -538,8 +538,8 @@ pub fn compare_scalar_values<'a>(
538538
// We don't need to do actual comparisons for nil.
539539
// () == () holds but () < () does not.
540540
match op {
541-
ast::BiEq | ast::BiLe | ast::BiGe => return C_i1(cx.ccx(), true),
542-
ast::BiNe | ast::BiLt | ast::BiGt => return C_i1(cx.ccx(), false),
541+
ast::BiEq | ast::BiLe | ast::BiGe => return C_bool(cx.ccx(), true),
542+
ast::BiNe | ast::BiLt | ast::BiGt => return C_bool(cx.ccx(), false),
543543
// refinements would be nice
544544
_ => die(cx)
545545
}
@@ -958,10 +958,42 @@ pub fn need_invoke(bcx: &Block) -> bool {
958958

959959
pub fn load_if_immediate(cx: &Block, v: ValueRef, t: ty::t) -> ValueRef {
960960
let _icx = push_ctxt("load_if_immediate");
961-
if type_is_immediate(cx.ccx(), t) { return Load(cx, v); }
961+
if type_is_immediate(cx.ccx(), t) { return load_ty(cx, v, t); }
962962
return v;
963963
}
964964

965+
pub fn load_ty(cx: &Block, ptr: ValueRef, t: ty::t) -> ValueRef {
966+
/*!
967+
* Helper for loading values from memory. Does the necessary conversion if
968+
* the in-memory type differs from the type used for SSA values. Also
969+
* handles various special cases where the type gives us better information
970+
* about what we are loading.
971+
*/
972+
if type_is_zero_size(cx.ccx(), t) {
973+
C_undef(type_of::type_of(cx.ccx(), t))
974+
} else if ty::type_is_bool(t) {
975+
Trunc(cx, LoadRangeAssert(cx, ptr, 0, 2, lib::llvm::False), Type::i1(cx.ccx()))
976+
} else if ty::type_is_char(t) {
977+
// a char is a unicode codepoint, and so takes values from 0
978+
// to 0x10FFFF inclusive only.
979+
LoadRangeAssert(cx, ptr, 0, 0x10FFFF + 1, lib::llvm::False)
980+
} else {
981+
Load(cx, ptr)
982+
}
983+
}
984+
985+
pub fn store_ty(cx: &Block, v: ValueRef, dst: ValueRef, t: ty::t) {
986+
/*!
987+
* Helper for storing values in memory. Does the necessary conversion if
988+
* the in-memory type differs from the type used for SSA values.
989+
*/
990+
if ty::type_is_bool(t) {
991+
Store(cx, ZExt(cx, v, Type::i8(cx.ccx())), dst);
992+
} else {
993+
Store(cx, v, dst);
994+
};
995+
}
996+
965997
pub fn ignore_lhs(_bcx: &Block, local: &ast::Local) -> bool {
966998
match local.pat.node {
967999
ast::PatWild => true, _ => false
@@ -1013,7 +1045,7 @@ pub fn call_memcpy(cx: &Block, dst: ValueRef, src: ValueRef, n_bytes: ValueRef,
10131045
let dst_ptr = PointerCast(cx, dst, Type::i8p(ccx));
10141046
let size = IntCast(cx, n_bytes, ccx.int_type);
10151047
let align = C_i32(ccx, align as i32);
1016-
let volatile = C_i1(ccx, false);
1048+
let volatile = C_bool(ccx, false);
10171049
Call(cx, memcpy, [dst_ptr, src_ptr, size, align, volatile], []);
10181050
}
10191051

@@ -1058,7 +1090,7 @@ fn memzero(b: &Builder, llptr: ValueRef, ty: Type) {
10581090
let llzeroval = C_u8(ccx, 0);
10591091
let size = machine::llsize_of(ccx, ty);
10601092
let align = C_i32(ccx, llalign_of_min(ccx, ty) as i32);
1061-
let volatile = C_i1(ccx, false);
1093+
let volatile = C_bool(ccx, false);
10621094
b.call(llintrinsicfn, [llptr, llzeroval, size, align, volatile], []);
10631095
}
10641096

@@ -1282,9 +1314,14 @@ fn copy_args_to_allocas<'a>(fcx: &FunctionContext<'a>,
12821314
// Ties up the llstaticallocas -> llloadenv -> lltop edges,
12831315
// and builds the return block.
12841316
pub fn finish_fn<'a>(fcx: &'a FunctionContext<'a>,
1285-
last_bcx: &'a Block<'a>) {
1317+
last_bcx: &'a Block<'a>,
1318+
retty: ty::t) {
12861319
let _icx = push_ctxt("finish_fn");
12871320

1321+
// This shouldn't need to recompute the return type,
1322+
// as new_fn_ctxt did it already.
1323+
let substd_retty = retty.substp(fcx.ccx.tcx(), fcx.param_substs);
1324+
12881325
let ret_cx = match fcx.llreturn.get() {
12891326
Some(llreturn) => {
12901327
if !last_bcx.terminated.get() {
@@ -1294,13 +1331,13 @@ pub fn finish_fn<'a>(fcx: &'a FunctionContext<'a>,
12941331
}
12951332
None => last_bcx
12961333
};
1297-
build_return_block(fcx, ret_cx);
1334+
build_return_block(fcx, ret_cx, substd_retty);
12981335
debuginfo::clear_source_location(fcx);
12991336
fcx.cleanup();
13001337
}
13011338

13021339
// Builds the return block for a function.
1303-
pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block) {
1340+
pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block, retty: ty::t) {
13041341
// Return the value if this function immediate; otherwise, return void.
13051342
if fcx.llretptr.get().is_none() || fcx.caller_expects_out_pointer {
13061343
return RetVoid(ret_cx);
@@ -1318,13 +1355,16 @@ pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block) {
13181355
retptr.erase_from_parent();
13191356
}
13201357

1321-
retval
1358+
if ty::type_is_bool(retty) {
1359+
Trunc(ret_cx, retval, Type::i1(fcx.ccx))
1360+
} else {
1361+
retval
1362+
}
13221363
}
13231364
// Otherwise, load the return value from the ret slot
1324-
None => Load(ret_cx, fcx.llretptr.get().unwrap())
1365+
None => load_ty(ret_cx, fcx.llretptr.get().unwrap(), retty)
13251366
};
13261367

1327-
13281368
Ret(ret_cx, retval);
13291369
}
13301370

@@ -1422,7 +1462,7 @@ pub fn trans_closure(ccx: &CrateContext,
14221462
}
14231463

14241464
// Insert the mandatory first few basic blocks before lltop.
1425-
finish_fn(&fcx, bcx);
1465+
finish_fn(&fcx, bcx, output_type);
14261466
}
14271467

14281468
// trans_fn: creates an LLVM function corresponding to a source language
@@ -1512,7 +1552,7 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext,
15121552
}
15131553
}
15141554

1515-
finish_fn(&fcx, bcx);
1555+
finish_fn(&fcx, bcx, result_ty);
15161556
}
15171557

15181558
fn trans_enum_def(ccx: &CrateContext, enum_definition: &ast::EnumDef,

‎src/librustc/middle/trans/cabi_arm.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ fn ty_size(ty: Type) -> uint {
8585

8686
fn classify_ret_ty(ccx: &CrateContext, ty: Type) -> ArgType {
8787
if is_reg_ty(ty) {
88-
let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
88+
let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
8989
return ArgType::direct(ty, None, None, attr);
9090
}
9191
let size = ty_size(ty);
@@ -104,7 +104,7 @@ fn classify_ret_ty(ccx: &CrateContext, ty: Type) -> ArgType {
104104

105105
fn classify_arg_ty(ccx: &CrateContext, ty: Type) -> ArgType {
106106
if is_reg_ty(ty) {
107-
let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
107+
let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
108108
return ArgType::direct(ty, None, None, attr);
109109
}
110110
let align = ty_align(ty);

‎src/librustc/middle/trans/cabi_mips.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ fn ty_size(ty: Type) -> uint {
8585

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

104104
if is_reg_ty(ty) {
105-
let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
105+
let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
106106
ArgType::direct(ty, None, None, attr)
107107
} else {
108108
ArgType::direct(

‎src/librustc/middle/trans/cabi_x86.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ pub fn compute_abi_info(ccx: &CrateContext,
5959
}
6060
}
6161
} else {
62-
let attr = if rty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
62+
let attr = if rty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
6363
ret_ty = ArgType::direct(rty, None, None, attr);
6464
}
6565

@@ -74,7 +74,7 @@ pub fn compute_abi_info(ccx: &CrateContext,
7474
}
7575
}
7676
_ => {
77-
let attr = if t == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
77+
let attr = if t == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
7878
ArgType::direct(t, None, None, attr)
7979
}
8080
};

‎src/librustc/middle/trans/cabi_x86_64.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ pub fn compute_abi_info(ccx: &CrateContext,
350350
None)
351351
}
352352
} else {
353-
let attr = if ty == Type::bool(ccx) { Some(ZExtAttribute) } else { None };
353+
let attr = if ty == Type::i1(ccx) { Some(ZExtAttribute) } else { None };
354354
ArgType::direct(ty, None, None, attr)
355355
}
356356
}

‎src/librustc/middle/trans/callee.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ pub fn trans_unboxing_shim(bcx: &Block,
345345
}).bcx;
346346

347347
bcx = fcx.pop_and_trans_custom_cleanup_scope(bcx, arg_scope);
348-
finish_fn(&fcx, bcx);
348+
finish_fn(&fcx, bcx, return_type);
349349

350350
llfn
351351
}
@@ -757,7 +757,7 @@ pub fn trans_call_inner<'a>(
757757
if !type_of::return_uses_outptr(bcx.ccx(), ret_ty) &&
758758
!type_is_zero_size(bcx.ccx(), ret_ty)
759759
{
760-
Store(bcx, llret, llretslot);
760+
store_ty(bcx, llret, llretslot, ret_ty)
761761
}
762762
}
763763
None => {}

‎src/librustc/middle/trans/common.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -522,10 +522,6 @@ pub fn C_nil(ccx: &CrateContext) -> ValueRef {
522522
}
523523

524524
pub fn C_bool(ccx: &CrateContext, val: bool) -> ValueRef {
525-
C_integral(Type::bool(ccx), val as u64, false)
526-
}
527-
528-
pub fn C_i1(ccx: &CrateContext, val: bool) -> ValueRef {
529525
C_integral(Type::i1(ccx), val as u64, false)
530526
}
531527

‎src/librustc/middle/trans/datum.rs

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,8 @@
1313
* Datums are and how they are intended to be used.
1414
*/
1515

16-
use lib;
1716
use lib::llvm::ValueRef;
1817
use middle::trans::base::*;
19-
use middle::trans::build::*;
2018
use middle::trans::common::*;
2119
use middle::trans::cleanup;
2220
use middle::trans::cleanup::CleanupMethods;
@@ -344,7 +342,7 @@ impl Datum<Rvalue> {
344342
match self.kind.mode {
345343
ByValue => DatumBlock::new(bcx, self),
346344
ByRef => {
347-
let llval = load(bcx, self.val, self.ty);
345+
let llval = load_ty(bcx, self.val, self.ty);
348346
DatumBlock::new(bcx, Datum::new(llval, self.ty, Rvalue::new(ByValue)))
349347
}
350348
}
@@ -471,7 +469,7 @@ impl Datum<Expr> {
471469
DatumBlock::new(bcx, scratch)
472470
}
473471
ByValue => {
474-
let v = load(bcx, l.val, l.ty);
472+
let v = load_ty(bcx, l.val, l.ty);
475473
bcx = l.kind.post_store(bcx, l.val, l.ty);
476474
DatumBlock::new(bcx, Datum::new(v, l.ty, Rvalue::new(ByValue)))
477475
}
@@ -516,24 +514,6 @@ impl Datum<Lvalue> {
516514
}
517515
}
518516

519-
fn load<'a>(bcx: &'a Block<'a>, llptr: ValueRef, ty: ty::t) -> ValueRef {
520-
/*!
521-
* Private helper for loading from a by-ref datum. Handles various
522-
* special cases where the type gives us better information about
523-
* what we are loading.
524-
*/
525-
526-
if type_is_zero_size(bcx.ccx(), ty) {
527-
C_undef(type_of::type_of(bcx.ccx(), ty))
528-
} else if ty::type_is_char(ty) {
529-
// a char is a unicode codepoint, and so takes values from 0
530-
// to 0x10FFFF inclusive only.
531-
LoadRangeAssert(bcx, llptr, 0, 0x10FFFF + 1, lib::llvm::False)
532-
} else {
533-
Load(bcx, llptr)
534-
}
535-
}
536-
537517
/**
538518
* Generic methods applicable to any sort of datum.
539519
*/
@@ -591,7 +571,7 @@ impl<K:KindOps> Datum<K> {
591571
if self.kind.is_by_ref() {
592572
memcpy_ty(bcx, dst, self.val, self.ty);
593573
} else {
594-
Store(bcx, self.val, dst);
574+
store_ty(bcx, self.val, dst, self.ty);
595575
}
596576

597577
return bcx;
@@ -642,7 +622,7 @@ impl<K:KindOps> Datum<K> {
642622
assert!(!ty::type_needs_drop(bcx.tcx(), self.ty));
643623
assert!(self.appropriate_rvalue_mode(bcx.ccx()) == ByValue);
644624
if self.kind.is_by_ref() {
645-
load(bcx, self.val, self.ty)
625+
load_ty(bcx, self.val, self.ty)
646626
} else {
647627
self.val
648628
}

‎src/librustc/middle/trans/expr.rs

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ fn trans_index<'a>(bcx: &'a Block<'a>,
505505

506506
let bounds_check = ICmp(bcx, lib::llvm::IntUGE, ix_val, len);
507507
let expect = ccx.get_intrinsic(&("llvm.expect.i1"));
508-
let expected = Call(bcx, expect, [bounds_check, C_i1(ccx, false)], []);
508+
let expected = Call(bcx, expect, [bounds_check, C_bool(ccx, false)], []);
509509
let bcx = with_cond(bcx, expected, |bcx| {
510510
controlflow::trans_fail_bounds_check(bcx, index_expr.span, ix_val, len)
511511
});
@@ -1149,13 +1149,7 @@ fn trans_unary<'a>(bcx: &'a Block<'a>,
11491149
match op {
11501150
ast::UnNot => {
11511151
let datum = unpack_datum!(bcx, trans(bcx, sub_expr));
1152-
let llresult = if ty::type_is_bool(un_ty) {
1153-
let val = datum.to_llscalarish(bcx);
1154-
Xor(bcx, val, C_bool(ccx, true))
1155-
} else {
1156-
// Note: `Not` is bitwise, not suitable for logical not.
1157-
Not(bcx, datum.to_llscalarish(bcx))
1158-
};
1152+
let llresult = Not(bcx, datum.to_llscalarish(bcx));
11591153
immediate_rvalue_bcx(bcx, llresult, un_ty).to_expr_datumblock()
11601154
}
11611155
ast::UnNeg => {
@@ -1380,7 +1374,7 @@ fn trans_lazy_binop<'a>(
13801374
}
13811375

13821376
Br(past_rhs, join.llbb);
1383-
let phi = Phi(join, Type::bool(bcx.ccx()), [lhs, rhs],
1377+
let phi = Phi(join, Type::i1(bcx.ccx()), [lhs, rhs],
13841378
[past_lhs.llbb, past_rhs.llbb]);
13851379

13861380
return immediate_rvalue_bcx(join, phi, binop_ty).to_expr_datumblock();
@@ -1597,8 +1591,8 @@ fn trans_imm_cast<'a>(bcx: &'a Block<'a>,
15971591
let k_in = cast_type_kind(t_in);
15981592
let k_out = cast_type_kind(t_out);
15991593
let s_in = k_in == cast_integral && ty::type_is_signed(t_in);
1600-
let ll_t_in = type_of::type_of(ccx, t_in);
1601-
let ll_t_out = type_of::type_of(ccx, t_out);
1594+
let ll_t_in = type_of::arg_type_of(ccx, t_in);
1595+
let ll_t_out = type_of::arg_type_of(ccx, t_out);
16021596

16031597
// Convert the value to be cast into a ValueRef, either by-ref or
16041598
// by-value as appropriate given its type:
@@ -1689,7 +1683,7 @@ fn trans_assign_op<'a>(
16891683
let dst_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, dst, "assign_op"));
16901684
assert!(!ty::type_needs_drop(bcx.tcx(), dst_datum.ty));
16911685
let dst_ty = dst_datum.ty;
1692-
let dst = Load(bcx, dst_datum.val);
1686+
let dst = load_ty(bcx, dst_datum.val, dst_datum.ty);
16931687

16941688
// Evaluate RHS
16951689
let rhs_datum = unpack_datum!(bcx, trans(bcx, &*src));

0 commit comments

Comments
 (0)