From 8ebd307d2a95840e193ff03aff21c87b5c643ef4 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Wed, 28 Feb 2024 00:09:24 -0500 Subject: [PATCH] use GEP inbounds for ZST and DST field offsets For the former, it's fine for `inbounds` offsets to be one-past-the-end, so it's okay even if the ZST is 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. https://llvm.org/docs/LangRef.html#getelementptr-instruction For the latter, even DST fields must always be inside the layout (or to its end for ZSTs), so using inbounds is also fine there. --- compiler/rustc_codegen_ssa/src/mir/place.rs | 7 +-- tests/codegen/dst-offset.rs | 69 +++++++++++++++++++++ tests/codegen/zst-offset.rs | 6 +- 3 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 tests/codegen/dst-offset.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs index 725d3bf4431..1ec6c351e25 100644 --- a/compiler/rustc_codegen_ssa/src/mir/place.rs +++ b/compiler/rustc_codegen_ssa/src/mir/place.rs @@ -104,10 +104,6 @@ pub fn project_field>( 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 @@ pub fn project_field>( 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 } } diff --git a/tests/codegen/dst-offset.rs b/tests/codegen/dst-offset.rs new file mode 100644 index 00000000000..55212112b3d --- /dev/null +++ b/tests/codegen/dst-offset.rs @@ -0,0 +1,69 @@ +//! 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"] + +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 { + 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 { +// 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) +} diff --git a/tests/codegen/zst-offset.rs b/tests/codegen/zst-offset.rs index 65d9cf39c4c..b623d492d9d 100644 --- a/tests/codegen/zst-offset.rs +++ b/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 fn scalarpair_layout(s: &(u64, u32, ())) { // 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 }