From f3bd7231015706bee72846a758e1234febaaa60a Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Fri, 10 Feb 2017 19:29:39 +0200 Subject: [PATCH] Fix intcast, use it where appropriate --- src/librustc/ty/layout.rs | 18 +++++++++++++++++- src/librustc_llvm/ffi.rs | 10 +++++----- src/librustc_trans/adt.rs | 2 +- src/librustc_trans/base.rs | 2 +- src/librustc_trans/builder.rs | 4 ++-- src/librustc_trans/mir/rvalue.rs | 14 ++------------ src/rustllvm/RustWrapper.cpp | 6 ++++++ 7 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 74f2692629c..6a2fdc6aab6 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -1310,7 +1310,6 @@ impl<'a, 'gcx, 'tcx> Layout { let discr_max = (variants.len() - 1) as i64; assert!(discr_max >= 0); let (min_ity, _) = Integer::repr_discr(tcx, ty, &hints[..], 0, discr_max); - let mut align = dl.aggregate_align; let mut size = Size::from_bytes(0); @@ -1351,6 +1350,23 @@ impl<'a, 'gcx, 'tcx> Layout { return Err(LayoutError::SizeOverflow(ty)); } + let typeck_ity = Integer::from_attr(dl, def.discr_ty); + if typeck_ity < min_ity { + // It is a bug if Layout decided on a greater discriminant size than typeck for + // some reason at this point (based on values discriminant can take on). Mostly + // because this discriminant will be loaded, and then stored into variable of + // type calculated by typeck. Consider such case (a bug): typeck decided on + // byte-sized discriminant, but layout thinks we need a 16-bit to store all + // discriminant values. That would be a bug, because then, in trans, in order + // to store this 16-bit discriminant into 8-bit sized temporary some of the + // space necessary to represent would have to be discarded (or layout is wrong + // on thinking it needs 16 bits) + bug!("layout decided on a larger discriminant type ({:?}) than typeck ({:?})", + min_ity, typeck_ity); + // However, it is fine to make discr type however large (as an optimisation) + // after this point – we’ll just truncate the value we load in trans. + } + // Check to see if we should use a different type for the // discriminant. We can safely use a type with the same size // as the alignment of the first field of each variant. diff --git a/src/librustc_llvm/ffi.rs b/src/librustc_llvm/ffi.rs index 5273910d1d9..8e2e072b584 100644 --- a/src/librustc_llvm/ffi.rs +++ b/src/librustc_llvm/ffi.rs @@ -1084,11 +1084,11 @@ extern "C" { DestTy: TypeRef, Name: *const c_char) -> ValueRef; - pub fn LLVMBuildIntCast(B: BuilderRef, - Val: ValueRef, - DestTy: TypeRef, - Name: *const c_char) - -> ValueRef; + pub fn LLVMRustBuildIntCast(B: BuilderRef, + Val: ValueRef, + DestTy: TypeRef, + IsSized: bool) + -> ValueRef; pub fn LLVMBuildFPCast(B: BuilderRef, Val: ValueRef, DestTy: TypeRef, diff --git a/src/librustc_trans/adt.rs b/src/librustc_trans/adt.rs index 59f2104ec14..11d3fae8238 100644 --- a/src/librustc_trans/adt.rs +++ b/src/librustc_trans/adt.rs @@ -318,7 +318,7 @@ pub fn trans_get_discr<'a, 'tcx>( }; match cast_to { None => val, - Some(llty) => if is_discr_signed(&l) { bcx.sext(val, llty) } else { bcx.zext(val, llty) } + Some(llty) => bcx.intcast(val, llty, is_discr_signed(&l)) } } diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index ce02c9725d1..41c0eaa52a7 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -536,7 +536,7 @@ pub fn call_memcpy<'a, 'tcx>(b: &Builder<'a, 'tcx>, let memcpy = ccx.get_intrinsic(&key); let src_ptr = b.pointercast(src, Type::i8p(ccx)); let dst_ptr = b.pointercast(dst, Type::i8p(ccx)); - let size = b.intcast(n_bytes, ccx.int_type()); + let size = b.intcast(n_bytes, ccx.int_type(), false); let align = C_i32(ccx, align as i32); let volatile = C_bool(ccx, false); b.call(memcpy, &[dst_ptr, src_ptr, size, align, volatile], None); diff --git a/src/librustc_trans/builder.rs b/src/librustc_trans/builder.rs index 89aaa8b6630..f64e581c177 100644 --- a/src/librustc_trans/builder.rs +++ b/src/librustc_trans/builder.rs @@ -780,10 +780,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - pub fn intcast(&self, val: ValueRef, dest_ty: Type) -> ValueRef { + pub fn intcast(&self, val: ValueRef, dest_ty: Type, is_signed: bool) -> ValueRef { self.count_insn("intcast"); unsafe { - llvm::LLVMBuildIntCast(self.llbuilder, val, dest_ty.to_ref(), noname()) + llvm::LLVMRustBuildIntCast(self.llbuilder, val, dest_ty.to_ref(), is_signed) } } diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index 2eb4ea6cedd..38ee67796c6 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -286,17 +286,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> { let newval = match (r_t_in, r_t_out) { (CastTy::Int(_), CastTy::Int(_)) => { - let srcsz = ll_t_in.int_width(); - let dstsz = ll_t_out.int_width(); - if srcsz == dstsz { - bcx.bitcast(llval, ll_t_out) - } else if srcsz > dstsz { - bcx.trunc(llval, ll_t_out) - } else if signed { - bcx.sext(llval, ll_t_out) - } else { - bcx.zext(llval, ll_t_out) - } + bcx.intcast(llval, ll_t_out, signed) } (CastTy::Float, CastTy::Float) => { let srcsz = ll_t_in.float_width(); @@ -439,7 +429,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> { let discr_ty = rvalue.ty(&*self.mir, bcx.tcx()).unwrap(); let discr_type = type_of::immediate_type_of(bcx.ccx, discr_ty); let discr = adt::trans_get_discr(&bcx, enum_ty, discr_lvalue.llval, - Some(discr_type), true); + discr_lvalue.alignment, Some(discr_type), true); (bcx, OperandRef { val: OperandValue::Immediate(discr), ty: discr_ty diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 27233a8b075..c15a0c3d25a 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -1308,6 +1308,12 @@ extern "C" LLVMRustVisibility LLVMRustGetVisibility(LLVMValueRef V) { return toRust(LLVMGetVisibility(V)); } +// Oh hey, a binding that makes sense for once? (because LLVM’s own do not) +extern "C" LLVMValueRef LLVMRustBuildIntCast(LLVMBuilderRef B, LLVMValueRef Val, + LLVMTypeRef DestTy, bool isSigned) { + return wrap(unwrap(B)->CreateIntCast(unwrap(Val), unwrap(DestTy), isSigned, "")); +} + extern "C" void LLVMRustSetVisibility(LLVMValueRef V, LLVMRustVisibility RustVisibility) { LLVMSetVisibility(V, fromRust(RustVisibility));