Skip to content

Commit

Permalink
use GEP inbounds for ZST and DST field offsets
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
erikdesjardins committed Mar 4, 2024
1 parent 7606c13 commit 8ebd307
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 9 deletions.
7 changes: 1 addition & 6 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
};
Expand Down Expand Up @@ -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 }
}
Expand Down
69 changes: 69 additions & 0 deletions tests/codegen/dst-offset.rs
Original file line number Diff line number Diff line change
@@ -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<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)
}
6 changes: 3 additions & 3 deletions tests/codegen/zst-offset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down

0 comments on commit 8ebd307

Please sign in to comment.