Skip to content

Commit a00aac1

Browse files
committed
Rework OperandRef::extract_field to stop calling to_immediate_scalar on things which are already immediates
That means it stops trying to truncate things that are already `i1`s.
1 parent 6629a40 commit a00aac1

File tree

7 files changed

+96
-84
lines changed

7 files changed

+96
-84
lines changed

compiler/rustc_codegen_gcc/src/builder.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -989,10 +989,14 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
989989
OperandValue::Ref(place.val)
990990
} else if place.layout.is_gcc_immediate() {
991991
let load = self.load(place.layout.gcc_type(self), place.val.llval, place.val.align);
992-
if let abi::BackendRepr::Scalar(ref scalar) = place.layout.backend_repr {
993-
scalar_load_metadata(self, load, scalar);
994-
}
995-
OperandValue::Immediate(self.to_immediate(load, place.layout))
992+
OperandValue::Immediate(
993+
if let abi::BackendRepr::Scalar(ref scalar) = place.layout.backend_repr {
994+
scalar_load_metadata(self, load, scalar);
995+
self.to_immediate_scalar(load, scalar)
996+
} else {
997+
load
998+
},
999+
)
9961000
} else if let abi::BackendRepr::ScalarPair(ref a, ref b) = place.layout.backend_repr {
9971001
let b_offset = a.size(self).align_to(b.align(self).abi);
9981002

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -181,14 +181,19 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc
181181
sym::volatile_load | sym::unaligned_volatile_load => {
182182
let tp_ty = fn_args.type_at(0);
183183
let ptr = args[0].immediate();
184+
let layout = self.layout_of(tp_ty);
184185
let load = if let PassMode::Cast { cast: ref ty, pad_i32: _ } = fn_abi.ret.mode {
185186
let gcc_ty = ty.gcc_type(self);
186187
self.volatile_load(gcc_ty, ptr)
187188
} else {
188-
self.volatile_load(self.layout_of(tp_ty).gcc_type(self), ptr)
189+
self.volatile_load(layout.gcc_type(self), ptr)
189190
};
190191
// TODO(antoyo): set alignment.
191-
self.to_immediate(load, self.layout_of(tp_ty))
192+
if let BackendRepr::Scalar(scalar) = layout.backend_repr {
193+
self.to_immediate_scalar(load, scalar)
194+
} else {
195+
load
196+
}
192197
}
193198
sym::volatile_store => {
194199
let dst = args[0].deref(self.cx());

compiler/rustc_codegen_llvm/src/builder.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -702,10 +702,12 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
702702
let load = self.load(llty, place.val.llval, place.val.align);
703703
if let abi::BackendRepr::Scalar(scalar) = place.layout.backend_repr {
704704
scalar_load_metadata(self, load, scalar, place.layout, Size::ZERO);
705+
self.to_immediate_scalar(load, scalar)
706+
} else {
707+
load
705708
}
706-
load
707709
});
708-
OperandValue::Immediate(self.to_immediate(llval, place.layout))
710+
OperandValue::Immediate(llval)
709711
} else if let abi::BackendRepr::ScalarPair(a, b) = place.layout.backend_repr {
710712
let b_offset = a.size(self).align_to(b.align(self).abi);
711713

@@ -899,6 +901,8 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
899901
}
900902

901903
fn unchecked_utrunc(&mut self, val: &'ll Value, dest_ty: &'ll Type) -> &'ll Value {
904+
debug_assert_ne!(self.val_ty(val), dest_ty);
905+
902906
let trunc = self.trunc(val, dest_ty);
903907
if llvm_util::get_version() >= (19, 0, 0) {
904908
unsafe {
@@ -911,6 +915,8 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
911915
}
912916

913917
fn unchecked_strunc(&mut self, val: &'ll Value, dest_ty: &'ll Type) -> &'ll Value {
918+
debug_assert_ne!(self.val_ty(val), dest_ty);
919+
914920
let trunc = self.trunc(val, dest_ty);
915921
if llvm_util::get_version() >= (19, 0, 0) {
916922
unsafe {

compiler/rustc_codegen_ssa/src/mir/block.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
10171017
let (idx, _) = op.layout.non_1zst_field(bx).expect(
10181018
"not exactly one non-1-ZST field in a `DispatchFromDyn` type",
10191019
);
1020-
op = op.extract_field(bx, idx);
1020+
op = op.extract_field(self, bx, idx);
10211021
}
10221022

10231023
// Now that we have `*dyn Trait` or `&dyn Trait`, split it up into its
@@ -1049,7 +1049,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
10491049
let (idx, _) = op.layout.non_1zst_field(bx).expect(
10501050
"not exactly one non-1-ZST field in a `DispatchFromDyn` type",
10511051
);
1052-
op = op.extract_field(bx, idx);
1052+
op = op.extract_field(self, bx, idx);
10531053
}
10541054

10551055
// Make sure that we've actually unwrapped the rcvr down
@@ -1549,9 +1549,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
15491549
if scalar.is_bool() {
15501550
bx.range_metadata(llval, WrappingRange { start: 0, end: 1 });
15511551
}
1552+
// We store bools as `i8` so we need to truncate to `i1`.
1553+
llval = bx.to_immediate_scalar(llval, scalar);
15521554
}
1553-
// We store bools as `i8` so we need to truncate to `i1`.
1554-
llval = bx.to_immediate(llval, arg.layout);
15551555
}
15561556
}
15571557

@@ -1581,7 +1581,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
15811581
} else {
15821582
// If the tuple is immediate, the elements are as well.
15831583
for i in 0..tuple.layout.fields.count() {
1584-
let op = tuple.extract_field(bx, i);
1584+
let op = tuple.extract_field(self, bx, i);
15851585
self.codegen_argument(bx, op, llargs, &args[i]);
15861586
}
15871587
}

compiler/rustc_codegen_ssa/src/mir/operand.rs

+64-62
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ use arrayvec::ArrayVec;
55
use either::Either;
66
use rustc_abi as abi;
77
use rustc_abi::{Align, BackendRepr, Size};
8-
use rustc_middle::bug;
98
use rustc_middle::mir::interpret::{Pointer, Scalar, alloc_range};
109
use rustc_middle::mir::{self, ConstValue};
1110
use rustc_middle::ty::Ty;
1211
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
12+
use rustc_middle::{bug, span_bug};
1313
use tracing::debug;
1414

1515
use super::place::{PlaceRef, PlaceValue};
@@ -352,79 +352,81 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
352352

353353
pub(crate) fn extract_field<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
354354
&self,
355+
fx: &mut FunctionCx<'a, 'tcx, Bx>,
355356
bx: &mut Bx,
356357
i: usize,
357358
) -> Self {
358359
let field = self.layout.field(bx.cx(), i);
359360
let offset = self.layout.fields.offset(i);
360361

361-
let mut val = match (self.val, self.layout.backend_repr) {
362-
// If the field is ZST, it has no data.
363-
_ if field.is_zst() => OperandValue::ZeroSized,
362+
let val = if field.size == self.layout.size
363+
&& let Some(field_val) = fx.codegen_transmute_operand(bx, *self, field)
364+
{
365+
assert_eq!(offset.bytes(), 0);
366+
field_val
367+
} else if field.is_zst() {
368+
OperandValue::ZeroSized
369+
} else {
370+
let (in_scalar, imm) = match (self.val, self.layout.backend_repr) {
371+
// Extract a scalar component from a pair.
372+
(OperandValue::Pair(a_llval, b_llval), BackendRepr::ScalarPair(a, b)) => {
373+
if offset.bytes() == 0 {
374+
assert_eq!(field.size, a.size(bx.cx()));
375+
(Some(a), a_llval)
376+
} else {
377+
assert_eq!(offset, a.size(bx.cx()).align_to(b.align(bx.cx()).abi));
378+
assert_eq!(field.size, b.size(bx.cx()));
379+
(Some(b), b_llval)
380+
}
381+
}
364382

365-
// Newtype of a scalar, scalar pair or vector.
366-
(OperandValue::Immediate(_) | OperandValue::Pair(..), _)
367-
if field.size == self.layout.size =>
368-
{
369-
assert_eq!(offset.bytes(), 0);
370-
self.val
371-
}
383+
// `#[repr(simd)]` types are also immediate.
384+
(OperandValue::Immediate(llval), BackendRepr::Vector { .. }) => {
385+
(None, bx.extract_element(llval, bx.cx().const_usize(i as u64)))
386+
}
372387

373-
// Extract a scalar component from a pair.
374-
(OperandValue::Pair(a_llval, b_llval), BackendRepr::ScalarPair(a, b)) => {
375-
if offset.bytes() == 0 {
376-
assert_eq!(field.size, a.size(bx.cx()));
377-
OperandValue::Immediate(a_llval)
378-
} else {
379-
assert_eq!(offset, a.size(bx.cx()).align_to(b.align(bx.cx()).abi));
380-
assert_eq!(field.size, b.size(bx.cx()));
381-
OperandValue::Immediate(b_llval)
388+
_ => {
389+
span_bug!(fx.mir.span, "OperandRef::extract_field({:?}): not applicable", self)
382390
}
383-
}
391+
};
392+
OperandValue::Immediate(match field.backend_repr {
393+
BackendRepr::Vector { .. } => imm,
394+
BackendRepr::Scalar(out_scalar) => {
395+
let Some(in_scalar) = in_scalar else {
396+
span_bug!(
397+
fx.mir.span,
398+
"OperandRef::extract_field({:?}): missing input scalar for output scalar",
399+
self
400+
)
401+
};
402+
if in_scalar != out_scalar {
403+
// If the backend and backend_immediate types might differ,
404+
// flip back to the backend type then to the new immediate.
405+
// This avoids nop truncations, but still handles things like
406+
// Bools in union fields needs to be truncated.
407+
let backend = bx.from_immediate(imm);
408+
bx.to_immediate_scalar(backend, out_scalar)
409+
} else {
410+
imm
411+
}
412+
}
413+
// Newtype vector of array, e.g. #[repr(simd)] struct S([i32; 4]);
414+
BackendRepr::Memory { sized: true } => {
415+
assert_matches!(self.layout.backend_repr, BackendRepr::Vector { .. });
384416

385-
// `#[repr(simd)]` types are also immediate.
386-
(OperandValue::Immediate(llval), BackendRepr::Vector { .. }) => {
387-
OperandValue::Immediate(bx.extract_element(llval, bx.cx().const_usize(i as u64)))
388-
}
417+
let llfield_ty = bx.cx().backend_type(field);
389418

390-
_ => bug!("OperandRef::extract_field({:?}): not applicable", self),
419+
// Can't bitcast an aggregate, so round trip through memory.
420+
let llptr = bx.alloca(field.size, field.align.abi);
421+
bx.store(imm, llptr, field.align.abi);
422+
bx.load(llfield_ty, llptr, field.align.abi)
423+
}
424+
BackendRepr::Uninhabited
425+
| BackendRepr::ScalarPair(_, _)
426+
| BackendRepr::Memory { sized: false } => bug!(),
427+
})
391428
};
392429

393-
match (&mut val, field.backend_repr) {
394-
(OperandValue::ZeroSized, _) => {}
395-
(
396-
OperandValue::Immediate(llval),
397-
BackendRepr::Scalar(_) | BackendRepr::ScalarPair(..) | BackendRepr::Vector { .. },
398-
) => {
399-
// Bools in union fields needs to be truncated.
400-
*llval = bx.to_immediate(*llval, field);
401-
}
402-
(OperandValue::Pair(a, b), BackendRepr::ScalarPair(a_abi, b_abi)) => {
403-
// Bools in union fields needs to be truncated.
404-
*a = bx.to_immediate_scalar(*a, a_abi);
405-
*b = bx.to_immediate_scalar(*b, b_abi);
406-
}
407-
// Newtype vector of array, e.g. #[repr(simd)] struct S([i32; 4]);
408-
(OperandValue::Immediate(llval), BackendRepr::Memory { sized: true }) => {
409-
assert_matches!(self.layout.backend_repr, BackendRepr::Vector { .. });
410-
411-
let llfield_ty = bx.cx().backend_type(field);
412-
413-
// Can't bitcast an aggregate, so round trip through memory.
414-
let llptr = bx.alloca(field.size, field.align.abi);
415-
bx.store(*llval, llptr, field.align.abi);
416-
*llval = bx.load(llfield_ty, llptr, field.align.abi);
417-
}
418-
(
419-
OperandValue::Immediate(_),
420-
BackendRepr::Uninhabited | BackendRepr::Memory { sized: false },
421-
) => {
422-
bug!()
423-
}
424-
(OperandValue::Pair(..), _) => bug!(),
425-
(OperandValue::Ref(..), _) => bug!(),
426-
}
427-
428430
OperandRef { val, layout: field }
429431
}
430432
}
@@ -587,7 +589,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
587589
"Bad PlaceRef: destructing pointers should use cast/PtrMetadata, \
588590
but tried to access field {f:?} of pointer {o:?}",
589591
);
590-
o = o.extract_field(bx, f.index());
592+
o = o.extract_field(self, bx, f.index());
591593
}
592594
mir::ProjectionElem::Index(_)
593595
| mir::ProjectionElem::ConstantIndex { .. } => {

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
231231
///
232232
/// Returns `None` for cases that can't work in that framework, such as for
233233
/// `Immediate`->`Ref` that needs an `alloc` to get the location.
234-
fn codegen_transmute_operand(
234+
pub(crate) fn codegen_transmute_operand(
235235
&mut self,
236236
bx: &mut Bx,
237237
operand: OperandRef<'tcx, Bx::Value>,
@@ -260,6 +260,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
260260
OperandValue::Ref(source_place_val) => {
261261
assert_eq!(source_place_val.llextra, None);
262262
assert_matches!(operand_kind, OperandValueKind::Ref);
263+
// The existing alignment is part of `source_place_val`,
264+
// so that alignment will be used, not `cast`'s.
263265
Some(bx.load_operand(source_place_val.with_type(cast)).val)
264266
}
265267
OperandValue::ZeroSized => {

compiler/rustc_codegen_ssa/src/traits/builder.rs

+1-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::assert_matches::assert_matches;
22
use std::ops::Deref;
33

4-
use rustc_abi::{Align, BackendRepr, Scalar, Size, WrappingRange};
4+
use rustc_abi::{Align, Scalar, Size, WrappingRange};
55
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
66
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, TyAndLayout};
77
use rustc_middle::ty::{Instance, Ty};
@@ -209,13 +209,6 @@ pub trait BuilderMethods<'a, 'tcx>:
209209
) -> (Self::Value, Self::Value);
210210

211211
fn from_immediate(&mut self, val: Self::Value) -> Self::Value;
212-
fn to_immediate(&mut self, val: Self::Value, layout: TyAndLayout<'_>) -> Self::Value {
213-
if let BackendRepr::Scalar(scalar) = layout.backend_repr {
214-
self.to_immediate_scalar(val, scalar)
215-
} else {
216-
val
217-
}
218-
}
219212
fn to_immediate_scalar(&mut self, val: Self::Value, scalar: Scalar) -> Self::Value;
220213

221214
fn alloca(&mut self, size: Size, align: Align) -> Self::Value;

0 commit comments

Comments
 (0)