Skip to content

Commit 9be16eb

Browse files
scottmcmscottmcm
committed
Refactoring after the PlaceValue addition
I added `PlaceValue` in 123775, but kept that one line-by-line simple because it touched so many places. This goes through to add more helpers & docs, and change some `PlaceRef` to `PlaceValue` where the type didn't need to be included. No behaviour changes.
1 parent 19dacee commit 9be16eb

File tree

7 files changed

+152
-127
lines changed

7 files changed

+152
-127
lines changed

‎compiler/rustc_codegen_ssa/src/base.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ pub fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
283283
}
284284

285285
if src_f.layout.ty == dst_f.layout.ty {
286-
bx.typed_place_copy(dst_f, src_f);
286+
bx.typed_place_copy(dst_f.val, src_f.val, src_f.layout);
287287
} else {
288288
coerce_unsized_into(bx, src_f, dst_f);
289289
}

‎compiler/rustc_codegen_ssa/src/mir/block.rs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,9 +1454,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
14541454
Some(pointee_align) => cmp::max(pointee_align, arg.layout.align.abi),
14551455
None => arg.layout.align.abi,
14561456
};
1457-
let scratch = PlaceRef::alloca_aligned(bx, arg.layout, required_align);
1458-
op.val.store(bx, scratch);
1459-
(scratch.val.llval, scratch.val.align, true)
1457+
let scratch = PlaceValue::alloca(bx, arg.layout.size, required_align);
1458+
op.val.store(bx, scratch.with_type(arg.layout));
1459+
(scratch.llval, scratch.align, true)
14601460
}
14611461
PassMode::Cast { .. } => {
14621462
let scratch = PlaceRef::alloca(bx, arg.layout);
@@ -1475,10 +1475,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
14751475
// For `foo(packed.large_field)`, and types with <4 byte alignment on x86,
14761476
// alignment requirements may be higher than the type's alignment, so copy
14771477
// to a higher-aligned alloca.
1478-
let scratch = PlaceRef::alloca_aligned(bx, arg.layout, required_align);
1479-
let op_place = PlaceRef { val: op_place_val, layout: op.layout };
1480-
bx.typed_place_copy(scratch, op_place);
1481-
(scratch.val.llval, scratch.val.align, true)
1478+
let scratch = PlaceValue::alloca(bx, arg.layout.size, required_align);
1479+
bx.typed_place_copy(scratch, op_place_val, op.layout);
1480+
(scratch.llval, scratch.align, true)
14821481
} else {
14831482
(op_place_val.llval, op_place_val.align, true)
14841483
}
@@ -1567,7 +1566,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
15671566
if place_val.llextra.is_some() {
15681567
bug!("closure arguments must be sized");
15691568
}
1570-
let tuple_ptr = PlaceRef { val: place_val, layout: tuple.layout };
1569+
let tuple_ptr = place_val.with_type(tuple.layout);
15711570
for i in 0..tuple.layout.fields.count() {
15721571
let field_ptr = tuple_ptr.project_field(bx, i);
15731572
let field = bx.load_operand(field_ptr);

‎compiler/rustc_codegen_ssa/src/mir/intrinsic.rs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::operand::{OperandRef, OperandValue};
1+
use super::operand::OperandRef;
22
use super::place::PlaceRef;
33
use super::FunctionCx;
44
use crate::errors;
@@ -93,9 +93,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
9393
// into the (unoptimized) direct swapping implementation, so we disable it.
9494
|| bx.sess().target.arch == "spirv"
9595
{
96-
let x_place = PlaceRef::new_sized(args[0].immediate(), pointee_layout);
97-
let y_place = PlaceRef::new_sized(args[1].immediate(), pointee_layout);
98-
bx.typed_place_swap(x_place, y_place);
96+
let align = pointee_layout.align.abi;
97+
let x_place = args[0].val.deref(align);
98+
let y_place = args[1].val.deref(align);
99+
bx.typed_place_swap(x_place, y_place, pointee_layout);
99100
return Ok(());
100101
}
101102
}
@@ -113,15 +114,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
113114
sym::va_end => bx.va_end(args[0].immediate()),
114115
sym::size_of_val => {
115116
let tp_ty = fn_args.type_at(0);
116-
let meta =
117-
if let OperandValue::Pair(_, meta) = args[0].val { Some(meta) } else { None };
117+
let (_, meta) = args[0].val.pointer_parts();
118118
let (llsize, _) = size_of_val::size_and_align_of_dst(bx, tp_ty, meta);
119119
llsize
120120
}
121121
sym::min_align_of_val => {
122122
let tp_ty = fn_args.type_at(0);
123-
let meta =
124-
if let OperandValue::Pair(_, meta) = args[0].val { Some(meta) } else { None };
123+
let (_, meta) = args[0].val.pointer_parts();
125124
let (_, llalign) = size_of_val::size_and_align_of_dst(bx, tp_ty, meta);
126125
llalign
127126
}

‎compiler/rustc_codegen_ssa/src/mir/operand.rs‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ pub enum OperandValue<V> {
6161
ZeroSized,
6262
}
6363

64-
impl<V> OperandValue<V> {
64+
impl<V: CodegenObject> OperandValue<V> {
6565
/// If this is ZeroSized/Immediate/Pair, return an array of the 0/1/2 values.
6666
/// If this is Ref, return the place.
6767
#[inline]
@@ -86,6 +86,30 @@ impl<V> OperandValue<V> {
8686
};
8787
OperandValue::Pair(a, b)
8888
}
89+
90+
/// Treat this value as a pointer and return the data pointer and
91+
/// optional metadata as backend values.
92+
///
93+
/// If you're making a place, use [`Self::deref`] instead.
94+
pub fn pointer_parts(self) -> (V, Option<V>) {
95+
match self {
96+
OperandValue::Immediate(llptr) => (llptr, None),
97+
OperandValue::Pair(llptr, llextra) => (llptr, Some(llextra)),
98+
_ => bug!("OperandValue cannot be a pointer: {self:?}"),
99+
}
100+
}
101+
102+
/// Treat this value as a pointer and return the place to which it points.
103+
///
104+
/// The pointer immediate doesn't inherently know its alignment,
105+
/// so you need to pass it in. If you want to get it from a type's ABI
106+
/// alignment, then maybe you want [`OperandRef::deref`] instead.
107+
///
108+
/// This is the inverse of [`PlaceValue::address`].
109+
pub fn deref(self, align: Align) -> PlaceValue<V> {
110+
let (llval, llextra) = self.pointer_parts();
111+
PlaceValue { llval, llextra, align }
112+
}
89113
}
90114

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

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

250-
let (llptr, llextra) = match self.val {
251-
OperandValue::Immediate(llptr) => (llptr, None),
252-
OperandValue::Pair(llptr, llextra) => (llptr, Some(llextra)),
253-
OperandValue::Ref(..) => bug!("Deref of by-Ref operand {:?}", self),
254-
OperandValue::ZeroSized => bug!("Deref of ZST operand {:?}", self),
255-
};
256283
let layout = cx.layout_of(projected_ty);
257-
let val = PlaceValue { llval: llptr, llextra, align: layout.align.abi };
258-
PlaceRef { val, layout }
284+
self.val.deref(layout.align.abi).with_type(layout)
259285
}
260286

261287
/// If this operand is a `Pair`, we return an aggregate with the two values.
@@ -448,8 +474,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
448474
if val.llextra.is_some() {
449475
bug!("cannot directly store unsized values");
450476
}
451-
let source_place = PlaceRef { val, layout: dest.layout };
452-
bx.typed_place_copy_with_flags(dest, source_place, flags);
477+
bx.typed_place_copy_with_flags(dest.val, val, dest.layout, flags);
453478
}
454479
OperandValue::Immediate(s) => {
455480
let val = bx.from_immediate(s);

‎compiler/rustc_codegen_ssa/src/mir/place.rs‎

Lines changed: 51 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,15 @@ use rustc_middle::mir;
1010
use rustc_middle::mir::tcx::PlaceTy;
1111
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout};
1212
use rustc_middle::ty::{self, Ty};
13-
use rustc_target::abi::{Align, FieldsShape, Int, Pointer, TagEncoding};
13+
use rustc_target::abi::{Align, FieldsShape, Int, Pointer, Size, TagEncoding};
1414
use rustc_target::abi::{VariantIdx, Variants};
1515

1616
/// The location and extra runtime properties of the place.
1717
///
1818
/// Typically found in a [`PlaceRef`] or an [`OperandValue::Ref`].
19+
///
20+
/// As a location in memory, this has no specific type. If you want to
21+
/// load or store it using a typed operation, use [`Self::with_type`].
1922
#[derive(Copy, Clone, Debug)]
2023
pub struct PlaceValue<V> {
2124
/// A pointer to the contents of the place.
@@ -35,6 +38,41 @@ impl<V: CodegenObject> PlaceValue<V> {
3538
pub fn new_sized(llval: V, align: Align) -> PlaceValue<V> {
3639
PlaceValue { llval, llextra: None, align }
3740
}
41+
42+
/// Allocates a stack slot in the function for a value
43+
/// of the specified size and alignment.
44+
///
45+
/// The allocation itself is untyped.
46+
pub fn alloca<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx, Value = V>>(
47+
bx: &mut Bx,
48+
size: Size,
49+
align: Align,
50+
) -> PlaceValue<V> {
51+
let llval = bx.alloca(size, align);
52+
PlaceValue::new_sized(llval, align)
53+
}
54+
55+
/// Creates a `PlaceRef` to this location with the given type.
56+
pub fn with_type<'tcx>(self, layout: TyAndLayout<'tcx>) -> PlaceRef<'tcx, V> {
57+
debug_assert!(
58+
layout.is_unsized() || layout.abi.is_uninhabited() || self.llextra.is_none(),
59+
"Had pointer metadata {:?} for sized type {layout:?}",
60+
self.llextra,
61+
);
62+
PlaceRef { val: self, layout }
63+
}
64+
65+
/// Gets the pointer to this place as an [`OperandValue::Immediate`]
66+
/// or, for those needing metadata, an [`OperandValue::Pair`].
67+
///
68+
/// This is the inverse of [`OperandValue::deref`].
69+
pub fn address(self) -> OperandValue<V> {
70+
if let Some(llextra) = self.llextra {
71+
OperandValue::Pair(self.llval, llextra)
72+
} else {
73+
OperandValue::Immediate(self.llval)
74+
}
75+
}
3876
}
3977

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

5391
impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
5492
pub fn new_sized(llval: V, layout: TyAndLayout<'tcx>) -> PlaceRef<'tcx, V> {
55-
assert!(layout.is_sized());
56-
let val = PlaceValue::new_sized(llval, layout.align.abi);
57-
PlaceRef { val, layout }
93+
PlaceRef::new_sized_aligned(llval, layout, layout.align.abi)
5894
}
5995

6096
pub fn new_sized_aligned(
@@ -63,27 +99,17 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
6399
align: Align,
64100
) -> PlaceRef<'tcx, V> {
65101
assert!(layout.is_sized());
66-
let val = PlaceValue::new_sized(llval, align);
67-
PlaceRef { val, layout }
102+
PlaceValue::new_sized(llval, align).with_type(layout)
68103
}
69104

70105
// FIXME(eddyb) pass something else for the name so no work is done
71106
// unless LLVM IR names are turned on (e.g. for `--emit=llvm-ir`).
72107
pub fn alloca<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
73108
bx: &mut Bx,
74109
layout: TyAndLayout<'tcx>,
75-
) -> Self {
76-
Self::alloca_aligned(bx, layout, layout.align.abi)
77-
}
78-
79-
pub fn alloca_aligned<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
80-
bx: &mut Bx,
81-
layout: TyAndLayout<'tcx>,
82-
align: Align,
83110
) -> Self {
84111
assert!(layout.is_sized(), "tried to statically allocate unsized place");
85-
let tmp = bx.alloca(layout.size, align);
86-
Self::new_sized_aligned(tmp, layout, align)
112+
PlaceValue::alloca(bx, layout.size, layout.align.abi).with_type(layout)
87113
}
88114

89115
/// Returns a place for an indirect reference to an unsized place.
@@ -132,18 +158,12 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
132158
} else {
133159
bx.inbounds_ptradd(self.val.llval, bx.const_usize(offset.bytes()))
134160
};
135-
PlaceRef {
136-
val: PlaceValue {
161+
let val = PlaceValue {
137162
llval,
138-
llextra: if bx.cx().type_has_metadata(field.ty) {
139-
self.val.llextra
140-
} else {
141-
None
142-
},
163+
llextra: if bx.cx().type_has_metadata(field.ty) { self.val.llextra } else { None },
143164
align: effective_field_align,
144-
},
145-
layout: field,
146-
}
165+
};
166+
val.with_type(field)
147167
};
148168

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

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

390-
PlaceRef {
391-
val: PlaceValue {
392-
llval: bx.inbounds_gep(
410+
let llval = bx.inbounds_gep(
393411
bx.cx().backend_type(self.layout),
394412
self.val.llval,
395413
&[bx.cx().const_usize(0), llindex],
396-
),
397-
llextra: None,
398-
align: self.val.align.restrict_for_offset(offset),
399-
},
400-
layout,
401-
}
414+
);
415+
let align = self.val.align.restrict_for_offset(offset);
416+
PlaceValue::new_sized(llval, align).with_type(layout)
402417
}
403418

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

0 commit comments

Comments
 (0)