Skip to content

Pass around i1 as is instead of going through u8 #50277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Pass around i1 as is instead of going through u8 #50277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librustc_trans/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
let llreturn_ty = match self.ret.mode {
PassMode::Ignore => Type::void(cx),
PassMode::Direct(_) | PassMode::Pair(..) => {
self.ret.layout.immediate_llvm_type(cx)
self.ret.layout.llvm_type(cx)
}
PassMode::Cast(cast) => cast.llvm_type(cx),
PassMode::Indirect(_) => {
Expand All @@ -575,7 +575,7 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {

let llarg_ty = match arg.mode {
PassMode::Ignore => continue,
PassMode::Direct(_) => arg.layout.immediate_llvm_type(cx),
PassMode::Direct(_) => arg.layout.llvm_type(cx),
PassMode::Pair(..) => {
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0));
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1));
Expand Down
19 changes: 1 addition & 18 deletions src/librustc_trans/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use rustc::middle::weak_lang_items;
use rustc::mir::mono::{Linkage, Visibility, Stats};
use rustc::middle::cstore::{EncodedMetadata};
use rustc::ty::{self, Ty, TyCtxt};
use rustc::ty::layout::{self, Align, TyLayout, LayoutOf};
use rustc::ty::layout::{Align, TyLayout, LayoutOf};
use rustc::ty::maps::Providers;
use rustc::dep_graph::{DepNode, DepConstructor};
use rustc::ty::subst::Kind;
Expand Down Expand Up @@ -387,23 +387,6 @@ pub fn call_assume<'a, 'tcx>(bx: &Builder<'a, 'tcx>, val: ValueRef) {
bx.call(assume_intrinsic, &[val], None);
}

pub fn from_immediate(bx: &Builder, val: ValueRef) -> ValueRef {
if val_ty(val) == Type::i1(bx.cx) {
bx.zext(val, Type::i8(bx.cx))
} else {
val
}
}

pub fn to_immediate(bx: &Builder, val: ValueRef, layout: layout::TyLayout) -> ValueRef {
if let layout::Abi::Scalar(ref scalar) = layout.abi {
if scalar.is_bool() {
return bx.trunc(val, Type::i1(bx.cx));
}
}
val
}

pub fn call_memcpy(bx: &Builder,
dst: ValueRef,
src: ValueRef,
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_trans/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
unsafe {
llvm::LLVMSetAlignment(load, cx.align_of(tp_ty).abi() as u32);
}
to_immediate(bx, load, cx.layout_of(tp_ty))
load
},
"volatile_store" => {
let tp_ty = substs.type_at(0);
Expand All @@ -259,7 +259,7 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
if dst.layout.is_zst() {
return;
}
from_immediate(bx, args[1].immediate())
args[1].immediate()
};
let ptr = bx.pointercast(dst.llval, val_ty(val).ptr_to());
let store = bx.volatile_store(val, ptr);
Expand Down Expand Up @@ -556,7 +556,7 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
let val = if let OperandValue::Ref(ptr, align) = args[1].val {
bx.load(ptr, align)
} else {
from_immediate(bx, args[1].immediate())
args[1].immediate()
};
let ptr = bx.pointercast(dst.llval, val_ty(val).ptr_to());
let store = bx.nontemporal_store(val, ptr);
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_trans/mir/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
let (otherwise, targets) = targets.split_last().unwrap();
let switch = bx.switch(discr.immediate(),
llblock(self, *otherwise), values.len());
let switch_llty = bx.cx.layout_of(switch_ty).immediate_llvm_type(bx.cx);
let switch_llty = bx.cx.layout_of(switch_ty).llvm_type(bx.cx);
for (&value, target) in values.iter().zip(targets) {
let llval = C_uint_big(switch_llty, value);
let llbb = llblock(self, *target);
Expand Down Expand Up @@ -651,8 +651,6 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
bx.range_metadata(llval, 0..2);
}
}
// We store bools as i8 so we need to truncate to i1.
llval = base::to_immediate(bx, llval, arg.layout);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/mir/constant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
};
Ok(primval_to_llvm(
bx.cx, prim, scalar,
layout.immediate_llvm_type(bx.cx),
layout.llvm_type(bx.cx),
))
},
other => bug!("simd shuffle field {:?}, {}", other, constant.ty),
Expand Down
12 changes: 6 additions & 6 deletions src/librustc_trans/mir/operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl<'a, 'tcx> OperandRef<'tcx> {
layout: TyLayout<'tcx>) -> OperandRef<'tcx> {
assert!(layout.is_zst());
OperandRef {
val: OperandValue::Immediate(C_undef(layout.immediate_llvm_type(cx))),
val: OperandValue::Immediate(C_undef(layout.llvm_type(cx))),
layout
}
}
Expand All @@ -113,7 +113,7 @@ impl<'a, 'tcx> OperandRef<'tcx> {
bx.cx,
x,
scalar,
layout.immediate_llvm_type(bx.cx),
layout.llvm_type(bx.cx),
);
OperandValue::Immediate(llval)
},
Expand Down Expand Up @@ -226,7 +226,7 @@ impl<'a, 'tcx> OperandRef<'tcx> {
// If we're uninhabited, or the field is ZST, it has no data.
_ if self.layout.abi == layout::Abi::Uninhabited || field.is_zst() => {
return OperandRef {
val: OperandValue::Immediate(C_undef(field.immediate_llvm_type(bx.cx))),
val: OperandValue::Immediate(C_undef(field.llvm_type(bx.cx))),
layout: field
};
}
Expand Down Expand Up @@ -263,7 +263,7 @@ impl<'a, 'tcx> OperandRef<'tcx> {
// HACK(eddyb) have to bitcast pointers until LLVM removes pointee types.
match val {
OperandValue::Immediate(ref mut llval) => {
*llval = bx.bitcast(*llval, field.immediate_llvm_type(bx.cx));
*llval = bx.bitcast(*llval, field.llvm_type(bx.cx));
}
OperandValue::Pair(ref mut a, ref mut b) => {
*a = bx.bitcast(*a, field.scalar_pair_element_llvm_type(bx.cx, 0));
Expand Down Expand Up @@ -292,7 +292,7 @@ impl<'a, 'tcx> OperandValue {
base::memcpy_ty(bx, dest.llval, r, dest.layout,
source_align.min(dest.align)),
OperandValue::Immediate(s) => {
bx.store(base::from_immediate(bx, s), dest.llval, dest.align);
bx.store(s, dest.llval, dest.align);
}
OperandValue::Pair(a, b) => {
for (i, &x) in [a, b].iter().enumerate() {
Expand All @@ -301,7 +301,7 @@ impl<'a, 'tcx> OperandValue {
if common::val_ty(x) == Type::i1(bx.cx) {
llptr = bx.pointercast(llptr, Type::i8p(bx.cx));
}
bx.store(base::from_immediate(bx, x), llptr, dest.align);
bx.store(x, llptr, dest.align);
}
}
}
Expand Down
22 changes: 7 additions & 15 deletions src/librustc_trans/mir/place.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl<'a, 'tcx> PlaceRef<'tcx> {
let scalar_load_metadata = |load, scalar: &layout::Scalar| {
let vr = scalar.valid_range.clone();
match scalar.value {
layout::Int(..) => {
layout::Int(..) if !scalar.is_bool() => {
let range = scalar.valid_range_exclusive(bx.cx);
if range.start != range.end {
bx.range_metadata(load, range);
Expand Down Expand Up @@ -124,21 +124,13 @@ impl<'a, 'tcx> PlaceRef<'tcx> {
}
load
};
OperandValue::Immediate(base::to_immediate(bx, llval, self.layout))
OperandValue::Immediate(llval)
} else if let layout::Abi::ScalarPair(ref a, ref b) = self.layout.abi {
let load = |i, scalar: &layout::Scalar| {
let mut llptr = bx.struct_gep(self.llval, i as u64);
// Make sure to always load i1 as i8.
if scalar.is_bool() {
llptr = bx.pointercast(llptr, Type::i8p(bx.cx));
}
let llptr = bx.struct_gep(self.llval, i as u64);
let load = bx.load(llptr, self.align);
scalar_load_metadata(load, scalar);
if scalar.is_bool() {
bx.trunc(load, Type::i1(bx.cx))
} else {
load
}
load
};
OperandValue::Pair(load(0, a), load(1, b))
} else {
Expand Down Expand Up @@ -254,7 +246,7 @@ impl<'a, 'tcx> PlaceRef<'tcx> {

/// Obtain the actual discriminant of a value.
pub fn trans_get_discr(self, bx: &Builder<'a, 'tcx>, cast_to: Ty<'tcx>) -> ValueRef {
let cast_to = bx.cx.layout_of(cast_to).immediate_llvm_type(bx.cx);
let cast_to = bx.cx.layout_of(cast_to).llvm_type(bx.cx);
if self.layout.abi == layout::Abi::Uninhabited {
return C_undef(cast_to);
}
Expand Down Expand Up @@ -286,7 +278,7 @@ impl<'a, 'tcx> PlaceRef<'tcx> {
niche_start,
..
} => {
let niche_llty = discr.layout.immediate_llvm_type(bx.cx);
let niche_llty = discr.layout.llvm_type(bx.cx);
if niche_variants.start() == niche_variants.end() {
// FIXME(eddyb) Check the actual primitive type here.
let niche_llval = if niche_start == 0 {
Expand Down Expand Up @@ -351,7 +343,7 @@ impl<'a, 'tcx> PlaceRef<'tcx> {
}

let niche = self.project_field(bx, 0);
let niche_llty = niche.layout.immediate_llvm_type(bx.cx);
let niche_llty = niche.layout.llvm_type(bx.cx);
let niche_value = ((variant_index - *niche_variants.start()) as u128)
.wrapping_add(niche_start);
// FIXME(eddyb) Check the actual primitive type here.
Expand Down
12 changes: 7 additions & 5 deletions src/librustc_trans/mir/rvalue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {

let start = dest.project_index(&bx, C_usize(bx.cx, 0)).llval;

if let OperandValue::Immediate(v) = tr_elem.val {
if let OperandValue::Immediate(mut v) = tr_elem.val {
let align = C_i32(bx.cx, dest.align.abi() as i32);
let size = C_usize(bx.cx, dest.layout.size.bytes());

Expand All @@ -113,7 +113,9 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
}

// Use llvm.memset.p0i8.* to initialize byte arrays
let v = base::from_immediate(&bx, v);
if common::val_ty(v) == Type::i1(bx.cx) {
v = bx.zext(v, Type::i8(bx.cx))
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am not sure about this, but I feel like boolean arrays should also use memset when they can, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.

if common::val_ty(v) == Type::i8(bx.cx) {
base::call_memset(&bx, start, v, size, align, false);
return bx;
Expand Down Expand Up @@ -256,7 +258,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
} else { // cast to thin-ptr
// Cast of fat-ptr to thin-ptr is an extraction of data-ptr and
// pointer-cast of that pointer to desired pointer type.
let llcast_ty = cast.immediate_llvm_type(bx.cx);
let llcast_ty = cast.llvm_type(bx.cx);
let llval = bx.pointercast(data_ptr, llcast_ty);
OperandValue::Immediate(llval)
}
Expand All @@ -266,7 +268,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
}
mir::CastKind::Misc => {
assert!(cast.is_llvm_immediate());
let ll_t_out = cast.immediate_llvm_type(bx.cx);
let ll_t_out = cast.llvm_type(bx.cx);
if operand.layout.abi == layout::Abi::Uninhabited {
return (bx, OperandRef {
val: OperandValue::Immediate(C_undef(ll_t_out)),
Expand All @@ -276,7 +278,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
let r_t_in = CastTy::from_ty(operand.layout.ty)
.expect("bad input type for cast");
let r_t_out = CastTy::from_ty(cast.ty).expect("bad output type for cast");
let ll_t_in = operand.layout.immediate_llvm_type(bx.cx);
let ll_t_in = operand.layout.llvm_type(bx.cx);
match operand.layout.variants {
layout::Variants::Single { index } => {
if let Some(def) = operand.layout.ty.ty_adt_def() {
Expand Down
20 changes: 3 additions & 17 deletions src/librustc_trans/type_of.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ pub trait LayoutLlvmExt<'tcx> {
fn is_llvm_immediate(&self) -> bool;
fn is_llvm_scalar_pair<'a>(&self) -> bool;
fn llvm_type<'a>(&self, cx: &CodegenCx<'a, 'tcx>) -> Type;
fn immediate_llvm_type<'a>(&self, cx: &CodegenCx<'a, 'tcx>) -> Type;
fn scalar_llvm_type_at<'a>(&self, cx: &CodegenCx<'a, 'tcx>,
scalar: &layout::Scalar, offset: Size) -> Type;
fn scalar_pair_element_llvm_type<'a>(&self, cx: &CodegenCx<'a, 'tcx>,
Expand Down Expand Up @@ -244,6 +243,9 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> {
/// that field and ensuring the struct has the right alignment.
fn llvm_type<'a>(&self, cx: &CodegenCx<'a, 'tcx>) -> Type {
if let layout::Abi::Scalar(ref scalar) = self.abi {
if scalar.is_bool() {
return Type::i1(cx);
}
// Use a different cache for scalars because pointers to DSTs
// can be either fat or thin (data pointers of fat pointers).
if let Some(&llty) = cx.scalar_lltypes.borrow().get(&self.ty) {
Expand Down Expand Up @@ -310,15 +312,6 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> {
llty
}

fn immediate_llvm_type<'a>(&self, cx: &CodegenCx<'a, 'tcx>) -> Type {
if let layout::Abi::Scalar(ref scalar) = self.abi {
if scalar.is_bool() {
return Type::i1(cx);
}
}
self.llvm_type(cx)
}

fn scalar_llvm_type_at<'a>(&self, cx: &CodegenCx<'a, 'tcx>,
scalar: &layout::Scalar, offset: Size) -> Type {
match scalar.value {
Expand Down Expand Up @@ -359,13 +352,6 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> {
};
let scalar = [a, b][index];

// Make sure to return the same type `immediate_llvm_type` would,
// to avoid dealing with two types and the associated conversions.
// This means that `(bool, bool)` is represented as `{i1, i1}`,
// both in memory and as an immediate, while `bool` is typically
// `i8` in memory and only `i1` when immediate. While we need to
// load/store `bool` as `i8` to avoid crippling LLVM optimizations,
// `i1` in a LLVM aggregate is valid and mostly equivalent to `i8`.
if scalar.is_bool() {
return Type::i1(cx);
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/codegen/function-arguments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub fn enum_id_1(x: Option<Result<u16, u16>>) -> Option<Result<u16, u16>> {
x
}

// CHECK: i16 @enum_id_2(i16)
// CHECK: { i1, i8 } @enum_id_2(i1 zeroext %x.0, i8 %x.1)
#[no_mangle]
pub fn enum_id_2(x: Option<u8>) -> Option<u8> {
x
Expand Down