From 738868b2ff5ab22057757bcd07f70f018285159d Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Sun, 18 Jul 2021 12:17:15 +0200 Subject: [PATCH 01/13] LLVM codegen: Don't emit zero-sized padding for fields LLVM codegen: Don't emit zero-sized padding for whiles because that has no use and makes it impossible to generate the return types that LLVM expects for certain ARM SIMD intrinsics. --- compiler/rustc_codegen_llvm/src/type_.rs | 2 +- compiler/rustc_codegen_llvm/src/type_of.rs | 51 +++++++++++++++------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/type_.rs b/compiler/rustc_codegen_llvm/src/type_.rs index 8fd0caae479a8..b01bef01566c0 100644 --- a/compiler/rustc_codegen_llvm/src/type_.rs +++ b/compiler/rustc_codegen_llvm/src/type_.rs @@ -262,7 +262,7 @@ impl LayoutTypeMethods<'tcx> for CodegenCx<'ll, 'tcx> { layout.is_llvm_scalar_pair() } fn backend_field_index(&self, layout: TyAndLayout<'tcx>, index: usize) -> u64 { - layout.llvm_field_index(index) + layout.llvm_field_index(self, index) } fn scalar_pair_element_backend_type( &self, diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index 0876907e1194b..91d4cfdd77b32 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -116,11 +116,12 @@ fn struct_llfields<'a, 'tcx>( ); assert!(target_offset >= offset); let padding = target_offset - offset; - let padding_align = prev_effective_align.min(effective_field_align); - assert_eq!(offset.align_to(padding_align) + padding, target_offset); - result.push(cx.type_padding_filler(padding, padding_align)); - debug!(" padding before: {:?}", padding); - + if padding != Size::ZERO { + let padding_align = prev_effective_align.min(effective_field_align); + assert_eq!(offset.align_to(padding_align) + padding, target_offset); + result.push(cx.type_padding_filler(padding, padding_align)); + debug!(" padding before: {:?}", padding); + } result.push(field.llvm_type(cx)); offset = target_offset + field.size; prev_effective_align = effective_field_align; @@ -130,14 +131,15 @@ fn struct_llfields<'a, 'tcx>( bug!("layout: {:#?} stride: {:?} offset: {:?}", layout, layout.size, offset); } let padding = layout.size - offset; - let padding_align = prev_effective_align; - assert_eq!(offset.align_to(padding_align) + padding, layout.size); - debug!( - "struct_llfields: pad_bytes: {:?} offset: {:?} stride: {:?}", - padding, offset, layout.size - ); - result.push(cx.type_padding_filler(padding, padding_align)); - assert_eq!(result.len(), 1 + field_count * 2); + if padding != Size::ZERO { + let padding_align = prev_effective_align; + assert_eq!(offset.align_to(padding_align) + padding, layout.size); + debug!( + "struct_llfields: pad_bytes: {:?} offset: {:?} stride: {:?}", + padding, offset, layout.size + ); + result.push(cx.type_padding_filler(padding, padding_align)); + } } else { debug!("struct_llfields: offset: {:?} stride: {:?}", offset, layout.size); } @@ -177,7 +179,7 @@ pub trait LayoutLlvmExt<'tcx> { index: usize, immediate: bool, ) -> &'a Type; - fn llvm_field_index(&self, index: usize) -> u64; + fn llvm_field_index<'a>(&self, cx: &CodegenCx<'a, 'tcx>, index: usize) -> u64; fn pointee_info_at<'a>(&self, cx: &CodegenCx<'a, 'tcx>, offset: Size) -> Option; } @@ -340,7 +342,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { self.scalar_llvm_type_at(cx, scalar, offset) } - fn llvm_field_index(&self, index: usize) -> u64 { + fn llvm_field_index<'a>(&self, cx: &CodegenCx<'a, 'tcx>, index: usize) -> u64 { match self.abi { Abi::Scalar(_) | Abi::ScalarPair(..) => { bug!("TyAndLayout::llvm_field_index({:?}): not applicable", self) @@ -354,7 +356,24 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { FieldsShape::Array { .. } => index as u64, - FieldsShape::Arbitrary { .. } => 1 + (self.fields.memory_index(index) as u64) * 2, + FieldsShape::Arbitrary { .. } => { + let mut llvm_index = 0; + let mut offset = Size::ZERO; + for i in self.fields.index_by_increasing_offset() { + let target_offset = self.fields.offset(i as usize); + let field = self.field(cx, i); + let padding = target_offset - offset; + if padding != Size::ZERO { + llvm_index += 1; + } + if i == index { + return llvm_index; + } + offset = target_offset + field.size; + llvm_index += 1; + } + bug!("TyAndLayout::llvm_field_index({:?}): index {} out of range", self, index) + } } } From 60a523de9207d0465a32cee5fd96820b116ff64c Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Sun, 18 Jul 2021 13:06:13 +0200 Subject: [PATCH 02/13] Remove 0-sized paddings from field loyout tests. --- src/test/codegen/align-enum.rs | 2 +- src/test/codegen/align-struct.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/codegen/align-enum.rs b/src/test/codegen/align-enum.rs index 0f2cf5a761678..441cd04690e70 100644 --- a/src/test/codegen/align-enum.rs +++ b/src/test/codegen/align-enum.rs @@ -8,7 +8,7 @@ pub enum Align64 { A(u32), B(u32), } -// CHECK: %Align64 = type { [0 x i32], i32, [15 x i32] } +// CHECK: %Align64 = type { i32, [15 x i32] } pub struct Nested64 { a: u8, diff --git a/src/test/codegen/align-struct.rs b/src/test/codegen/align-struct.rs index 82eec67af0fac..acc5a2d5499ff 100644 --- a/src/test/codegen/align-struct.rs +++ b/src/test/codegen/align-struct.rs @@ -5,7 +5,7 @@ #[repr(align(64))] pub struct Align64(i32); -// CHECK: %Align64 = type { [0 x i32], i32, [15 x i32] } +// CHECK: %Align64 = type { i32, [15 x i32] } pub struct Nested64 { a: Align64, @@ -13,20 +13,20 @@ pub struct Nested64 { c: i32, d: i8, } -// CHECK: %Nested64 = type { [0 x i64], %Align64, [0 x i32], i32, [0 x i32], i32, [0 x i8], i8, [55 x i8] } +// CHECK: %Nested64 = type { %Align64, i32, i32, i8, [55 x i8] } pub enum Enum4 { A(i32), B(i32), } -// CHECK: %"Enum4::A" = type { [1 x i32], i32, [0 x i32] } +// CHECK: %"Enum4::A" = type { [1 x i32], i32 } pub enum Enum64 { A(Align64), B(i32), } -// CHECK: %Enum64 = type { [0 x i32], i32, [31 x i32] } -// CHECK: %"Enum64::A" = type { [8 x i64], %Align64, [0 x i64] } +// CHECK: %Enum64 = type { i32, [31 x i32] } +// CHECK: %"Enum64::A" = type { [8 x i64], %Align64 } // CHECK-LABEL: @align64 #[no_mangle] From e4106581de25d31d94695372b299aa2c8b0bce8e Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Mon, 19 Jul 2021 09:27:18 +0200 Subject: [PATCH 03/13] Replace on-the-fly llvm field index calculation with cache --- compiler/rustc_codegen_llvm/src/context.rs | 6 ++++++ compiler/rustc_codegen_llvm/src/type_of.rs | 25 +++++++--------------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 59259857b4b0d..52de4bcd7379c 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -79,6 +79,11 @@ pub struct CodegenCx<'ll, 'tcx> { pub pointee_infos: RefCell, Size), Option>>, pub isize_ty: &'ll Type, + /// Cache for the mapping from source index to llvm index for struct fields, + /// necessary because the mapping depends on padding and thus depens on + /// TyAndLayout. + pub field_projection_cache: RefCell, Vec>>, + pub coverage_cx: Option>, pub dbg_cx: Option>, @@ -308,6 +313,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { scalar_lltypes: Default::default(), pointee_infos: Default::default(), isize_ty, + field_projection_cache: Default::default(), coverage_cx, dbg_cx, eh_personality: Cell::new(None), diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index 91d4cfdd77b32..ad9696588952e 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -98,6 +98,7 @@ fn struct_llfields<'a, 'tcx>( let mut offset = Size::ZERO; let mut prev_effective_align = layout.align.abi; let mut result: Vec<_> = Vec::with_capacity(1 + field_count * 2); + let mut projection = vec![0; field_count]; for i in layout.fields.index_by_increasing_offset() { let target_offset = layout.fields.offset(i as usize); let field = layout.field(cx, i); @@ -122,6 +123,7 @@ fn struct_llfields<'a, 'tcx>( result.push(cx.type_padding_filler(padding, padding_align)); debug!(" padding before: {:?}", padding); } + projection[i] = result.len() as u32; result.push(field.llvm_type(cx)); offset = target_offset + field.size; prev_effective_align = effective_field_align; @@ -143,6 +145,7 @@ fn struct_llfields<'a, 'tcx>( } else { debug!("struct_llfields: offset: {:?} stride: {:?}", offset, layout.size); } + cx.field_projection_cache.borrow_mut().insert(layout, projection); (result, packed) } @@ -356,24 +359,12 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { FieldsShape::Array { .. } => index as u64, - FieldsShape::Arbitrary { .. } => { - let mut llvm_index = 0; - let mut offset = Size::ZERO; - for i in self.fields.index_by_increasing_offset() { - let target_offset = self.fields.offset(i as usize); - let field = self.field(cx, i); - let padding = target_offset - offset; - if padding != Size::ZERO { - llvm_index += 1; - } - if i == index { - return llvm_index; - } - offset = target_offset + field.size; - llvm_index += 1; + FieldsShape::Arbitrary { .. } => match cx.field_projection_cache.borrow().get(self) { + Some(projection) => projection[index] as u64, + None => { + bug!("TyAndLayout::llvm_field_index({:?}): field projection not cached", self) } - bug!("TyAndLayout::llvm_field_index({:?}): index {} out of range", self, index) - } + }, } } From e89908231bede75f2e21959dfd851df85efd25c5 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Tue, 20 Jul 2021 07:38:00 +0200 Subject: [PATCH 04/13] Don't cache projection if no padding is used. In this case we can just return memory_index(index) which is readily available. --- compiler/rustc_codegen_llvm/src/type_of.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index ad9696588952e..da2927566730f 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -99,6 +99,7 @@ fn struct_llfields<'a, 'tcx>( let mut prev_effective_align = layout.align.abi; let mut result: Vec<_> = Vec::with_capacity(1 + field_count * 2); let mut projection = vec![0; field_count]; + let mut padding_used = false; for i in layout.fields.index_by_increasing_offset() { let target_offset = layout.fields.offset(i as usize); let field = layout.field(cx, i); @@ -118,6 +119,7 @@ fn struct_llfields<'a, 'tcx>( assert!(target_offset >= offset); let padding = target_offset - offset; if padding != Size::ZERO { + padding_used = true; let padding_align = prev_effective_align.min(effective_field_align); assert_eq!(offset.align_to(padding_align) + padding, target_offset); result.push(cx.type_padding_filler(padding, padding_align)); @@ -145,7 +147,9 @@ fn struct_llfields<'a, 'tcx>( } else { debug!("struct_llfields: offset: {:?} stride: {:?}", offset, layout.size); } - cx.field_projection_cache.borrow_mut().insert(layout, projection); + if padding_used { + cx.field_projection_cache.borrow_mut().insert(layout, projection); + } (result, packed) } @@ -361,9 +365,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { FieldsShape::Arbitrary { .. } => match cx.field_projection_cache.borrow().get(self) { Some(projection) => projection[index] as u64, - None => { - bug!("TyAndLayout::llvm_field_index({:?}): field projection not cached", self) - } + None => self.fields.memory_index(index) as u64, }, } } From 50be80b22d5edf508198fa757a0a88316b009af6 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Wed, 21 Jul 2021 12:41:27 +0200 Subject: [PATCH 05/13] Improve/add comments --- compiler/rustc_codegen_llvm/src/context.rs | 3 +-- compiler/rustc_codegen_llvm/src/type_of.rs | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 52de4bcd7379c..b088b6e730bdc 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -80,8 +80,7 @@ pub struct CodegenCx<'ll, 'tcx> { pub isize_ty: &'ll Type, /// Cache for the mapping from source index to llvm index for struct fields, - /// necessary because the mapping depends on padding and thus depens on - /// TyAndLayout. + /// only present if synthetic fields are inserted for padding. pub field_projection_cache: RefCell, Vec>>, pub coverage_cx: Option>, diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index da2927566730f..79bf51929e9db 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -363,6 +363,8 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { FieldsShape::Array { .. } => index as u64, + // Look up llvm field index in projection cache if present. If no projection cache + // is present no padding is used and the llvm field index matches the memory index. FieldsShape::Arbitrary { .. } => match cx.field_projection_cache.borrow().get(self) { Some(projection) => projection[index] as u64, None => self.fields.memory_index(index) as u64, From 4a8202c4a68ec6161dbc31dcb5b0ec51d84c9d87 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Wed, 21 Jul 2021 13:20:35 +0200 Subject: [PATCH 06/13] Add testcase for proper LLVM representation of SIMD types. Testcase to make sure that no 0-sized padding is inserted in structs and that structs are represented as expected by Neon intrinsics in LLVM. --- src/test/codegen/unpadded-simd.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 src/test/codegen/unpadded-simd.rs diff --git a/src/test/codegen/unpadded-simd.rs b/src/test/codegen/unpadded-simd.rs new file mode 100644 index 0000000000000..eb44dbd931352 --- /dev/null +++ b/src/test/codegen/unpadded-simd.rs @@ -0,0 +1,14 @@ +// Make sure that no 0-sized padding is inserted in structs and that +// structs are represented as expected by Neon intrinsics in LLVM. +// See #87254. + +#![crate_type = "lib"] +#![feature(repr_simd)] + +#[derive(Copy, Clone, Debug)] +#[repr(simd)] +pub struct int16x4_t(pub i16, pub i16, pub i16, pub i16); + +#[derive(Copy, Clone, Debug)] +pub struct int16x4x2_t(pub int16x4_t, pub int16x4_t); +// CHECK: %int16x4x2_t = type { <4 x i16>, <4 x i16> } From 7dbc568325c929eeefc261532b034f904cf4d5b4 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Tue, 3 Aug 2021 05:11:17 +0000 Subject: [PATCH 07/13] Fix va_args calling on aarch64 non-macos/ios. emit_aapcs_va_arg() emits hardcoded field indexes to access the aarch64-specific `VaListImpl` struct. Due to the removed padding those indexes have changed. --- compiler/rustc_codegen_llvm/src/va_arg.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/va_arg.rs b/compiler/rustc_codegen_llvm/src/va_arg.rs index c9fb09570c35a..4888f7f355fa9 100644 --- a/compiler/rustc_codegen_llvm/src/va_arg.rs +++ b/compiler/rustc_codegen_llvm/src/va_arg.rs @@ -110,13 +110,13 @@ fn emit_aapcs_va_arg( let gr_type = target_ty.is_any_ptr() || target_ty.is_integral(); let (reg_off, reg_top_index, slot_size) = if gr_type { - let gr_offs = bx.struct_gep(va_list_ty, va_list_addr, 7); + let gr_offs = bx.struct_gep(va_list_ty, va_list_addr, 3); let nreg = (layout.size.bytes() + 7) / 8; - (gr_offs, 3, nreg * 8) + (gr_offs, 1, nreg * 8) } else { - let vr_off = bx.struct_gep(va_list_ty, va_list_addr, 9); + let vr_off = bx.struct_gep(va_list_ty, va_list_addr, 4); let nreg = (layout.size.bytes() + 15) / 16; - (vr_off, 5, nreg * 16) + (vr_off, 2, nreg * 16) }; // if the offset >= 0 then the value will be on the stack From 4a4e9e3cf81e8c85880d8da521ab16f253c93392 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Thu, 5 Aug 2021 19:14:55 +0200 Subject: [PATCH 08/13] Store field remapping information together with the LLVM type in a new TypeLowering struct instead of an extra cache. --- compiler/rustc_codegen_llvm/src/context.rs | 22 ++++++--- compiler/rustc_codegen_llvm/src/type_of.rs | 54 ++++++++++++++-------- 2 files changed, 50 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index b088b6e730bdc..d28a579ff1059 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -74,15 +74,15 @@ pub struct CodegenCx<'ll, 'tcx> { /// See for details pub used_statics: RefCell>, - pub lltypes: RefCell, Option), &'ll Type>>, + /// Mapping of non-scalar types to llvm types and field remapping if needed. + pub type_lowering: RefCell, Option), TypeLowering<'ll>>>, + + /// Mapping of scalar types to llvm types. pub scalar_lltypes: RefCell, &'ll Type>>, + pub pointee_infos: RefCell, Size), Option>>, pub isize_ty: &'ll Type, - /// Cache for the mapping from source index to llvm index for struct fields, - /// only present if synthetic fields are inserted for padding. - pub field_projection_cache: RefCell, Vec>>, - pub coverage_cx: Option>, pub dbg_cx: Option>, @@ -96,6 +96,15 @@ pub struct CodegenCx<'ll, 'tcx> { local_gen_sym_counter: Cell, } +pub struct TypeLowering<'ll> { + /// Associated LLVM type + pub lltype: &'ll Type, + + /// If padding is used the slice maps fields from source order + /// to llvm order. + pub field_remapping: Option>, +} + fn to_llvm_tls_model(tls_model: TlsModel) -> llvm::ThreadLocalMode { match tls_model { TlsModel::GeneralDynamic => llvm::ThreadLocalMode::GeneralDynamic, @@ -308,11 +317,10 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { const_globals: Default::default(), statics_to_rauw: RefCell::new(Vec::new()), used_statics: RefCell::new(Vec::new()), - lltypes: Default::default(), + type_lowering: Default::default(), scalar_lltypes: Default::default(), pointee_infos: Default::default(), isize_ty, - field_projection_cache: Default::default(), coverage_cx, dbg_cx, eh_personality: Cell::new(None), diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index 79bf51929e9db..24669bec96531 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -1,5 +1,6 @@ use crate::abi::FnAbi; use crate::common::*; +use crate::context::TypeLowering; use crate::type_::Type; use rustc_codegen_ssa::traits::*; use rustc_middle::bug; @@ -17,6 +18,7 @@ fn uncached_llvm_type<'a, 'tcx>( cx: &CodegenCx<'a, 'tcx>, layout: TyAndLayout<'tcx>, defer: &mut Option<(&'a Type, TyAndLayout<'tcx>)>, + field_remapping: &mut Option>, ) -> &'a Type { match layout.abi { Abi::Scalar(_) => bug!("handled elsewhere"), @@ -75,7 +77,8 @@ fn uncached_llvm_type<'a, 'tcx>( FieldsShape::Array { count, .. } => cx.type_array(layout.field(cx, 0).llvm_type(cx), count), FieldsShape::Arbitrary { .. } => match name { None => { - let (llfields, packed) = struct_llfields(cx, layout); + let (llfields, packed, new_field_remapping) = struct_llfields(cx, layout); + *field_remapping = new_field_remapping; cx.type_struct(&llfields, packed) } Some(ref name) => { @@ -90,7 +93,7 @@ fn uncached_llvm_type<'a, 'tcx>( fn struct_llfields<'a, 'tcx>( cx: &CodegenCx<'a, 'tcx>, layout: TyAndLayout<'tcx>, -) -> (Vec<&'a Type>, bool) { +) -> (Vec<&'a Type>, bool, Option>) { debug!("struct_llfields: {:#?}", layout); let field_count = layout.fields.count(); @@ -147,11 +150,8 @@ fn struct_llfields<'a, 'tcx>( } else { debug!("struct_llfields: offset: {:?} stride: {:?}", offset, layout.size); } - if padding_used { - cx.field_projection_cache.borrow_mut().insert(layout, projection); - } - (result, packed) + (result, packed, padding_used.then_some(projection.into_boxed_slice())) } impl<'a, 'tcx> CodegenCx<'a, 'tcx> { @@ -243,8 +243,8 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { Variants::Single { index } => Some(index), _ => None, }; - if let Some(&llty) = cx.lltypes.borrow().get(&(self.ty, variant_index)) { - return llty; + if let Some(ref llty) = cx.type_lowering.borrow().get(&(self.ty, variant_index)) { + return llty.lltype; } debug!("llvm_type({:#?})", self); @@ -256,6 +256,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { let normal_ty = cx.tcx.erase_regions(self.ty); let mut defer = None; + let mut field_remapping = None; let llty = if self.ty != normal_ty { let mut layout = cx.layout_of(normal_ty); if let Some(v) = variant_index { @@ -263,17 +264,21 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { } layout.llvm_type(cx) } else { - uncached_llvm_type(cx, *self, &mut defer) + uncached_llvm_type(cx, *self, &mut defer, &mut field_remapping) }; debug!("--> mapped {:#?} to llty={:?}", self, llty); - cx.lltypes.borrow_mut().insert((self.ty, variant_index), llty); + cx.type_lowering + .borrow_mut() + .insert((self.ty, variant_index), TypeLowering { lltype: llty, field_remapping: None }); if let Some((llty, layout)) = defer { - let (llfields, packed) = struct_llfields(cx, layout); - cx.set_struct_body(llty, &llfields, packed) + let (llfields, packed, new_field_remapping) = struct_llfields(cx, layout); + cx.set_struct_body(llty, &llfields, packed); + field_remapping = new_field_remapping; } - + cx.type_lowering.borrow_mut().get_mut(&(self.ty, variant_index)).unwrap().field_remapping = + field_remapping; llty } @@ -363,12 +368,23 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { FieldsShape::Array { .. } => index as u64, - // Look up llvm field index in projection cache if present. If no projection cache - // is present no padding is used and the llvm field index matches the memory index. - FieldsShape::Arbitrary { .. } => match cx.field_projection_cache.borrow().get(self) { - Some(projection) => projection[index] as u64, - None => self.fields.memory_index(index) as u64, - }, + FieldsShape::Arbitrary { .. } => { + let variant_index = match self.variants { + Variants::Single { index } => Some(index), + _ => None, + }; + + // Look up llvm field if indexes do not match memory order due to padding. If + // `field_remapping` is `None` no padding was used and the llvm field index + // matches the memory index. + match cx.type_lowering.borrow().get(&(self.ty, variant_index)) { + Some(TypeLowering { field_remapping: Some(ref prj), .. }) => prj[index] as u64, + Some(_) => self.fields.memory_index(index) as u64, + None => { + bug!("TyAndLayout::llvm_field_index({:?}): type info not found", self) + } + } + } } } From 89a369ad2e367a473385f1363896e03e1e390718 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Thu, 5 Aug 2021 22:40:32 +0000 Subject: [PATCH 09/13] Replace hard-coded field indexes with lookup on aarch64 non-macos. The indexes into the VaListImpl struct used on aarch64 ABI (not macos/ios) are hard-coded which is brittle so we replace them with the usual lookup. The varargs ffi is tested in ui/abi/variadic-ffi.rs on aarch64 Linux. --- compiler/rustc_codegen_llvm/src/va_arg.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/va_arg.rs b/compiler/rustc_codegen_llvm/src/va_arg.rs index 4888f7f355fa9..2208ec37a4235 100644 --- a/compiler/rustc_codegen_llvm/src/va_arg.rs +++ b/compiler/rustc_codegen_llvm/src/va_arg.rs @@ -98,7 +98,8 @@ fn emit_aapcs_va_arg( // Implementation of the AAPCS64 calling convention for va_args see // https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst let va_list_addr = list.immediate(); - let va_list_ty = list.deref(bx.cx).layout.llvm_type(bx); + let va_list_layout = list.deref(bx.cx).layout; + let va_list_ty = va_list_layout.llvm_type(bx); let layout = bx.cx.layout_of(target_ty); let mut maybe_reg = bx.build_sibling_block("va_arg.maybe_reg"); @@ -110,13 +111,15 @@ fn emit_aapcs_va_arg( let gr_type = target_ty.is_any_ptr() || target_ty.is_integral(); let (reg_off, reg_top_index, slot_size) = if gr_type { - let gr_offs = bx.struct_gep(va_list_ty, va_list_addr, 3); + let gr_offs = + bx.struct_gep(va_list_ty, va_list_addr, va_list_layout.llvm_field_index(bx.cx, 3)); let nreg = (layout.size.bytes() + 7) / 8; - (gr_offs, 1, nreg * 8) + (gr_offs, va_list_layout.llvm_field_index(bx.cx, 1), nreg * 8) } else { - let vr_off = bx.struct_gep(va_list_ty, va_list_addr, 4); + let vr_off = + bx.struct_gep(va_list_ty, va_list_addr, va_list_layout.llvm_field_index(bx.cx, 4)); let nreg = (layout.size.bytes() + 15) / 16; - (vr_off, 2, nreg * 16) + (vr_off, va_list_layout.llvm_field_index(bx.cx, 2), nreg * 16) }; // if the offset >= 0 then the value will be on the stack From c627c0d88ba8bf73793d9fb9bf1f1452c497f26a Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Mon, 9 Aug 2021 12:08:18 +0000 Subject: [PATCH 10/13] Fix nits. --- compiler/rustc_codegen_llvm/src/type_of.rs | 28 ++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index 24669bec96531..46e824d879fec 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -101,8 +101,7 @@ fn struct_llfields<'a, 'tcx>( let mut offset = Size::ZERO; let mut prev_effective_align = layout.align.abi; let mut result: Vec<_> = Vec::with_capacity(1 + field_count * 2); - let mut projection = vec![0; field_count]; - let mut padding_used = false; + let mut field_remapping = vec![0; field_count]; for i in layout.fields.index_by_increasing_offset() { let target_offset = layout.fields.offset(i as usize); let field = layout.field(cx, i); @@ -122,17 +121,17 @@ fn struct_llfields<'a, 'tcx>( assert!(target_offset >= offset); let padding = target_offset - offset; if padding != Size::ZERO { - padding_used = true; let padding_align = prev_effective_align.min(effective_field_align); assert_eq!(offset.align_to(padding_align) + padding, target_offset); result.push(cx.type_padding_filler(padding, padding_align)); debug!(" padding before: {:?}", padding); } - projection[i] = result.len() as u32; + field_remapping[i] = result.len() as u32; result.push(field.llvm_type(cx)); offset = target_offset + field.size; prev_effective_align = effective_field_align; } + let padding_used = result.len() > field_count; if !layout.is_unsized() && field_count > 0 { if offset > layout.size { bug!("layout: {:#?} stride: {:?} offset: {:?}", layout, layout.size, offset); @@ -151,7 +150,7 @@ fn struct_llfields<'a, 'tcx>( debug!("struct_llfields: offset: {:?} stride: {:?}", offset, layout.size); } - (result, packed, padding_used.then_some(projection.into_boxed_slice())) + (result, packed, padding_used.then_some(field_remapping.into_boxed_slice())) } impl<'a, 'tcx> CodegenCx<'a, 'tcx> { @@ -268,17 +267,20 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { }; debug!("--> mapped {:#?} to llty={:?}", self, llty); - cx.type_lowering - .borrow_mut() - .insert((self.ty, variant_index), TypeLowering { lltype: llty, field_remapping: None }); + cx.type_lowering.borrow_mut().insert( + (self.ty, variant_index), + TypeLowering { lltype: llty, field_remapping: field_remapping }, + ); if let Some((llty, layout)) = defer { let (llfields, packed, new_field_remapping) = struct_llfields(cx, layout); cx.set_struct_body(llty, &llfields, packed); - field_remapping = new_field_remapping; + cx.type_lowering + .borrow_mut() + .get_mut(&(self.ty, variant_index)) + .unwrap() + .field_remapping = new_field_remapping; } - cx.type_lowering.borrow_mut().get_mut(&(self.ty, variant_index)).unwrap().field_remapping = - field_remapping; llty } @@ -378,7 +380,9 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { // `field_remapping` is `None` no padding was used and the llvm field index // matches the memory index. match cx.type_lowering.borrow().get(&(self.ty, variant_index)) { - Some(TypeLowering { field_remapping: Some(ref prj), .. }) => prj[index] as u64, + Some(TypeLowering { field_remapping: Some(ref remap), .. }) => { + remap[index] as u64 + } Some(_) => self.fields.memory_index(index) as u64, None => { bug!("TyAndLayout::llvm_field_index({:?}): type info not found", self) From c1d0f0a65c36e73ebf7a1ac899b13fb82e3f6483 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Mon, 9 Aug 2021 12:25:33 +0000 Subject: [PATCH 11/13] TEST: Use SmallVec<[u32; 4]> for field projection. --- compiler/rustc_codegen_llvm/src/context.rs | 3 ++- compiler/rustc_codegen_llvm/src/type_of.rs | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index d28a579ff1059..225514ea863cb 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -24,6 +24,7 @@ use rustc_span::source_map::{Span, DUMMY_SP}; use rustc_span::symbol::Symbol; use rustc_target::abi::{HasDataLayout, LayoutOf, PointeeInfo, Size, TargetDataLayout, VariantIdx}; use rustc_target::spec::{HasTargetSpec, RelocModel, Target, TlsModel}; +use smallvec::SmallVec; use std::cell::{Cell, RefCell}; use std::ffi::CStr; @@ -102,7 +103,7 @@ pub struct TypeLowering<'ll> { /// If padding is used the slice maps fields from source order /// to llvm order. - pub field_remapping: Option>, + pub field_remapping: Option>>, } fn to_llvm_tls_model(tls_model: TlsModel) -> llvm::ThreadLocalMode { diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index 46e824d879fec..225044c848857 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -10,6 +10,7 @@ use rustc_middle::ty::{self, Ty, TypeFoldable}; use rustc_target::abi::{Abi, AddressSpace, Align, FieldsShape}; use rustc_target::abi::{Int, Pointer, F32, F64}; use rustc_target::abi::{LayoutOf, PointeeInfo, Scalar, Size, TyAndLayoutMethods, Variants}; +use smallvec::{smallvec, SmallVec}; use tracing::debug; use std::fmt::Write; @@ -18,7 +19,7 @@ fn uncached_llvm_type<'a, 'tcx>( cx: &CodegenCx<'a, 'tcx>, layout: TyAndLayout<'tcx>, defer: &mut Option<(&'a Type, TyAndLayout<'tcx>)>, - field_remapping: &mut Option>, + field_remapping: &mut Option>>, ) -> &'a Type { match layout.abi { Abi::Scalar(_) => bug!("handled elsewhere"), @@ -93,7 +94,7 @@ fn uncached_llvm_type<'a, 'tcx>( fn struct_llfields<'a, 'tcx>( cx: &CodegenCx<'a, 'tcx>, layout: TyAndLayout<'tcx>, -) -> (Vec<&'a Type>, bool, Option>) { +) -> (Vec<&'a Type>, bool, Option>>) { debug!("struct_llfields: {:#?}", layout); let field_count = layout.fields.count(); @@ -101,7 +102,7 @@ fn struct_llfields<'a, 'tcx>( let mut offset = Size::ZERO; let mut prev_effective_align = layout.align.abi; let mut result: Vec<_> = Vec::with_capacity(1 + field_count * 2); - let mut field_remapping = vec![0; field_count]; + let mut field_remapping = smallvec![0; field_count]; for i in layout.fields.index_by_increasing_offset() { let target_offset = layout.fields.offset(i as usize); let field = layout.field(cx, i); @@ -150,7 +151,7 @@ fn struct_llfields<'a, 'tcx>( debug!("struct_llfields: offset: {:?} stride: {:?}", offset, layout.size); } - (result, packed, padding_used.then_some(field_remapping.into_boxed_slice())) + (result, packed, padding_used.then_some(Box::new(field_remapping))) } impl<'a, 'tcx> CodegenCx<'a, 'tcx> { From 1d4972e0e923bd5fffb11465373c824be109375f Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Mon, 9 Aug 2021 12:59:23 +0000 Subject: [PATCH 12/13] Avoid unnecessary allocation. --- compiler/rustc_codegen_llvm/src/type_of.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index 225044c848857..180da1a6046e6 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -150,8 +150,8 @@ fn struct_llfields<'a, 'tcx>( } else { debug!("struct_llfields: offset: {:?} stride: {:?}", offset, layout.size); } - - (result, packed, padding_used.then_some(Box::new(field_remapping))) + let field_remapping = if padding_used { Some(Box::new(field_remapping)) } else { None }; + (result, packed, field_remapping) } impl<'a, 'tcx> CodegenCx<'a, 'tcx> { From 02295f464aaf78ece81a80e5b99a034119e74748 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Mon, 9 Aug 2021 15:42:37 +0000 Subject: [PATCH 13/13] Test: Use smallvec directly instead of boxed. --- compiler/rustc_codegen_llvm/src/context.rs | 2 +- compiler/rustc_codegen_llvm/src/type_of.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 225514ea863cb..5a8aa66c61119 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -103,7 +103,7 @@ pub struct TypeLowering<'ll> { /// If padding is used the slice maps fields from source order /// to llvm order. - pub field_remapping: Option>>, + pub field_remapping: Option>, } fn to_llvm_tls_model(tls_model: TlsModel) -> llvm::ThreadLocalMode { diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index 180da1a6046e6..85efe3e64836c 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -19,7 +19,7 @@ fn uncached_llvm_type<'a, 'tcx>( cx: &CodegenCx<'a, 'tcx>, layout: TyAndLayout<'tcx>, defer: &mut Option<(&'a Type, TyAndLayout<'tcx>)>, - field_remapping: &mut Option>>, + field_remapping: &mut Option>, ) -> &'a Type { match layout.abi { Abi::Scalar(_) => bug!("handled elsewhere"), @@ -94,7 +94,7 @@ fn uncached_llvm_type<'a, 'tcx>( fn struct_llfields<'a, 'tcx>( cx: &CodegenCx<'a, 'tcx>, layout: TyAndLayout<'tcx>, -) -> (Vec<&'a Type>, bool, Option>>) { +) -> (Vec<&'a Type>, bool, Option>) { debug!("struct_llfields: {:#?}", layout); let field_count = layout.fields.count(); @@ -150,7 +150,7 @@ fn struct_llfields<'a, 'tcx>( } else { debug!("struct_llfields: offset: {:?} stride: {:?}", offset, layout.size); } - let field_remapping = if padding_used { Some(Box::new(field_remapping)) } else { None }; + let field_remapping = if padding_used { Some(field_remapping) } else { None }; (result, packed, field_remapping) }