rustc_codegen_ssa: don't use LLVM struct types for field offsets.

eddyb · eddyb · commit 67d2c9f4cd5c · 2022-06-28T09:39:35.000Z
diff --git 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<PointeeInfo>;
 }
 
@@ -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<PointeeInfo> {
         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
@@ -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
@@ -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<PointeeInfo>;
 }
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
 }

Original file line number	Diff line number	Diff line change
`@@ -194,6 +194,7 @@ pub trait LayoutLlvmExt<'tcx> {`
`194`	`194`	`index: usize,`
`195`	`195`	`immediate: bool,`
`196`	`196`	`) -> &'a Type;`
	`197`	+ // FIXME(eddyb) remove (used only by `va_arg::emit_aapcs_va_arg`).
`197`	`198`	`fn llvm_field_index<'a>(&self, cx: &CodegenCx<'a, 'tcx>, index: usize) -> u64;`
`198`	`199`	`fn pointee_info_at<'a>(&self, cx: &CodegenCx<'a, 'tcx>, offset: Size) -> Option<PointeeInfo>;`
`199`	`200`	`}`
`@@ -366,6 +367,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {`
`366`	`367`	`self.scalar_llvm_type_at(cx, scalar, offset)`
`367`	`368`	`}`
`368`	`369`
	`370`	+ // FIXME(eddyb) remove (used only by `va_arg::emit_aapcs_va_arg`).
`369`	`371`	`fn llvm_field_index<'a>(&self, cx: &CodegenCx<'a, 'tcx>, index: usize) -> u64 {`
`370`	`372`	`match self.abi {`
`371`	`373`	`Abi::Scalar(_) \| Abi::ScalarPair(..) => {`