Auto merge of rust-lang#122048 - erikdesjardins:inbounds, r=oli-obk

Use GEP inbounds for ZST and DST field offsets

ZST field offsets have been non-`inbounds` since I made [this old layout change](https://github.com/rust-lang/rust/pull/73453/files#diff-160634de1c336f2cf325ff95b312777326f1ab29fec9b9b21d5ee9aae215ecf5). Before that, they would have been `inbounds` due to using `struct_gep`. Using `inbounds` for ZSTs likely doesn't matter for performance, but I'd like to remove the special case.

DST field offsets have been non-`inbounds` since the alignment-aware DST field offset computation was first [implemented](erikdesjardins@a2557d4#diff-04fd352da30ca186fe0bb71cc81a503d1eb8a02ca17a3769e1b95981cd20964aR1188) in 1.6 (back then `GEPi()` would be used for `inbounds`), but I don't think there was any reason for it.

Split out from rust-lang#121577 / rust-lang#121665.

r? `@oli-obk`

cc `@RalfJung` -- is there some weird situation where field offsets can't be `inbounds`?

Note that it's fine for `inbounds` offsets to be one-past-the-end, so it's okay even if there's a ZST as the last field in the layout:

> The base pointer has an in bounds address of an allocated object, which means that it points into an allocated object, or to its end. [(link)](https://llvm.org/docs/LangRef.html#getelementptr-instruction)

For rust-lang/unsafe-code-guidelines#93, zero-offset GEP is (now) always `inbounds`:

> Note that getelementptr with all-zero indices is always considered to be inbounds, even if the base pointer does not point to an allocated object. [(link)](https://llvm.org/docs/LangRef.html#getelementptr-instruction)

Author: bors

compiler/rustc_codegen_ssa/src/mir/place.rs

@@ -104,10 +104,6 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
         let mut simple = || {
             let llval = if offset.bytes() == 0 {
                 self.llval
-            } else if field.is_zst() {
-                // FIXME(erikdesjardins): it should be fine to use inbounds for ZSTs too;
-                // keeping this logic for now to preserve previous behavior.
-                bx.ptradd(self.llval, bx.const_usize(offset.bytes()))
             } else {
                 bx.inbounds_ptradd(self.llval, bx.const_usize(offset.bytes()))
             };
@@ -168,8 +164,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
         debug!("struct_field_ptr: DST field offset: {:?}", offset);
 
         // Adjust pointer.
-        // FIXME(erikdesjardins): should be able to use inbounds here too.
-        let ptr = bx.ptradd(self.llval, offset);
+        let ptr = bx.inbounds_ptradd(self.llval, offset);
 
         PlaceRef { llval: ptr, llextra: self.llextra, layout: field, align: effective_field_align }
     }

tests/codegen/dst-offset.rs

@@ -0,0 +1,84 @@
+//! This file tests that we correctly generate GEP instructions for DST
+//! field offsets.
+//@ compile-flags: -C no-prepopulate-passes -Copt-level=0
+
+#![crate_type = "lib"]
+
+#![feature(extern_types)]
+
+use std::ptr::addr_of;
+
+// Hack to get the correct type for usize
+// CHECK: @helper([[USIZE:i[0-9]+]] %_1)
+#[no_mangle]
+pub fn helper(_: usize) {
+}
+
+struct Dst<T: ?Sized> {
+    x: u32,
+    y: u8,
+    z: T,
+}
+
+// CHECK: @dst_dyn_trait_offset(ptr align {{[0-9]+}} [[DATA_PTR:%.+]], ptr align {{[0-9]+}} [[VTABLE_PTR:%.+]])
+#[no_mangle]
+pub fn dst_dyn_trait_offset(s: &Dst<dyn Drop>) -> &dyn Drop {
+// The alignment of dyn trait is unknown, so we compute the offset based on align from the vtable.
+
+// CHECK: [[SIZE_PTR:%[0-9]+]] = getelementptr inbounds {{.+}} [[VTABLE_PTR]]
+// CHECK: load [[USIZE]], ptr [[SIZE_PTR]]
+// CHECK: [[ALIGN_PTR:%[0-9]+]] = getelementptr inbounds {{.+}} [[VTABLE_PTR]]
+// CHECK: load [[USIZE]], ptr [[ALIGN_PTR]]
+
+// CHECK: getelementptr inbounds i8, ptr [[DATA_PTR]]
+// CHECK-NEXT: insertvalue
+// CHECK-NEXT: insertvalue
+// CHECK-NEXT: ret
+    &s.z
+}
+
+// CHECK-LABEL: @dst_slice_offset
+#[no_mangle]
+pub fn dst_slice_offset(s: &Dst<[u16]>) -> &[u16] {
+// The alignment of [u16] is known, so we generate a GEP directly.
+
+// CHECK: start:
+// CHECK-NEXT: getelementptr inbounds i8, {{.+}}, [[USIZE]] 6
+// CHECK-NEXT: insertvalue
+// CHECK-NEXT: insertvalue
+// CHECK-NEXT: ret
+    &s.z
+}
+
+#[repr(packed)]
+struct PackedDstSlice {
+    x: u32,
+    y: u8,
+    z: [u16],
+}
+
+// CHECK-LABEL: @packed_dst_slice_offset
+#[no_mangle]
+pub fn packed_dst_slice_offset(s: &PackedDstSlice) -> *const [u16] {
+// The alignment of [u16] is known, so we generate a GEP directly.
+
+// CHECK: start:
+// CHECK-NEXT: getelementptr inbounds i8, {{.+}}, [[USIZE]] 5
+// CHECK-NEXT: insertvalue
+// CHECK-NEXT: insertvalue
+// CHECK-NEXT: ret
+    addr_of!(s.z)
+}
+
+extern {
+    pub type Extern;
+}
+
+// CHECK-LABEL: @dst_extern
+#[no_mangle]
+pub fn dst_extern(s: &Dst<Extern>) -> &Extern {
+// Computing the alignment of an extern type is currently unsupported and just panics.
+
+// CHECK: call void @{{.+}}panic
+    &s.z
+}

tests/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;
     witness(&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;
     witness(&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;
     witness(&x); // keep variable in an alloca
 }