From 973756d715f9caaeee44e9387f5c60458ae5c4fc Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 31 Jan 2018 00:23:25 +0200 Subject: rustc_trans: keep LLVM types for trait objects anonymous. --- src/test/codegen/function-arguments.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/test/codegen/function-arguments.rs') diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index f8945a6ee8d..0e98d3f9050 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -122,13 +122,13 @@ pub fn unsafe_slice(_: &[UnsafeInner]) { pub fn str(_: &[u8]) { } -// CHECK: @trait_borrow(%"core::ops::drop::Drop"* nonnull %arg0.0, {}* noalias nonnull readonly %arg0.1) +// CHECK: @trait_borrow({}* nonnull %arg0.0, {}* noalias nonnull readonly %arg0.1) // FIXME #25759 This should also have `nocapture` #[no_mangle] pub fn trait_borrow(_: &Drop) { } -// CHECK: @trait_box(%"core::ops::drop::Drop"* noalias nonnull, {}* noalias nonnull readonly) +// CHECK: @trait_box({}* noalias nonnull, {}* noalias nonnull readonly) #[no_mangle] pub fn trait_box(_: Box) { } -- cgit 1.4.1-3-g733a5 From bda718fd255237167f08198b0fc80ab0d484d58e Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 26 Mar 2018 16:26:03 +0200 Subject: Allow niche-filling dataful variants to be represented as a ScalarPair --- src/librustc/ty/layout.rs | 19 +++++++++++++++---- src/test/codegen/function-arguments.rs | 6 ++++++ 2 files changed, 21 insertions(+), 4 deletions(-) (limited to 'src/test/codegen/function-arguments.rs') diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 029dd6f1fb4..5f9c305d92f 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -1517,10 +1517,21 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { let offset = st[i].fields.offset(field_index) + offset; let size = st[i].size; - let abi = if offset.bytes() == 0 && niche.value.size(dl) == size { - Abi::Scalar(niche.clone()) - } else { - Abi::Aggregate { sized: true } + let abi = match st[i].abi { + Abi::Scalar(_) => Abi::Scalar(niche.clone()), + Abi::ScalarPair(ref first, ref second) => { + // We need to use scalar_unit to reset the + // valid range to the maximal one for that + // primitive, because only the niche is + // guaranteed to be initialised, not the + // other primitive. + if offset.bytes() == 0 { + Abi::ScalarPair(niche.clone(), scalar_unit(second.value)) + } else { + Abi::ScalarPair(scalar_unit(first.value), niche.clone()) + } + } + _ => Abi::Aggregate { sized: true }, }; return Ok(tcx.intern_layout(LayoutDetails { diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index 0e98d3f9050..de302c69056 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -133,6 +133,12 @@ pub fn trait_borrow(_: &Drop) { pub fn trait_box(_: Box) { } +// CHECK: { i8*, i8* } @trait_option(i8* noalias %x.0, i8* %x.1) +#[no_mangle] +pub fn trait_option(x: Option>) -> Option> { + x +} + // CHECK: { [0 x i16]*, [[USIZE]] } @return_slice([0 x i16]* noalias nonnull readonly %x.0, [[USIZE]] %x.1) #[no_mangle] pub fn return_slice(x: &[u16]) -> &[u16] { -- cgit 1.4.1-3-g733a5 From 3ca6ad922eb6d8c3139d961c844a0194eaf58770 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 27 Mar 2018 16:44:03 +0200 Subject: Use ScalarPair for tagged enums --- src/librustc/ty/layout.rs | 81 ++++++++++++++++++++++++++++++---- src/test/codegen/align-struct.rs | 3 +- src/test/codegen/function-arguments.rs | 12 +++++ src/test/codegen/lifetime_start_end.rs | 8 ++-- 4 files changed, 90 insertions(+), 14 deletions(-) (limited to 'src/test/codegen/function-arguments.rs') diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 77e2e9447f1..cfed0839acb 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -1622,7 +1622,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { } // Create the set of structs that represent each variant. - let mut variants = variants.into_iter().enumerate().map(|(i, field_layouts)| { + let mut layout_variants = variants.iter().enumerate().map(|(i, field_layouts)| { let mut st = univariant_uninterned(&field_layouts, &def.repr, StructKind::Prefixed(min_ity.size(), prefix_align))?; st.variants = Variants::Single { index: i }; @@ -1683,7 +1683,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { // Patch up the variants' first few fields. let old_ity_size = min_ity.size(); let new_ity_size = ity.size(); - for variant in &mut variants { + for variant in &mut layout_variants { if variant.abi == Abi::Uninhabited { continue; } @@ -1710,15 +1710,80 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { value: Int(ity, signed), valid_range: (min as u128 & tag_mask)..=(max as u128 & tag_mask), }; - let abi = if tag.value.size(dl) == size { - Abi::Scalar(tag.clone()) - } else { - Abi::Aggregate { sized: true } - }; + let mut abi = Abi::Aggregate { sized: true }; + if tag.value.size(dl) == size { + abi = Abi::Scalar(tag.clone()); + } else if !tag.is_bool() { + // HACK(nox): Blindly using ScalarPair for all tagged enums + // where applicable leads to Option being handled as {i1, i8}, + // which later confuses SROA and some loop optimisations, + // ultimately leading to the repeat-trusted-len test + // failing. We make the trade-off of using ScalarPair only + // for types where the tag isn't a boolean. + let mut common_prim = None; + for (field_layouts, layout_variant) in variants.iter().zip(&layout_variants) { + let offsets = match layout_variant.fields { + FieldPlacement::Arbitrary { ref offsets, .. } => offsets, + _ => bug!(), + }; + let mut fields = field_layouts + .iter() + .zip(offsets) + .filter(|p| !p.0.is_zst()); + let (field, offset) = match (fields.next(), fields.next()) { + (None, None) => continue, + (Some(pair), None) => pair, + _ => { + common_prim = None; + break; + } + }; + let prim = match field.details.abi { + Abi::Scalar(ref scalar) => scalar.value, + _ => { + common_prim = None; + break; + } + }; + if let Some(pair) = common_prim { + // This is pretty conservative. We could go fancier + // by conflating things like i32 and u32, or even + // realising that (u8, u8) could just cohabit with + // u16 or even u32. + if pair != (prim, offset) { + common_prim = None; + break; + } + } else { + common_prim = Some((prim, offset)); + } + } + if let Some((prim, offset)) = common_prim { + let pair = scalar_pair(tag.clone(), scalar_unit(prim)); + let pair_offsets = match pair.fields { + FieldPlacement::Arbitrary { + ref offsets, + ref memory_index + } => { + assert_eq!(memory_index, &[0, 1]); + offsets + } + _ => bug!() + }; + if pair_offsets[0] == Size::from_bytes(0) && + pair_offsets[1] == *offset && + align == pair.align && + size == pair.size { + // We can use `ScalarPair` only when it matches our + // already computed layout (including `#[repr(C)]`). + abi = pair.abi; + } + } + } tcx.intern_layout(LayoutDetails { variants: Variants::Tagged { discr: tag, - variants + variants: layout_variants, }, fields: FieldPlacement::Arbitrary { offsets: vec![Size::from_bytes(0)], diff --git a/src/test/codegen/align-struct.rs b/src/test/codegen/align-struct.rs index 155319cb154..f306608f432 100644 --- a/src/test/codegen/align-struct.rs +++ b/src/test/codegen/align-struct.rs @@ -29,7 +29,6 @@ pub enum Enum4 { A(i32), B(i32), } -// CHECK: %Enum4 = type { [0 x i32], i32, [1 x i32] } // CHECK: %"Enum4::A" = type { [1 x i32], i32, [0 x i32] } pub enum Enum64 { @@ -59,7 +58,7 @@ pub fn nested64(a: Align64, b: i32, c: i32, d: i8) -> Nested64 { // CHECK-LABEL: @enum4 #[no_mangle] pub fn enum4(a: i32) -> Enum4 { -// CHECK: %e4 = alloca %Enum4, align 4 +// CHECK: %e4 = alloca { i32, i32 }, align 4 let e4 = Enum4::A(a); e4 } diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index de302c69056..40a9ea5a181 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -145,6 +145,18 @@ pub fn return_slice(x: &[u16]) -> &[u16] { x } +// CHECK: { i16, i16 } @enum_id_1(i16 %x.0, i16 %x.1) +#[no_mangle] +pub fn enum_id_1(x: Option>) -> Option> { + x +} + +// CHECK: i16 @enum_id_2(i16) +#[no_mangle] +pub fn enum_id_2(x: Option) -> Option { + x +} + // CHECK: noalias i8* @allocator() #[no_mangle] #[allocator] diff --git a/src/test/codegen/lifetime_start_end.rs b/src/test/codegen/lifetime_start_end.rs index 62aa93398ac..ea3f0de5d08 100644 --- a/src/test/codegen/lifetime_start_end.rs +++ b/src/test/codegen/lifetime_start_end.rs @@ -25,16 +25,16 @@ pub fn test() { let b = &Some(a); &b; // keep variable in an alloca -// CHECK: [[S_b:%[0-9]+]] = bitcast %"core::option::Option"** %b to i8* +// CHECK: [[S_b:%[0-9]+]] = bitcast { i32, i32 }** %b to i8* // CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S_b]]) -// CHECK: [[S__4:%[0-9]+]] = bitcast %"core::option::Option"* %_4 to i8* +// CHECK: [[S__4:%[0-9]+]] = bitcast { i32, i32 }* %_4 to i8* // CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S__4]]) -// CHECK: [[E_b:%[0-9]+]] = bitcast %"core::option::Option"** %b to i8* +// CHECK: [[E_b:%[0-9]+]] = bitcast { i32, i32 }** %b to i8* // CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E_b]]) -// CHECK: [[E__4:%[0-9]+]] = bitcast %"core::option::Option"* %_4 to i8* +// CHECK: [[E__4:%[0-9]+]] = bitcast { i32, i32 }* %_4 to i8* // CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E__4]]) } -- cgit 1.4.1-3-g733a5 From 12308139ec76dfa050ed012606495250391aaf74 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 14 May 2018 15:20:51 +0200 Subject: Emit noalias on &mut parameters by default This used to be disabled due to LLVM bugs in the handling of noalias information in conjunction with unwinding. However, according to #31681 all known LLVM bugs have been fixed by LLVM 6.0, so it's probably time to reenable this optimization. Noalias annotations will not be emitted by default if either -C panic=abort (as previously) or LLVM >= 6.0 (new). -Z mutable-noalias=no is left as an escape-hatch to allow debugging problems suspected to stem from this change. --- src/librustc/session/config.rs | 4 ++-- src/librustc_codegen_llvm/type_of.rs | 10 ++++++++-- src/test/codegen/function-arguments.rs | 10 ++++------ 3 files changed, 14 insertions(+), 10 deletions(-) (limited to 'src/test/codegen/function-arguments.rs') diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 2344cd11f66..bcad3fbc841 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1239,8 +1239,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "print the result of the monomorphization collection pass"), mir_opt_level: usize = (1, parse_uint, [TRACKED], "set the MIR optimization level (0-3, default: 1)"), - mutable_noalias: bool = (false, parse_bool, [TRACKED], - "emit noalias metadata for mutable references"), + mutable_noalias: Option = (None, parse_opt_bool, [TRACKED], + "emit noalias metadata for mutable references (default: yes on LLVM >= 6)"), arg_align_attributes: bool = (false, parse_bool, [TRACKED], "emit align metadata for reference arguments"), dump_mir: Option = (None, parse_opt_string, [UNTRACKED], diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs index 5f186e7514e..21436b74731 100644 --- a/src/librustc_codegen_llvm/type_of.rs +++ b/src/librustc_codegen_llvm/type_of.rs @@ -10,6 +10,7 @@ use abi::{FnType, FnTypeExt}; use common::*; +use llvm; use rustc::hir; use rustc::ty::{self, Ty, TypeFoldable}; use rustc::ty::layout::{self, Align, LayoutOf, Size, TyLayout}; @@ -428,8 +429,13 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> { PointerKind::Shared }, hir::MutMutable => { - if cx.tcx.sess.opts.debugging_opts.mutable_noalias || - cx.tcx.sess.panic_strategy() == PanicStrategy::Abort { + // Only emit noalias annotations for LLVM >= 6 or in panic=abort + // mode, as prior versions had many bugs in conjunction with + // unwinding. See also issue #31681. + let mutable_noalias = cx.tcx.sess.opts.debugging_opts.mutable_noalias + .unwrap_or(unsafe { llvm::LLVMRustVersionMajor() >= 6 } + || cx.tcx.sess.panic_strategy() == PanicStrategy::Abort); + if mutable_noalias { PointerKind::UniqueBorrowed } else { PointerKind::Shared diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index 40a9ea5a181..e3fa7a7db39 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -10,6 +10,7 @@ // compile-flags: -C no-prepopulate-passes // ignore-tidy-linelength +// min-llvm-version 6.0 #![crate_type = "lib"] #![feature(custom_attribute)] @@ -52,16 +53,14 @@ pub fn named_borrow<'r>(_: &'r i32) { pub fn unsafe_borrow(_: &UnsafeInner) { } -// CHECK: @mutable_unsafe_borrow(i16* dereferenceable(2) %arg0) +// CHECK: @mutable_unsafe_borrow(i16* noalias dereferenceable(2) %arg0) // ... unless this is a mutable borrow, those never alias -// ... except that there's this LLVM bug that forces us to not use noalias, see #29485 #[no_mangle] pub fn mutable_unsafe_borrow(_: &mut UnsafeInner) { } -// CHECK: @mutable_borrow(i32* dereferenceable(4) %arg0) +// CHECK: @mutable_borrow(i32* noalias dereferenceable(4) %arg0) // FIXME #25759 This should also have `nocapture` -// ... there's this LLVM bug that forces us to not use noalias, see #29485 #[no_mangle] pub fn mutable_borrow(_: &mut i32) { } @@ -103,9 +102,8 @@ pub fn helper(_: usize) { pub fn slice(_: &[u8]) { } -// CHECK: @mutable_slice([0 x i8]* nonnull %arg0.0, [[USIZE]] %arg0.1) +// CHECK: @mutable_slice([0 x i8]* noalias nonnull %arg0.0, [[USIZE]] %arg0.1) // FIXME #25759 This should also have `nocapture` -// ... there's this LLVM bug that forces us to not use noalias, see #29485 #[no_mangle] pub fn mutable_slice(_: &mut [u8]) { } -- cgit 1.4.1-3-g733a5 From e5789765606baebd983bd9b1c870cb8b57a0627b Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Jun 2018 15:47:54 -0700 Subject: Store scalar pair bools as i8 in memory We represent `bool` as `i1` in a `ScalarPair`, unlike other aggregates, to optimize IR for checked operators and the like. With this patch, we still do so when the pair is an immediate value, but we use the `i8` memory type when the value is loaded or stored as an LLVM aggregate. So `(bool, bool)` looks like an `{ i1, i1 }` immediate, but `{ i8, i8 }` in memory. When a pair is a direct function argument, `PassMode::Pair`, it is still passed using the immediate `i1` type, but as a return value it will use the `i8` memory type. Also, `bool`-like` enum tags will now use scalar pairs when possible, where they were previously excluded due to optimization issues. --- src/librustc/ty/layout.rs | 9 ++----- src/librustc_codegen_llvm/abi.rs | 4 ++-- src/librustc_codegen_llvm/base.rs | 15 ++++++++---- src/librustc_codegen_llvm/mir/operand.rs | 23 +++++++++--------- src/librustc_codegen_llvm/mir/place.rs | 6 ++--- src/librustc_codegen_llvm/mir/rvalue.rs | 4 ++-- src/librustc_codegen_llvm/type_of.rs | 25 ++++++++++---------- src/test/codegen/function-arguments.rs | 2 +- src/test/codegen/scalar-pair-bool.rs | 40 ++++++++++++++++++++++++++++++++ 9 files changed, 84 insertions(+), 44 deletions(-) create mode 100644 src/test/codegen/scalar-pair-bool.rs (limited to 'src/test/codegen/function-arguments.rs') diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index a32fdbb285d..faad32a5d99 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -1020,13 +1020,8 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { let mut abi = Abi::Aggregate { sized: true }; if tag.value.size(dl) == size { abi = Abi::Scalar(tag.clone()); - } else if !tag.is_bool() { - // HACK(nox): Blindly using ScalarPair for all tagged enums - // where applicable leads to Option being handled as {i1, i8}, - // which later confuses SROA and some loop optimisations, - // ultimately leading to the repeat-trusted-len test - // failing. We make the trade-off of using ScalarPair only - // for types where the tag isn't a boolean. + } else { + // Try to use a ScalarPair for all tagged enums. let mut common_prim = None; for (field_layouts, layout_variant) in variants.iter().zip(&layout_variants) { let offsets = match layout_variant.fields { diff --git a/src/librustc_codegen_llvm/abi.rs b/src/librustc_codegen_llvm/abi.rs index 6b5baa402b4..4ff31ec7d1c 100644 --- a/src/librustc_codegen_llvm/abi.rs +++ b/src/librustc_codegen_llvm/abi.rs @@ -582,8 +582,8 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> { PassMode::Ignore => continue, PassMode::Direct(_) => arg.layout.immediate_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)); + llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true)); + llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true)); continue; } PassMode::Cast(cast) => cast.llvm_type(cx), diff --git a/src/librustc_codegen_llvm/base.rs b/src/librustc_codegen_llvm/base.rs index a4709739a23..4605b5f3f1d 100644 --- a/src/librustc_codegen_llvm/base.rs +++ b/src/librustc_codegen_llvm/base.rs @@ -265,8 +265,8 @@ pub fn unsize_thin_ptr<'a, 'tcx>( } let (lldata, llextra) = result.unwrap(); // HACK(eddyb) have to bitcast pointers until LLVM removes pointee types. - (bx.bitcast(lldata, dst_layout.scalar_pair_element_llvm_type(bx.cx, 0)), - bx.bitcast(llextra, dst_layout.scalar_pair_element_llvm_type(bx.cx, 1))) + (bx.bitcast(lldata, dst_layout.scalar_pair_element_llvm_type(bx.cx, 0, true)), + bx.bitcast(llextra, dst_layout.scalar_pair_element_llvm_type(bx.cx, 1, true))) } _ => bug!("unsize_thin_ptr: called on bad types"), } @@ -396,9 +396,14 @@ pub fn from_immediate(bx: &Builder, val: ValueRef) -> ValueRef { 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)); - } + return to_immediate_scalar(bx, val, scalar); + } + val +} + +pub fn to_immediate_scalar(bx: &Builder, val: ValueRef, scalar: &layout::Scalar) -> ValueRef { + if scalar.is_bool() { + return bx.trunc(val, Type::i1(bx.cx)); } val } diff --git a/src/librustc_codegen_llvm/mir/operand.rs b/src/librustc_codegen_llvm/mir/operand.rs index 3d3a4400bd8..3069a155d1f 100644 --- a/src/librustc_codegen_llvm/mir/operand.rs +++ b/src/librustc_codegen_llvm/mir/operand.rs @@ -128,13 +128,13 @@ impl<'a, 'tcx> OperandRef<'tcx> { bx.cx, a, a_scalar, - layout.scalar_pair_element_llvm_type(bx.cx, 0), + layout.scalar_pair_element_llvm_type(bx.cx, 0, true), ); let b_llval = scalar_to_llvm( bx.cx, b, b_scalar, - layout.scalar_pair_element_llvm_type(bx.cx, 1), + layout.scalar_pair_element_llvm_type(bx.cx, 1, true), ); OperandValue::Pair(a_llval, b_llval) }, @@ -193,8 +193,8 @@ impl<'a, 'tcx> OperandRef<'tcx> { self, llty); // Reconstruct the immediate aggregate. let mut llpair = C_undef(llty); - llpair = bx.insert_value(llpair, a, 0); - llpair = bx.insert_value(llpair, b, 1); + llpair = bx.insert_value(llpair, base::from_immediate(bx, a), 0); + llpair = bx.insert_value(llpair, base::from_immediate(bx, b), 1); llpair } else { self.immediate() @@ -206,13 +206,14 @@ impl<'a, 'tcx> OperandRef<'tcx> { llval: ValueRef, layout: TyLayout<'tcx>) -> OperandRef<'tcx> { - let val = if layout.is_llvm_scalar_pair() { + let val = if let layout::Abi::ScalarPair(ref a, ref b) = layout.abi { debug!("Operand::from_immediate_or_packed_pair: unpacking {:?} @ {:?}", llval, layout); // Deconstruct the immediate aggregate. - OperandValue::Pair(bx.extract_value(llval, 0), - bx.extract_value(llval, 1)) + let a_llval = base::to_immediate_scalar(bx, bx.extract_value(llval, 0), a); + let b_llval = base::to_immediate_scalar(bx, bx.extract_value(llval, 1), b); + OperandValue::Pair(a_llval, b_llval) } else { OperandValue::Immediate(llval) }; @@ -264,8 +265,8 @@ impl<'a, 'tcx> OperandRef<'tcx> { *llval = bx.bitcast(*llval, field.immediate_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)); - *b = bx.bitcast(*b, field.scalar_pair_element_llvm_type(bx.cx, 1)); + *a = bx.bitcast(*a, field.scalar_pair_element_llvm_type(bx.cx, 0, true)); + *b = bx.bitcast(*b, field.scalar_pair_element_llvm_type(bx.cx, 1, true)); } OperandValue::Ref(..) => bug!() } @@ -308,10 +309,10 @@ impl<'a, 'tcx> OperandValue { } OperandValue::Pair(a, b) => { for (i, &x) in [a, b].iter().enumerate() { - let mut llptr = bx.struct_gep(dest.llval, i as u64); + let llptr = bx.struct_gep(dest.llval, i as u64); // Make sure to always store i1 as i8. if common::val_ty(x) == Type::i1(bx.cx) { - llptr = bx.pointercast(llptr, Type::i8p(bx.cx)); + assert_eq!(common::val_ty(llptr), Type::i8p(bx.cx)); } let val = base::from_immediate(bx, x); bx.store_with_flags(val, llptr, dest.align, flags); diff --git a/src/librustc_codegen_llvm/mir/place.rs b/src/librustc_codegen_llvm/mir/place.rs index 2a1e3980adb..36dcd04b02e 100644 --- a/src/librustc_codegen_llvm/mir/place.rs +++ b/src/librustc_codegen_llvm/mir/place.rs @@ -16,7 +16,7 @@ use rustc::mir::tcx::PlaceTy; use rustc_data_structures::indexed_vec::Idx; use base; use builder::Builder; -use common::{CodegenCx, C_undef, C_usize, C_u8, C_u32, C_uint, C_null, C_uint_big}; +use common::{CodegenCx, C_undef, C_usize, C_u8, C_u32, C_uint, C_null, C_uint_big, val_ty}; use consts; use type_of::LayoutLlvmExt; use type_::Type; @@ -127,10 +127,10 @@ impl<'a, 'tcx> PlaceRef<'tcx> { OperandValue::Immediate(base::to_immediate(bx, llval, self.layout)) } 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); + let 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)); + assert_eq!(val_ty(llptr), Type::i8p(bx.cx)); } let load = bx.load(llptr, self.align); scalar_load_metadata(load, scalar); diff --git a/src/librustc_codegen_llvm/mir/rvalue.rs b/src/librustc_codegen_llvm/mir/rvalue.rs index 0fd81c6074e..2e81fc16a58 100644 --- a/src/librustc_codegen_llvm/mir/rvalue.rs +++ b/src/librustc_codegen_llvm/mir/rvalue.rs @@ -232,7 +232,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> { // HACK(eddyb) have to bitcast pointers // until LLVM removes pointee types. let lldata = bx.pointercast(lldata, - cast.scalar_pair_element_llvm_type(bx.cx, 0)); + cast.scalar_pair_element_llvm_type(bx.cx, 0, true)); OperandValue::Pair(lldata, llextra) } OperandValue::Immediate(lldata) => { @@ -251,7 +251,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> { if let OperandValue::Pair(data_ptr, meta) = operand.val { if cast.is_llvm_scalar_pair() { let data_cast = bx.pointercast(data_ptr, - cast.scalar_pair_element_llvm_type(bx.cx, 0)); + cast.scalar_pair_element_llvm_type(bx.cx, 0, true)); OperandValue::Pair(data_cast, meta) } else { // cast to thin-ptr // Cast of fat-ptr to thin-ptr is an extraction of data-ptr and diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs index 88b75ff9c09..195f1c3b85a 100644 --- a/src/librustc_codegen_llvm/type_of.rs +++ b/src/librustc_codegen_llvm/type_of.rs @@ -47,8 +47,8 @@ fn uncached_llvm_type<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, } layout::Abi::ScalarPair(..) => { return Type::struct_(cx, &[ - layout.scalar_pair_element_llvm_type(cx, 0), - layout.scalar_pair_element_llvm_type(cx, 1), + layout.scalar_pair_element_llvm_type(cx, 0, false), + layout.scalar_pair_element_llvm_type(cx, 1, false), ], false); } layout::Abi::Uninhabited | @@ -206,7 +206,7 @@ pub trait LayoutLlvmExt<'tcx> { 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>, - index: usize) -> Type; + index: usize, immediate: bool) -> Type; fn llvm_field_index(&self, index: usize) -> u64; fn pointee_info_at<'a>(&self, cx: &CodegenCx<'a, 'tcx>, offset: Size) -> Option; @@ -340,7 +340,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> { } fn scalar_pair_element_llvm_type<'a>(&self, cx: &CodegenCx<'a, 'tcx>, - index: usize) -> Type { + index: usize, immediate: bool) -> Type { // HACK(eddyb) special-case fat pointers until LLVM removes // pointee types, to avoid bitcasting every `OperandRef::deref`. match self.ty.sty { @@ -350,7 +350,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> { } ty::TyAdt(def, _) if def.is_box() => { let ptr_ty = cx.tcx.mk_mut_ptr(self.ty.boxed_ty()); - return cx.layout_of(ptr_ty).scalar_pair_element_llvm_type(cx, index); + return cx.layout_of(ptr_ty).scalar_pair_element_llvm_type(cx, index, immediate); } _ => {} } @@ -361,14 +361,13 @@ 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() { + // Make sure to return the same type `immediate_llvm_type` would when + // dealing with an immediate pair. This means that `(bool, bool)` is + // effectively represented as `{i8, i8}` in memory and `{i1, i1}` as an + // immediate, just like `bool` is typically `i8` in memory and only `i1` + // when immediate. We need to load/store `bool` as `i8` to avoid + // crippling LLVM optimizations or triggering other LLVM bugs with `i1`. + if immediate && scalar.is_bool() { return Type::i1(cx); } diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index e3fa7a7db39..c027dece014 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -149,7 +149,7 @@ pub fn enum_id_1(x: Option>) -> Option> { x } -// CHECK: i16 @enum_id_2(i16) +// CHECK: { i8, i8 } @enum_id_2(i1 zeroext %x.0, i8 %x.1) #[no_mangle] pub fn enum_id_2(x: Option) -> Option { x diff --git a/src/test/codegen/scalar-pair-bool.rs b/src/test/codegen/scalar-pair-bool.rs new file mode 100644 index 00000000000..2078a245085 --- /dev/null +++ b/src/test/codegen/scalar-pair-bool.rs @@ -0,0 +1,40 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -O + +#![crate_type = "lib"] + +// CHECK: define { i8, i8 } @pair_bool_bool(i1 zeroext %pair.0, i1 zeroext %pair.1) +#[no_mangle] +pub fn pair_bool_bool(pair: (bool, bool)) -> (bool, bool) { + pair +} + +// CHECK: define { i8, i32 } @pair_bool_i32(i1 zeroext %pair.0, i32 %pair.1) +#[no_mangle] +pub fn pair_bool_i32(pair: (bool, i32)) -> (bool, i32) { + pair +} + +// CHECK: define { i32, i8 } @pair_i32_bool(i32 %pair.0, i1 zeroext %pair.1) +#[no_mangle] +pub fn pair_i32_bool(pair: (i32, bool)) -> (i32, bool) { + pair +} + +// CHECK: define { i8, i8 } @pair_and_or(i1 zeroext %arg0.0, i1 zeroext %arg0.1) +#[no_mangle] +pub fn pair_and_or((a, b): (bool, bool)) -> (bool, bool) { + // Make sure it can operate directly on the unpacked args + // CHECK: and i1 %arg0.0, %arg0.1 + // CHECK: or i1 %arg0.0, %arg0.1 + (a && b, a || b) +} -- cgit 1.4.1-3-g733a5 From 5ba76335bb76e8c31a24d77771885c57614ace9b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jul 2018 12:14:51 +0200 Subject: update codegen tests --- src/test/codegen/function-arguments.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/test/codegen/function-arguments.rs') diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index c027dece014..5d5c7ced9f0 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -120,13 +120,13 @@ pub fn unsafe_slice(_: &[UnsafeInner]) { pub fn str(_: &[u8]) { } -// CHECK: @trait_borrow({}* nonnull %arg0.0, {}* noalias nonnull readonly %arg0.1) +// CHECK: @trait_borrow({}* nonnull %arg0.0, [4 x [[USIZE]]]* noalias readonly dereferenceable({{32|16}}) %arg0.1) // FIXME #25759 This should also have `nocapture` #[no_mangle] pub fn trait_borrow(_: &Drop) { } -// CHECK: @trait_box({}* noalias nonnull, {}* noalias nonnull readonly) +// CHECK: @trait_box({}* noalias nonnull, [4 x [[USIZE]]]* noalias readonly dereferenceable({{32|16}})) #[no_mangle] pub fn trait_box(_: Box) { } -- cgit 1.4.1-3-g733a5 From 86e59ccf34214e33273af145df13a771607dc42e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jul 2018 12:34:19 +0200 Subject: dont hardcode vtable size in codegen test --- src/test/codegen/function-arguments.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/test/codegen/function-arguments.rs') diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index 5d5c7ced9f0..09031508da1 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -120,13 +120,13 @@ pub fn unsafe_slice(_: &[UnsafeInner]) { pub fn str(_: &[u8]) { } -// CHECK: @trait_borrow({}* nonnull %arg0.0, [4 x [[USIZE]]]* noalias readonly dereferenceable({{32|16}}) %arg0.1) +// CHECK: @trait_borrow({}* nonnull %arg0.0, [4 x [[USIZE]]]* noalias readonly dereferenceable({{.*}}) %arg0.1) // FIXME #25759 This should also have `nocapture` #[no_mangle] pub fn trait_borrow(_: &Drop) { } -// CHECK: @trait_box({}* noalias nonnull, [4 x [[USIZE]]]* noalias readonly dereferenceable({{32|16}})) +// CHECK: @trait_box({}* noalias nonnull, [4 x [[USIZE]]]* noalias readonly dereferenceable({{.*}})) #[no_mangle] pub fn trait_box(_: Box) { } -- cgit 1.4.1-3-g733a5