Skip to content

Refactoring after the PlaceValue addition #124153

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 12, 2024
Merged

Refactoring after the PlaceValue addition #124153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ pub fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
}

if src_f.layout.ty == dst_f.layout.ty {
bx.typed_place_copy(dst_f, src_f);
bx.typed_place_copy(dst_f.val, src_f.val, src_f.layout);
} else {
coerce_unsized_into(bx, src_f, dst_f);
}
Expand Down
15 changes: 7 additions & 8 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1454,9 +1454,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Some(pointee_align) => cmp::max(pointee_align, arg.layout.align.abi),
None => arg.layout.align.abi,
};
let scratch = PlaceRef::alloca_aligned(bx, arg.layout, required_align);
op.val.store(bx, scratch);
(scratch.val.llval, scratch.val.align, true)
let scratch = PlaceValue::alloca(bx, arg.layout.size, required_align);
op.val.store(bx, scratch.with_type(arg.layout));
(scratch.llval, scratch.align, true)
}
PassMode::Cast { .. } => {
let scratch = PlaceRef::alloca(bx, arg.layout);
Expand All @@ -1475,10 +1475,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// For `foo(packed.large_field)`, and types with <4 byte alignment on x86,
// alignment requirements may be higher than the type's alignment, so copy
// to a higher-aligned alloca.
let scratch = PlaceRef::alloca_aligned(bx, arg.layout, required_align);
let op_place = PlaceRef { val: op_place_val, layout: op.layout };
bx.typed_place_copy(scratch, op_place);
(scratch.val.llval, scratch.val.align, true)
let scratch = PlaceValue::alloca(bx, arg.layout.size, required_align);
bx.typed_place_copy(scratch, op_place_val, op.layout);
(scratch.llval, scratch.align, true)
} else {
(op_place_val.llval, op_place_val.align, true)
}
Expand Down Expand Up @@ -1567,7 +1566,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
if place_val.llextra.is_some() {
bug!("closure arguments must be sized");
}
let tuple_ptr = PlaceRef { val: place_val, layout: tuple.layout };
let tuple_ptr = place_val.with_type(tuple.layout);
for i in 0..tuple.layout.fields.count() {
let field_ptr = tuple_ptr.project_field(bx, i);
let field = bx.load_operand(field_ptr);
Expand Down
15 changes: 7 additions & 8 deletions compiler/rustc_codegen_ssa/src/mir/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::operand::{OperandRef, OperandValue};
use super::operand::OperandRef;
use super::place::PlaceRef;
use super::FunctionCx;
use crate::errors;
Expand Down Expand Up @@ -93,9 +93,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// into the (unoptimized) direct swapping implementation, so we disable it.
|| bx.sess().target.arch == "spirv"
{
let x_place = PlaceRef::new_sized(args[0].immediate(), pointee_layout);
let y_place = PlaceRef::new_sized(args[1].immediate(), pointee_layout);
bx.typed_place_swap(x_place, y_place);
let align = pointee_layout.align.abi;
let x_place = args[0].val.deref(align);
let y_place = args[1].val.deref(align);
bx.typed_place_swap(x_place, y_place, pointee_layout);
return Ok(());
}
}
Expand All @@ -113,15 +114,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
sym::va_end => bx.va_end(args[0].immediate()),
sym::size_of_val => {
let tp_ty = fn_args.type_at(0);
let meta =
if let OperandValue::Pair(_, meta) = args[0].val { Some(meta) } else { None };
let (_, meta) = args[0].val.pointer_parts();
let (llsize, _) = size_of_val::size_and_align_of_dst(bx, tp_ty, meta);
llsize
}
sym::min_align_of_val => {
let tp_ty = fn_args.type_at(0);
let meta =
if let OperandValue::Pair(_, meta) = args[0].val { Some(meta) } else { None };
let (_, meta) = args[0].val.pointer_parts();
let (_, llalign) = size_of_val::size_and_align_of_dst(bx, tp_ty, meta);
llalign
}
Expand Down
47 changes: 36 additions & 11 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub enum OperandValue<V> {
ZeroSized,
}

impl<V> OperandValue<V> {
impl<V: CodegenObject> OperandValue<V> {
/// If this is ZeroSized/Immediate/Pair, return an array of the 0/1/2 values.
/// If this is Ref, return the place.
#[inline]
Expand All @@ -86,6 +86,30 @@ impl<V> OperandValue<V> {
};
OperandValue::Pair(a, b)
}

/// Treat this value as a pointer and return the data pointer and
/// optional metadata as backend values.
///
/// If you're making a place, use [`Self::deref`] instead.
pub fn pointer_parts(self) -> (V, Option<V>) {
match self {
OperandValue::Immediate(llptr) => (llptr, None),
OperandValue::Pair(llptr, llextra) => (llptr, Some(llextra)),
_ => bug!("OperandValue cannot be a pointer: {self:?}"),
}
}

/// Treat this value as a pointer and return the place to which it points.
///
/// The pointer immediate doesn't inherently know its alignment,
/// so you need to pass it in. If you want to get it from a type's ABI
/// alignment, then maybe you want [`OperandRef::deref`] instead.
///
/// This is the inverse of [`PlaceValue::address`].
pub fn deref(self, align: Align) -> PlaceValue<V> {
let (llval, llextra) = self.pointer_parts();
PlaceValue { llval, llextra, align }
}
}

/// An `OperandRef` is an "SSA" reference to a Rust value, along with
Expand Down Expand Up @@ -235,6 +259,15 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
}
}

/// Asserts that this operand is a pointer (or reference) and returns
/// the place to which it points. (This requires no code to be emitted
/// as we represent places using the pointer to the place.)
///
/// This uses [`Ty::builtin_deref`] to include the type of the place and
/// assumes the place is aligned to the pointee's usual ABI alignment.
///
/// If you don't need the type, see [`OperandValue::pointer_parts`]
/// or [`OperandValue::deref`].
pub fn deref<Cx: LayoutTypeMethods<'tcx>>(self, cx: &Cx) -> PlaceRef<'tcx, V> {
if self.layout.ty.is_box() {
// Derefer should have removed all Box derefs
Expand All @@ -247,15 +280,8 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
.builtin_deref(true)
.unwrap_or_else(|| bug!("deref of non-pointer {:?}", self));

let (llptr, llextra) = match self.val {
OperandValue::Immediate(llptr) => (llptr, None),
OperandValue::Pair(llptr, llextra) => (llptr, Some(llextra)),
OperandValue::Ref(..) => bug!("Deref of by-Ref operand {:?}", self),
OperandValue::ZeroSized => bug!("Deref of ZST operand {:?}", self),
};
let layout = cx.layout_of(projected_ty);
let val = PlaceValue { llval: llptr, llextra, align: layout.align.abi };
PlaceRef { val, layout }
self.val.deref(layout.align.abi).with_type(layout)
}

/// If this operand is a `Pair`, we return an aggregate with the two values.
Expand Down Expand Up @@ -448,8 +474,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
if val.llextra.is_some() {
bug!("cannot directly store unsized values");
}
let source_place = PlaceRef { val, layout: dest.layout };
bx.typed_place_copy_with_flags(dest, source_place, flags);
bx.typed_place_copy_with_flags(dest.val, val, dest.layout, flags);
}
OperandValue::Immediate(s) => {
let val = bx.from_immediate(s);
Expand Down
87 changes: 51 additions & 36 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ 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::{Align, FieldsShape, Int, Pointer, TagEncoding};
use rustc_target::abi::{Align, FieldsShape, Int, Pointer, Size, TagEncoding};
use rustc_target::abi::{VariantIdx, Variants};

/// The location and extra runtime properties of the place.
///
/// Typically found in a [`PlaceRef`] or an [`OperandValue::Ref`].
///
/// As a location in memory, this has no specific type. If you want to
/// load or store it using a typed operation, use [`Self::with_type`].
#[derive(Copy, Clone, Debug)]
pub struct PlaceValue<V> {
/// A pointer to the contents of the place.
Expand All @@ -35,6 +38,41 @@ impl<V: CodegenObject> PlaceValue<V> {
pub fn new_sized(llval: V, align: Align) -> PlaceValue<V> {
PlaceValue { llval, llextra: None, align }
}

/// Allocates a stack slot in the function for a value
/// of the specified size and alignment.
///
/// The allocation itself is untyped.
pub fn alloca<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx, Value = V>>(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy that #122053 landed since it means that it's reasonable to alloca a PlaceValue without a type.

bx: &mut Bx,
size: Size,
align: Align,
) -> PlaceValue<V> {
let llval = bx.alloca(size, align);
PlaceValue::new_sized(llval, align)
}

/// Creates a `PlaceRef` to this location with the given type.
pub fn with_type<'tcx>(self, layout: TyAndLayout<'tcx>) -> PlaceRef<'tcx, V> {
debug_assert!(
layout.is_unsized() || layout.abi.is_uninhabited() || self.llextra.is_none(),
"Had pointer metadata {:?} for sized type {layout:?}",
self.llextra,
);
PlaceRef { val: self, layout }
}

/// Gets the pointer to this place as an [`OperandValue::Immediate`]
/// or, for those needing metadata, an [`OperandValue::Pair`].
///
/// This is the inverse of [`OperandValue::deref`].
pub fn address(self) -> OperandValue<V> {
if let Some(llextra) = self.llextra {
OperandValue::Pair(self.llval, llextra)
} else {
OperandValue::Immediate(self.llval)
}
}
}

#[derive(Copy, Clone, Debug)]
Expand All @@ -52,9 +90,7 @@ pub struct PlaceRef<'tcx, V> {

impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
pub fn new_sized(llval: V, layout: TyAndLayout<'tcx>) -> PlaceRef<'tcx, V> {
assert!(layout.is_sized());
let val = PlaceValue::new_sized(llval, layout.align.abi);
PlaceRef { val, layout }
PlaceRef::new_sized_aligned(llval, layout, layout.align.abi)
}

pub fn new_sized_aligned(
Expand All @@ -63,27 +99,17 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
align: Align,
) -> PlaceRef<'tcx, V> {
assert!(layout.is_sized());
let val = PlaceValue::new_sized(llval, align);
PlaceRef { val, layout }
PlaceValue::new_sized(llval, align).with_type(layout)
}

// FIXME(eddyb) pass something else for the name so no work is done
// unless LLVM IR names are turned on (e.g. for `--emit=llvm-ir`).
pub fn alloca<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
bx: &mut Bx,
layout: TyAndLayout<'tcx>,
) -> Self {
Self::alloca_aligned(bx, layout, layout.align.abi)
}

pub fn alloca_aligned<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
bx: &mut Bx,
layout: TyAndLayout<'tcx>,
align: Align,
) -> Self {
assert!(layout.is_sized(), "tried to statically allocate unsized place");
let tmp = bx.alloca(layout.size, align);
Self::new_sized_aligned(tmp, layout, align)
PlaceValue::alloca(bx, layout.size, layout.align.abi).with_type(layout)
}

/// Returns a place for an indirect reference to an unsized place.
Expand Down Expand Up @@ -132,18 +158,12 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
} else {
bx.inbounds_ptradd(self.val.llval, bx.const_usize(offset.bytes()))
};
PlaceRef {
val: PlaceValue {
let val = PlaceValue {
llval,
llextra: if bx.cx().type_has_metadata(field.ty) {
self.val.llextra
} else {
None
},
llextra: if bx.cx().type_has_metadata(field.ty) { self.val.llextra } else { None },
align: effective_field_align,
},
layout: field,
}
};
val.with_type(field)
};

// Simple cases, which don't need DST adjustment:
Expand Down Expand Up @@ -198,7 +218,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
let ptr = bx.inbounds_ptradd(self.val.llval, offset);
let val =
PlaceValue { llval: ptr, llextra: self.val.llextra, align: effective_field_align };
PlaceRef { val, layout: field }
val.with_type(field)
}

/// Obtain the actual discriminant of a value.
Expand Down Expand Up @@ -387,18 +407,13 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
layout.size
};

PlaceRef {
val: PlaceValue {
llval: bx.inbounds_gep(
let llval = bx.inbounds_gep(
bx.cx().backend_type(self.layout),
self.val.llval,
&[bx.cx().const_usize(0), llindex],
),
llextra: None,
align: self.val.align.restrict_for_offset(offset),
},
layout,
}
);
let align = self.val.align.restrict_for_offset(offset);
PlaceValue::new_sized(llval, align).with_type(layout)
}

pub fn project_downcast<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
Expand Down
Loading
Loading