Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 0 additions & 23 deletions compiler/rustc_codegen_gcc/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_llvm/src/type_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
}
Expand Down Expand Up @@ -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(..) => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/va_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
43 changes: 11 additions & 32 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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());
// This is always `inbounds`. For example, ZSTs cannot arise
// because `offset` is larger than `0`, which means the
// allocation the place points into has at least one byte, so
// any possible offset in the layout *should* be in-bounds)
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_codegen_ssa/src/traits/type_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
6 changes: 3 additions & 3 deletions src/test/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;
&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;
&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;
&x; // keep variable in an alloca
}