Skip to content

Commit

Permalink
Rollup merge of rust-lang#107248 - erikdesjardins:addrspace, r=oli-obk
Browse files Browse the repository at this point in the history
abi: add AddressSpace field to Primitive::Pointer

...and remove it from `PointeeInfo`, which isn't meant for this.

There are still various places (marked with FIXMEs) that assume all pointers
have the same size and alignment. Fixing this requires parsing non-default
address spaces in the data layout string (and various other changes),
which will be done in a followup.
(That is, if it's actually worth it to support multiple different pointer sizes.
There is a lot of code that would be affected by that.)

Fixes rust-lang#106367

r? `@oli-obk`
cc `@Patryk27`
  • Loading branch information
matthiaskrgr committed Jan 25, 2023
2 parents 3c8ae7e + adc1890 commit 6bf0e73
Show file tree
Hide file tree
Showing 28 changed files with 229 additions and 197 deletions.
36 changes: 14 additions & 22 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,9 @@ impl TargetDataLayout {
["a", ref a @ ..] => dl.aggregate_align = align(a, "a")?,
["f32", ref a @ ..] => dl.f32_align = align(a, "f32")?,
["f64", ref a @ ..] => dl.f64_align = align(a, "f64")?,
// FIXME(erikdesjardins): we should be parsing nonzero address spaces
// this will require replacing TargetDataLayout::{pointer_size,pointer_align}
// with e.g. `fn pointer_size_in(AddressSpace)`
[p @ "p", s, ref a @ ..] | [p @ "p0", s, ref a @ ..] => {
dl.pointer_size = size(s, p)?;
dl.pointer_align = align(a, p)?;
Expand Down Expand Up @@ -861,7 +864,7 @@ pub enum Primitive {
Int(Integer, bool),
F32,
F64,
Pointer,
Pointer(AddressSpace),
}

impl Primitive {
Expand All @@ -872,7 +875,10 @@ impl Primitive {
Int(i, _) => i.size(),
F32 => Size::from_bits(32),
F64 => Size::from_bits(64),
Pointer => dl.pointer_size,
// FIXME(erikdesjardins): ignoring address space is technically wrong, pointers in
// different address spaces can have different sizes
// (but TargetDataLayout doesn't currently parse that part of the DL string)
Pointer(_) => dl.pointer_size,
}
}

Expand All @@ -883,26 +889,12 @@ impl Primitive {
Int(i, _) => i.align(dl),
F32 => dl.f32_align,
F64 => dl.f64_align,
Pointer => dl.pointer_align,
// FIXME(erikdesjardins): ignoring address space is technically wrong, pointers in
// different address spaces can have different alignments
// (but TargetDataLayout doesn't currently parse that part of the DL string)
Pointer(_) => dl.pointer_align,
}
}

// FIXME(eddyb) remove, it's trivial thanks to `matches!`.
#[inline]
pub fn is_float(self) -> bool {
matches!(self, F32 | F64)
}

// FIXME(eddyb) remove, it's completely unused.
#[inline]
pub fn is_int(self) -> bool {
matches!(self, Int(..))
}

#[inline]
pub fn is_ptr(self) -> bool {
matches!(self, Pointer)
}
}

/// Inclusive wrap-around range of valid values, that is, if
Expand Down Expand Up @@ -1188,7 +1180,8 @@ impl FieldsShape {
/// An identifier that specifies the address space that some operation
/// should operate on. Special address spaces have an effect on code generation,
/// depending on the target and the address spaces it implements.
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
pub struct AddressSpace(pub u32);

impl AddressSpace {
Expand Down Expand Up @@ -1468,7 +1461,6 @@ pub struct PointeeInfo {
pub size: Size,
pub align: Align,
pub safe: Option<PointerKind>,
pub address_space: AddressSpace,
}

/// Used in `might_permit_raw_init` to indicate the kind of initialisation
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_cranelift/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ pub(crate) fn scalar_to_clif_type(tcx: TyCtxt<'_>, scalar: Scalar) -> Type {
},
Primitive::F32 => types::F32,
Primitive::F64 => types::F64,
Primitive::Pointer => pointer_ty(tcx),
// FIXME(erikdesjardins): handle non-default addrspace ptr sizes
Primitive::Pointer(_) => pointer_ty(tcx),
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
bx.range_metadata(load, vr);
}
}
abi::Pointer if vr.start < vr.end && !vr.contains(0) => {
abi::Pointer(_) if vr.start < vr.end && !vr.contains(0) => {
bx.nonnull_metadata(load);
}
_ => {}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
let base_addr = self.const_bitcast(base_addr, self.usize_type);
let offset = self.context.new_rvalue_from_long(self.usize_type, offset.bytes() as i64);
let ptr = self.const_bitcast(base_addr + offset, ptr_type);
if layout.primitive() != Pointer {
if !matches!(layout.primitive(), Pointer(_)) {
self.const_bitcast(ptr.dereference(None).to_rvalue(), ty)
}
else {
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_codegen_gcc/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,16 @@ pub fn const_alloc_to_gcc<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, alloc: ConstAl
)
.expect("const_alloc_to_llvm: could not read relocation pointer")
as u64;

let address_space = cx.tcx.global_alloc(alloc_id).address_space(cx);

llvals.push(cx.scalar_to_backend(
InterpScalar::from_pointer(
interpret::Pointer::new(alloc_id, Size::from_bytes(ptr_offset)),
&cx.tcx,
),
abi::Scalar::Initialized { value: Primitive::Pointer, valid_range: WrappingRange::full(dl.pointer_size) },
cx.type_i8p(),
abi::Scalar::Initialized { value: Primitive::Pointer(address_space), valid_range: WrappingRange::full(dl.pointer_size) },
cx.type_i8p_ext(address_space),
));
next_offset = offset + pointer_size;
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_gcc/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
Int(i, false) => cx.type_from_unsigned_integer(i),
F32 => cx.type_f32(),
F64 => cx.type_f64(),
Pointer => {
Pointer(address_space) => {
// If we know the alignment, pick something better than i8.
let pointee =
if let Some(pointee) = self.pointee_info_at(cx, offset) {
Expand All @@ -262,7 +262,7 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
else {
cx.type_i8()
};
cx.type_ptr_to(pointee)
cx.type_ptr_to_ext(pointee, address_space)
}
}
}
Expand Down
13 changes: 9 additions & 4 deletions compiler/rustc_codegen_llvm/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,14 +849,16 @@ fn dummy_output_type<'ll>(cx: &CodegenCx<'ll, '_>, reg: InlineAsmRegClass) -> &'
/// Helper function to get the LLVM type for a Scalar. Pointers are returned as
/// the equivalent integer type.
fn llvm_asm_scalar_type<'ll>(cx: &CodegenCx<'ll, '_>, scalar: Scalar) -> &'ll Type {
let dl = &cx.tcx.data_layout;
match scalar.primitive() {
Primitive::Int(Integer::I8, _) => cx.type_i8(),
Primitive::Int(Integer::I16, _) => cx.type_i16(),
Primitive::Int(Integer::I32, _) => cx.type_i32(),
Primitive::Int(Integer::I64, _) => cx.type_i64(),
Primitive::F32 => cx.type_f32(),
Primitive::F64 => cx.type_f64(),
Primitive::Pointer => cx.type_isize(),
// FIXME(erikdesjardins): handle non-default addrspace ptr sizes
Primitive::Pointer(_) => cx.type_from_integer(dl.ptr_sized_integer()),
_ => unreachable!(),
}
}
Expand All @@ -868,6 +870,7 @@ fn llvm_fixup_input<'ll, 'tcx>(
reg: InlineAsmRegClass,
layout: &TyAndLayout<'tcx>,
) -> &'ll Value {
let dl = &bx.tcx.data_layout;
match (reg, layout.abi) {
(InlineAsmRegClass::AArch64(AArch64InlineAsmRegClass::vreg), Abi::Scalar(s)) => {
if let Primitive::Int(Integer::I8, _) = s.primitive() {
Expand All @@ -881,8 +884,10 @@ fn llvm_fixup_input<'ll, 'tcx>(
let elem_ty = llvm_asm_scalar_type(bx.cx, s);
let count = 16 / layout.size.bytes();
let vec_ty = bx.cx.type_vector(elem_ty, count);
if let Primitive::Pointer = s.primitive() {
value = bx.ptrtoint(value, bx.cx.type_isize());
// FIXME(erikdesjardins): handle non-default addrspace ptr sizes
if let Primitive::Pointer(_) = s.primitive() {
let t = bx.type_from_integer(dl.ptr_sized_integer());
value = bx.ptrtoint(value, t);
}
bx.insert_element(bx.const_undef(vec_ty), value, bx.const_i32(0))
}
Expand Down Expand Up @@ -958,7 +963,7 @@ fn llvm_fixup_output<'ll, 'tcx>(
}
(InlineAsmRegClass::AArch64(AArch64InlineAsmRegClass::vreg_low16), Abi::Scalar(s)) => {
value = bx.extract_element(value, bx.const_i32(0));
if let Primitive::Pointer = s.primitive() {
if let Primitive::Pointer(_) = s.primitive() {
value = bx.inttoptr(value, layout.llvm_type(bx.cx));
}
value
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
bx.range_metadata(load, scalar.valid_range(bx));
}
}
abi::Pointer => {
abi::Pointer(_) => {
if !scalar.valid_range(bx).contains(0) {
bx.nonnull_metadata(load);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
Scalar::Int(int) => {
let data = int.assert_bits(layout.size(self));
let llval = self.const_uint_big(self.type_ix(bitsize), data);
if layout.primitive() == Pointer {
if matches!(layout.primitive(), Pointer(_)) {
unsafe { llvm::LLVMConstIntToPtr(llval, llty) }
} else {
self.const_bitcast(llval, llty)
Expand Down Expand Up @@ -284,7 +284,7 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
1,
)
};
if layout.primitive() != Pointer {
if !matches!(layout.primitive(), Pointer(_)) {
unsafe { llvm::LLVMConstPtrToInt(llval, llty) }
} else {
self.const_bitcast(llval, llty)
Expand Down
15 changes: 4 additions & 11 deletions compiler/rustc_codegen_llvm/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,15 @@ use rustc_codegen_ssa::traits::*;
use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::mir::interpret::{
read_target_uint, Allocation, ConstAllocation, ErrorHandled, GlobalAlloc, InitChunk, Pointer,
read_target_uint, Allocation, ConstAllocation, ErrorHandled, InitChunk, Pointer,
Scalar as InterpScalar,
};
use rustc_middle::mir::mono::MonoItem;
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::{self, Instance, Ty};
use rustc_middle::{bug, span_bug};
use rustc_session::config::Lto;
use rustc_target::abi::{
AddressSpace, Align, HasDataLayout, Primitive, Scalar, Size, WrappingRange,
};
use rustc_target::abi::{Align, HasDataLayout, Primitive, Scalar, Size, WrappingRange};
use std::ops::Range;

pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: ConstAllocation<'_>) -> &'ll Value {
Expand Down Expand Up @@ -98,20 +96,15 @@ pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: ConstAllocation<
.expect("const_alloc_to_llvm: could not read relocation pointer")
as u64;

let address_space = match cx.tcx.global_alloc(alloc_id) {
GlobalAlloc::Function(..) => cx.data_layout().instruction_address_space,
GlobalAlloc::Static(..) | GlobalAlloc::Memory(..) | GlobalAlloc::VTable(..) => {
AddressSpace::DATA
}
};
let address_space = cx.tcx.global_alloc(alloc_id).address_space(cx);

llvals.push(cx.scalar_to_backend(
InterpScalar::from_pointer(
Pointer::new(alloc_id, Size::from_bytes(ptr_offset)),
&cx.tcx,
),
Scalar::Initialized {
value: Primitive::Pointer,
value: Primitive::Pointer(address_space),
valid_range: WrappingRange::full(dl.pointer_size),
},
cx.type_i8p_ext(address_space),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ fn tag_base_type<'ll, 'tcx>(
Primitive::Int(t, _) => t,
Primitive::F32 => Integer::I32,
Primitive::F64 => Integer::I64,
Primitive::Pointer => {
// FIXME(erikdesjardins): handle non-default addrspace ptr sizes
Primitive::Pointer(_) => {
// If the niche is the NULL value of a reference, then `discr_enum_ty` will be
// a RawPtr. CodeView doesn't know what to do with enums whose base type is a
// pointer so we fix this up to just be `usize`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<'ll, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'_, 'll, 'tcx> {
emit_va_arg(self, args[0], ret_ty)
}
}
Primitive::F64 | Primitive::Pointer => {
Primitive::F64 | Primitive::Pointer(_) => {
emit_va_arg(self, args[0], ret_ty)
}
// `va_arg` should never be used with the return type f32.
Expand Down
15 changes: 7 additions & 8 deletions compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_middle::bug;
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, TyAndLayout};
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::{self, Ty, TypeVisitable};
use rustc_target::abi::{Abi, AddressSpace, Align, FieldsShape};
use rustc_target::abi::{Abi, Align, FieldsShape};
use rustc_target::abi::{Int, Pointer, F32, F64};
use rustc_target::abi::{PointeeInfo, Scalar, Size, TyAbiInterface, Variants};
use smallvec::{smallvec, SmallVec};
Expand Down Expand Up @@ -312,14 +312,13 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {
Int(i, _) => cx.type_from_integer(i),
F32 => cx.type_f32(),
F64 => cx.type_f64(),
Pointer => {
Pointer(address_space) => {
// If we know the alignment, pick something better than i8.
let (pointee, address_space) =
if let Some(pointee) = self.pointee_info_at(cx, offset) {
(cx.type_pointee_for_align(pointee.align), pointee.address_space)
} else {
(cx.type_i8(), AddressSpace::DATA)
};
let pointee = if let Some(pointee) = self.pointee_info_at(cx, offset) {
cx.type_pointee_for_align(pointee.align)
} else {
cx.type_i8()
};
cx.type_ptr_to_ext(pointee, address_space)
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1801,8 +1801,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
match (src.layout.abi, dst.layout.abi) {
(abi::Abi::Scalar(src_scalar), abi::Abi::Scalar(dst_scalar)) => {
// HACK(eddyb) LLVM doesn't like `bitcast`s between pointers and non-pointers.
let src_is_ptr = src_scalar.primitive() == abi::Pointer;
let dst_is_ptr = dst_scalar.primitive() == abi::Pointer;
let src_is_ptr = matches!(src_scalar.primitive(), abi::Pointer(_));
let dst_is_ptr = matches!(dst_scalar.primitive(), abi::Pointer(_));
if src_is_ptr == dst_is_ptr {
assert_eq!(src.layout.size, dst.layout.size);

Expand Down
17 changes: 10 additions & 7 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,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::{Abi, Align, FieldsShape, Int, Pointer, TagEncoding};
use rustc_target::abi::{VariantIdx, Variants};

#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -209,6 +209,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
bx: &mut Bx,
cast_to: Ty<'tcx>,
) -> V {
let dl = &bx.tcx().data_layout;
let cast_to_layout = bx.cx().layout_of(cast_to);
let cast_to_size = cast_to_layout.layout.size();
let cast_to = bx.cx().immediate_backend_type(cast_to_layout);
Expand Down Expand Up @@ -250,12 +251,14 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
// Cast to an integer so we don't have to treat a pointer as a
// special case.
let (tag, tag_llty) = if tag_scalar.primitive().is_ptr() {
let t = bx.type_isize();
let tag = bx.ptrtoint(tag_imm, t);
(tag, t)
} else {
(tag_imm, bx.cx().immediate_backend_type(tag_op.layout))
let (tag, tag_llty) = match tag_scalar.primitive() {
// FIXME(erikdesjardins): handle non-default addrspace ptr sizes
Pointer(_) => {
let t = bx.type_from_integer(dl.ptr_sized_integer());
let tag = bx.ptrtoint(tag_imm, t);
(tag, t)
}
_ => (tag_imm, bx.cx().immediate_backend_type(tag_op.layout)),
};

let tag_size = tag_scalar.size(bx.cx());
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
assert_eq!(size, mplace.layout.size, "abi::Scalar size does not match layout size");
let scalar = alloc.read_scalar(
alloc_range(Size::ZERO, size),
/*read_provenance*/ s.is_ptr(),
/*read_provenance*/ matches!(s, abi::Pointer(_)),
)?;
Some(ImmTy { imm: scalar.into(), layout: mplace.layout })
}
Expand All @@ -335,11 +335,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
assert!(b_offset.bytes() > 0); // in `operand_field` we use the offset to tell apart the fields
let a_val = alloc.read_scalar(
alloc_range(Size::ZERO, a_size),
/*read_provenance*/ a.is_ptr(),
/*read_provenance*/ matches!(a, abi::Pointer(_)),
)?;
let b_val = alloc.read_scalar(
alloc_range(b_offset, b_size),
/*read_provenance*/ b.is_ptr(),
/*read_provenance*/ matches!(b, abi::Pointer(_)),
)?;
Some(ImmTy { imm: Immediate::ScalarPair(a_val, b_val), layout: mplace.layout })
}
Expand Down
Loading

0 comments on commit 6bf0e73

Please sign in to comment.