Skip to content

Commit dd4112b

Browse files
dotdashdotdash
committed
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 rust-lang#15203
1 parent d2a22f5 commit dd4112b

15 files changed

+115
-65
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -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: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -959,10 +959,42 @@ pub fn need_invoke(bcx: &Block) -> bool {
959959

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

966+
pub fn load_ty(cx: &Block, ptr: ValueRef, t: ty::t) -> ValueRef {
967+
/*!
968+
* Helper for loading values from memory. Does the necessary conversion if
969+
* the in-memory type differs from the type used for SSA values. Also
970+
* handles various special cases where the type gives us better information
971+
* about what we are loading.
972+
*/
973+
if type_is_zero_size(cx.ccx(), t) {
974+
C_undef(type_of::type_of(cx.ccx(), t))
975+
} else if ty::type_is_bool(t) {
976+
Trunc(cx, LoadRangeAssert(cx, ptr, 0, 2, lib::llvm::False), Type::i1(cx.ccx()))
977+
} else if ty::type_is_char(t) {
978+
// a char is a unicode codepoint, and so takes values from 0
979+
// to 0x10FFFF inclusive only.
980+
LoadRangeAssert(cx, ptr, 0, 0x10FFFF + 1, lib::llvm::False)
981+
} else {
982+
Load(cx, ptr)
983+
}
984+
}
985+
986+
pub fn store_ty(cx: &Block, v: ValueRef, dst: ValueRef, t: ty::t) {
987+
/*!
988+
* Helper for storing values in memory. Does the necessary conversion if
989+
* the in-memory type differs from the type used for SSA values.
990+
*/
991+
if ty::type_is_bool(t) {
992+
Store(cx, ZExt(cx, v, Type::i8(cx.ccx())), dst);
993+
} else {
994+
Store(cx, v, dst);
995+
};
996+
}
997+
966998
pub fn ignore_lhs(_bcx: &Block, local: &ast::Local) -> bool {
967999
match local.pat.node {
9681000
ast::PatWild => true, _ => false
@@ -1285,9 +1317,14 @@ fn copy_args_to_allocas<'a>(fcx: &FunctionContext<'a>,
12851317
// Ties up the llstaticallocas -> llloadenv -> lltop edges,
12861318
// and builds the return block.
12871319
pub fn finish_fn<'a>(fcx: &'a FunctionContext<'a>,
1288-
last_bcx: &'a Block<'a>) {
1320+
last_bcx: &'a Block<'a>,
1321+
retty: ty::t) {
12891322
let _icx = push_ctxt("finish_fn");
12901323

1324+
// This shouldn't need to recompute the return type,
1325+
// as new_fn_ctxt did it already.
1326+
let substd_retty = retty.substp(fcx.ccx.tcx(), fcx.param_substs);
1327+
12911328
let ret_cx = match fcx.llreturn.get() {
12921329
Some(llreturn) => {
12931330
if !last_bcx.terminated.get() {
@@ -1297,13 +1334,13 @@ pub fn finish_fn<'a>(fcx: &'a FunctionContext<'a>,
12971334
}
12981335
None => last_bcx
12991336
};
1300-
build_return_block(fcx, ret_cx);
1337+
build_return_block(fcx, ret_cx, substd_retty);
13011338
debuginfo::clear_source_location(fcx);
13021339
fcx.cleanup();
13031340
}
13041341

13051342
// Builds the return block for a function.
1306-
pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block) {
1343+
pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block, retty: ty::t) {
13071344
// Return the value if this function immediate; otherwise, return void.
13081345
if fcx.llretptr.get().is_none() || fcx.caller_expects_out_pointer {
13091346
return RetVoid(ret_cx);
@@ -1321,13 +1358,16 @@ pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block) {
13211358
retptr.erase_from_parent();
13221359
}
13231360

1324-
retval
1361+
if ty::type_is_bool(retty) {
1362+
Trunc(ret_cx, retval, Type::i1(fcx.ccx))
1363+
} else {
1364+
retval
1365+
}
13251366
}
13261367
// Otherwise, load the return value from the ret slot
1327-
None => Load(ret_cx, fcx.llretptr.get().unwrap())
1368+
None => load_ty(ret_cx, fcx.llretptr.get().unwrap(), retty)
13281369
};
13291370

1330-
13311371
Ret(ret_cx, retval);
13321372
}
13331373

@@ -1429,7 +1469,7 @@ pub fn trans_closure(ccx: &CrateContext,
14291469
}
14301470

14311471
// Insert the mandatory first few basic blocks before lltop.
1432-
finish_fn(&fcx, bcx);
1472+
finish_fn(&fcx, bcx, output_type);
14331473
}
14341474

14351475
// 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,
15211561
}
15221562
}
15231563

1524-
finish_fn(&fcx, bcx);
1564+
finish_fn(&fcx, bcx, result_ty);
15251565
}
15261566

15271567
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
@@ -346,7 +346,7 @@ pub fn trans_unboxing_shim(bcx: &Block,
346346
}).bcx;
347347

348348
bcx = fcx.pop_and_trans_custom_cleanup_scope(bcx, arg_scope);
349-
finish_fn(&fcx, bcx);
349+
finish_fn(&fcx, bcx, return_type);
350350

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

‎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: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,7 +1374,7 @@ fn trans_lazy_binop<'a>(
13741374
}
13751375

13761376
Br(past_rhs, join.llbb);
1377-
let phi = Phi(join, Type::bool(bcx.ccx()), [lhs, rhs],
1377+
let phi = Phi(join, Type::i1(bcx.ccx()), [lhs, rhs],
13781378
[past_lhs.llbb, past_rhs.llbb]);
13791379

13801380
return immediate_rvalue_bcx(join, phi, binop_ty).to_expr_datumblock();
@@ -1591,8 +1591,8 @@ fn trans_imm_cast<'a>(bcx: &'a Block<'a>,
15911591
let k_in = cast_type_kind(t_in);
15921592
let k_out = cast_type_kind(t_out);
15931593
let s_in = k_in == cast_integral && ty::type_is_signed(t_in);
1594-
let ll_t_in = type_of::type_of(ccx, t_in);
1595-
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);
15961596

15971597
// Convert the value to be cast into a ValueRef, either by-ref or
15981598
// by-value as appropriate given its type:
@@ -1683,7 +1683,7 @@ fn trans_assign_op<'a>(
16831683
let dst_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, dst, "assign_op"));
16841684
assert!(!ty::type_needs_drop(bcx.tcx(), dst_datum.ty));
16851685
let dst_ty = dst_datum.ty;
1686-
let dst = Load(bcx, dst_datum.val);
1686+
let dst = load_ty(bcx, dst_datum.val, dst_datum.ty);
16871687

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

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

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ pub fn trans_native_call<'a>(
325325
base::alloca(bcx,
326326
type_of::type_of(ccx, *passed_arg_tys.get(i)),
327327
"__arg");
328-
Store(bcx, llarg_rust, scratch);
328+
base::store_ty(bcx, llarg_rust, scratch, *passed_arg_tys.get(i));
329329
llarg_rust = scratch;
330330
}
331331

@@ -346,7 +346,12 @@ pub fn trans_native_call<'a>(
346346
let llarg_foreign = if foreign_indirect {
347347
llarg_rust
348348
} else {
349-
Load(bcx, llarg_rust)
349+
if ty::type_is_bool(*passed_arg_tys.get(i)) {
350+
let val = LoadRangeAssert(bcx, llarg_rust, 0, 2, lib::llvm::False);
351+
Trunc(bcx, val, Type::i1(bcx.ccx()))
352+
} else {
353+
Load(bcx, llarg_rust)
354+
}
350355
};
351356

352357
debug!("argument {}, llarg_foreign={}",
@@ -431,7 +436,7 @@ pub fn trans_native_call<'a>(
431436
debug!("llforeign_ret_ty={}", ccx.tn.type_to_str(llforeign_ret_ty));
432437

433438
if llrust_ret_ty == llforeign_ret_ty {
434-
Store(bcx, llforeign_retval, llretptr);
439+
base::store_ty(bcx, llforeign_retval, llretptr, fn_sig.output)
435440
} else {
436441
// The actual return type is a struct, but the ABI
437442
// adaptation code has cast it into some scalar type. The
@@ -715,9 +720,15 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: &CrateContext,
715720
// pointer). It makes adapting types easier, since we can
716721
// always just bitcast pointers.
717722
if !foreign_indirect {
718-
let lltemp = builder.alloca(val_ty(llforeign_arg), "");
719-
builder.store(llforeign_arg, lltemp);
720-
llforeign_arg = lltemp;
723+
llforeign_arg = if ty::type_is_bool(rust_ty) {
724+
let lltemp = builder.alloca(Type::bool(ccx), "");
725+
builder.store(builder.zext(llforeign_arg, Type::bool(ccx)), lltemp);
726+
lltemp
727+
} else {
728+
let lltemp = builder.alloca(val_ty(llforeign_arg), "");
729+
builder.store(llforeign_arg, lltemp);
730+
lltemp
731+
}
721732
}
722733

723734
// 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,
731742
let llrust_arg = if rust_indirect {
732743
llforeign_arg
733744
} else {
734-
builder.load(llforeign_arg)
745+
if ty::type_is_bool(rust_ty) {
746+
let tmp = builder.load_range_assert(llforeign_arg, 0, 2, lib::llvm::False);
747+
builder.trunc(tmp, Type::i1(ccx))
748+
} else {
749+
builder.load(llforeign_arg)
750+
}
735751
};
736752

737753
debug!("llrust_arg {}{}: {}", "#",
@@ -828,8 +844,8 @@ fn foreign_signature(ccx: &CrateContext, fn_sig: &ty::FnSig, arg_tys: &[ty::t])
828844
* values by pointer like we do.
829845
*/
830846

831-
let llarg_tys = arg_tys.iter().map(|&arg| type_of(ccx, arg)).collect();
832-
let llret_ty = type_of::type_of(ccx, fn_sig.output);
847+
let llarg_tys = arg_tys.iter().map(|&arg| arg_type_of(ccx, arg)).collect();
848+
let llret_ty = type_of::arg_type_of(ccx, fn_sig.output);
833849
LlvmSignature {
834850
llarg_tys: llarg_tys,
835851
llret_ty: llret_ty

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ fn trans_struct_drop_flag<'a>(bcx: &'a Block<'a>,
234234
-> &'a Block<'a> {
235235
let repr = adt::represent_type(bcx.ccx(), t);
236236
let drop_flag = adt::trans_drop_flag_ptr(bcx, &*repr, v0);
237-
with_cond(bcx, Load(bcx, drop_flag), |cx| {
237+
with_cond(bcx, load_ty(bcx, drop_flag, ty::mk_bool()), |cx| {
238238
trans_struct_drop(cx, t, v0, dtor_did, class_did, substs)
239239
})
240240
}
@@ -505,7 +505,7 @@ fn make_generic_glue(ccx: &CrateContext,
505505
let bcx = fcx.entry_bcx.borrow().clone().unwrap();
506506
let llrawptr0 = unsafe { llvm::LLVMGetParam(llfn, fcx.arg_pos(0) as c_uint) };
507507
let bcx = helper(bcx, llrawptr0, t);
508-
finish_fn(&fcx, bcx);
508+
finish_fn(&fcx, bcx, ty::mk_nil());
509509

510510
llfn
511511
}

0 commit comments

Comments
 (0)