From 67d2c9f4cd5c60058b1f561fe7dc4e0b2752eaab Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 28 Jun 2022 09:39:35 +0000 Subject: [PATCH] rustc_codegen_ssa: don't use LLVM struct types for field offsets. --- compiler/rustc_codegen_gcc/src/type_of.rs | 23 ---------- compiler/rustc_codegen_llvm/src/type_.rs | 3 -- compiler/rustc_codegen_llvm/src/type_of.rs | 2 + compiler/rustc_codegen_llvm/src/va_arg.rs | 1 + compiler/rustc_codegen_ssa/src/mir/place.rs | 43 +++++-------------- .../rustc_codegen_ssa/src/traits/type_.rs | 1 - src/test/codegen/zst-offset.rs | 6 +-- 7 files changed, 17 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/type_of.rs b/compiler/rustc_codegen_gcc/src/type_of.rs index 569ee2925b13c..55a070ee086f4 100644 --- a/compiler/rustc_codegen_gcc/src/type_of.rs +++ b/compiler/rustc_codegen_gcc/src/type_of.rs @@ -137,7 +137,6 @@ pub trait LayoutGccExt<'tcx> { fn immediate_gcc_type<'gcc>(&self, cx: &CodegenCx<'gcc, 'tcx>) -> Type<'gcc>; fn scalar_gcc_type_at<'gcc>(&self, cx: &CodegenCx<'gcc, 'tcx>, scalar: &abi::Scalar, offset: Size) -> Type<'gcc>; fn scalar_pair_element_gcc_type<'gcc>(&self, cx: &CodegenCx<'gcc, 'tcx>, index: usize, immediate: bool) -> Type<'gcc>; - fn gcc_field_index(&self, index: usize) -> u64; fn pointee_info_at<'gcc>(&self, cx: &CodegenCx<'gcc, 'tcx>, offset: Size) -> Option; } @@ -311,24 +310,6 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> { self.scalar_gcc_type_at(cx, scalar, offset) } - fn gcc_field_index(&self, index: usize) -> u64 { - match self.abi { - Abi::Scalar(_) | Abi::ScalarPair(..) => { - bug!("TyAndLayout::gcc_field_index({:?}): not applicable", self) - } - _ => {} - } - match self.fields { - FieldsShape::Primitive | FieldsShape::Union(_) => { - bug!("TyAndLayout::gcc_field_index({:?}): not applicable", self) - } - - FieldsShape::Array { .. } => index as u64, - - FieldsShape::Arbitrary { .. } => 1 + (self.fields.memory_index(index) as u64) * 2, - } - } - fn pointee_info_at<'a>(&self, cx: &CodegenCx<'a, 'tcx>, offset: Size) -> Option { if let Some(&pointee) = cx.pointee_infos.borrow().get(&(self.ty, offset)) { return pointee; @@ -358,10 +339,6 @@ impl<'gcc, 'tcx> LayoutTypeMethods<'tcx> for CodegenCx<'gcc, 'tcx> { layout.is_gcc_scalar_pair() } - fn backend_field_index(&self, layout: TyAndLayout<'tcx>, index: usize) -> u64 { - layout.gcc_field_index(index) - } - fn scalar_pair_element_backend_type(&self, layout: TyAndLayout<'tcx>, index: usize, immediate: bool) -> Type<'gcc> { layout.scalar_pair_element_gcc_type(self, index, immediate) } diff --git a/compiler/rustc_codegen_llvm/src/type_.rs b/compiler/rustc_codegen_llvm/src/type_.rs index cf2d3c423c335..c8f61080afa4d 100644 --- a/compiler/rustc_codegen_llvm/src/type_.rs +++ b/compiler/rustc_codegen_llvm/src/type_.rs @@ -265,9 +265,6 @@ impl<'ll, 'tcx> LayoutTypeMethods<'tcx> for CodegenCx<'ll, 'tcx> { fn is_backend_scalar_pair(&self, layout: TyAndLayout<'tcx>) -> bool { layout.is_llvm_scalar_pair() } - fn backend_field_index(&self, layout: TyAndLayout<'tcx>, index: usize) -> u64 { - layout.llvm_field_index(self, index) - } fn scalar_pair_element_backend_type( &self, layout: TyAndLayout<'tcx>, diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs index 862805236311d..f0b3aa5fb79ea 100644 --- a/compiler/rustc_codegen_llvm/src/type_of.rs +++ b/compiler/rustc_codegen_llvm/src/type_of.rs @@ -194,6 +194,7 @@ pub trait LayoutLlvmExt<'tcx> { index: usize, immediate: bool, ) -> &'a Type; + // FIXME(eddyb) remove (used only by `va_arg::emit_aapcs_va_arg`). 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; } @@ -366,6 +367,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { self.scalar_llvm_type_at(cx, scalar, offset) } + // FIXME(eddyb) remove (used only by `va_arg::emit_aapcs_va_arg`). fn llvm_field_index<'a>(&self, cx: &CodegenCx<'a, 'tcx>, index: usize) -> u64 { match self.abi { Abi::Scalar(_) | Abi::ScalarPair(..) => { diff --git a/compiler/rustc_codegen_llvm/src/va_arg.rs b/compiler/rustc_codegen_llvm/src/va_arg.rs index ceb3d5a84abf3..9374fd12313da 100644 --- a/compiler/rustc_codegen_llvm/src/va_arg.rs +++ b/compiler/rustc_codegen_llvm/src/va_arg.rs @@ -110,6 +110,7 @@ fn emit_aapcs_va_arg<'ll, 'tcx>( let offset_align = Align::from_bytes(4).unwrap(); let gr_type = target_ty.is_any_ptr() || target_ty.is_integral(); + // FIXME(eddyb) this should use the higher-level `PlaceRef` APIs. let (reg_off, reg_top_index, slot_size) = if gr_type { let gr_offs = bx.struct_gep(va_list_ty, va_list_addr, va_list_layout.llvm_field_index(bx.cx, 3)); diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs index 5b88635982f54..a956e610d5bf5 100644 --- a/compiler/rustc_codegen_ssa/src/mir/place.rs +++ b/compiler/rustc_codegen_ssa/src/mir/place.rs @@ -10,7 +10,7 @@ use rustc_middle::mir; use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout}; use rustc_middle::ty::{self, Ty}; -use rustc_target::abi::{Abi, Align, FieldsShape, Int, TagEncoding}; +use rustc_target::abi::{Align, FieldsShape, Int, TagEncoding}; use rustc_target::abi::{VariantIdx, Variants}; #[derive(Copy, Clone, Debug)] @@ -93,37 +93,15 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> { let effective_field_align = self.align.restrict_for_offset(offset); let mut simple = || { - let llval = match self.layout.abi { - _ if offset.bytes() == 0 => { - // Unions and newtypes only use an offset of 0. - // Also handles the first field of Scalar, ScalarPair, and Vector layouts. - self.llval - } - Abi::ScalarPair(a, b) - if offset == a.size(bx.cx()).align_to(b.align(bx.cx()).abi) => - { - // Offset matches second field. - let ty = bx.backend_type(self.layout); - bx.struct_gep(ty, self.llval, 1) - } - Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } if field.is_zst() => { - // ZST fields are not included in Scalar, ScalarPair, and Vector layouts, so manually offset the pointer. - let byte_ptr = bx.pointercast(self.llval, bx.cx().type_i8p()); - bx.gep(bx.cx().type_i8(), byte_ptr, &[bx.const_usize(offset.bytes())]) - } - Abi::Scalar(_) | Abi::ScalarPair(..) => { - // All fields of Scalar and ScalarPair layouts must have been handled by this point. - // Vector layouts have additional fields for each element of the vector, so don't panic in that case. - bug!( - "offset of non-ZST field `{:?}` does not match layout `{:#?}`", - field, - self.layout - ); - } - _ => { - let ty = bx.backend_type(self.layout); - bx.struct_gep(ty, self.llval, bx.cx().backend_field_index(self.layout, ix)) - } + let llval = if offset.bytes() == 0 { + self.llval + } else { + let byte_ptr = bx.pointercast(self.llval, bx.cx().type_i8p()); + // FIXME(eddyb) when can this not be `inbounds`? ZSTs? + // (but since `offset` is larger than `0`, that would mean the + // allocation the place points into has at least one byte, so + // any possible offset in the layout *should* be in-bounds) + bx.inbounds_gep(bx.cx().type_i8(), byte_ptr, &[bx.const_usize(offset.bytes())]) }; PlaceRef { // HACK(eddyb): have to bitcast pointers until LLVM removes pointee types. @@ -189,6 +167,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> { // Cast and adjust pointer. let byte_ptr = bx.pointercast(self.llval, bx.cx().type_i8p()); + // FIXME(eddyb) this should be `inbounds` (why not? ZSTs?) let byte_ptr = bx.gep(bx.cx().type_i8(), byte_ptr, &[offset]); // Finally, cast back to the type expected. diff --git a/compiler/rustc_codegen_ssa/src/traits/type_.rs b/compiler/rustc_codegen_ssa/src/traits/type_.rs index 5d3f07317a37b..916029ea80392 100644 --- a/compiler/rustc_codegen_ssa/src/traits/type_.rs +++ b/compiler/rustc_codegen_ssa/src/traits/type_.rs @@ -108,7 +108,6 @@ pub trait LayoutTypeMethods<'tcx>: Backend<'tcx> { fn immediate_backend_type(&self, layout: TyAndLayout<'tcx>) -> Self::Type; fn is_backend_immediate(&self, layout: TyAndLayout<'tcx>) -> bool; fn is_backend_scalar_pair(&self, layout: TyAndLayout<'tcx>) -> bool; - fn backend_field_index(&self, layout: TyAndLayout<'tcx>, index: usize) -> u64; fn scalar_pair_element_backend_type( &self, layout: TyAndLayout<'tcx>, diff --git a/src/test/codegen/zst-offset.rs b/src/test/codegen/zst-offset.rs index 29d2a1754a3af..c44175ebdeb8d 100644 --- a/src/test/codegen/zst-offset.rs +++ b/src/test/codegen/zst-offset.rs @@ -13,7 +13,7 @@ pub fn helper(_: usize) { // CHECK-LABEL: @scalar_layout #[no_mangle] pub fn scalar_layout(s: &(u64, ())) { -// CHECK: getelementptr i8, {{.+}}, [[USIZE]] 8 +// CHECK: getelementptr inbounds i8, {{.+}}, [[USIZE]] 8 let x = &s.1; &x; // keep variable in an alloca } @@ -22,7 +22,7 @@ pub fn scalar_layout(s: &(u64, ())) { // CHECK-LABEL: @scalarpair_layout #[no_mangle] pub fn scalarpair_layout(s: &(u64, u32, ())) { -// CHECK: getelementptr i8, {{.+}}, [[USIZE]] 12 +// CHECK: getelementptr inbounds i8, {{.+}}, [[USIZE]] 12 let x = &s.2; &x; // keep variable in an alloca } @@ -34,7 +34,7 @@ pub struct U64x4(u64, u64, u64, u64); // CHECK-LABEL: @vector_layout #[no_mangle] pub fn vector_layout(s: &(U64x4, ())) { -// CHECK: getelementptr i8, {{.+}}, [[USIZE]] 32 +// CHECK: getelementptr inbounds i8, {{.+}}, [[USIZE]] 32 let x = &s.1; &x; // keep variable in an alloca }