Skip to content

Commit 1d0d3d3

Browse files
committed
Auto merge of rust-lang#123886 - scottmcm:more-rvalue-operands, r=<try>
Avoid `alloca`s in codegen for simple pairs and simple transparent structs Even something simple like constructing a ```rust #[repr(transparent)] struct Foo(u32); ``` forces an `alloca` to be generated in nightly right now. Certainly LLVM can optimize that away, but it would be nice if it didn't have to. Quick example: ```rust #[repr(transparent)] pub struct Transparent32(u32); #[no_mangle] pub fn make_transparent(x: u32) -> Transparent32 { let a = Transparent32(x); a } ``` on nightly we produce <https://rust.godbolt.org/z/zcvoM79ae> ```llvm define noundef i32 `@make_transparent(i32` noundef %x) unnamed_addr #0 { %a = alloca i32, align 4 store i32 %x, ptr %a, align 4 %0 = load i32, ptr %a, align 4, !noundef !3 ret i32 %0 } ``` but after this PR we produce ```llvm define noundef i32 `@make_transparent(i32` noundef %x) unnamed_addr #0 { start: ret i32 %x } ``` (even before the optimizer runs).
2 parents 6cfd809 + 7cb89e3 commit 1d0d3d3

File tree

6 files changed

+143
-13
lines changed

6 files changed

+143
-13
lines changed

compiler/rustc_codegen_cranelift/src/vtable.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ pub(crate) fn get_ptr_and_method_ref<'tcx>(
5353
.layout()
5454
.non_1zst_field(fx)
5555
.expect("not exactly one non-1-ZST field in a `DispatchFromDyn` type");
56-
arg = arg.value_field(fx, FieldIdx::new(idx));
56+
arg = arg.value_field(fx, idx);
5757
}
5858
}
5959

compiler/rustc_codegen_ssa/src/mir/block.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
10191019
let (idx, _) = op.layout.non_1zst_field(bx).expect(
10201020
"not exactly one non-1-ZST field in a `DispatchFromDyn` type",
10211021
);
1022-
op = op.extract_field(bx, idx);
1022+
op = op.extract_field(bx, idx.as_usize());
10231023
}
10241024

10251025
// now that we have `*dyn Trait` or `&dyn Trait`, split it up into its
@@ -1051,7 +1051,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
10511051
let (idx, _) = op.layout.non_1zst_field(bx).expect(
10521052
"not exactly one non-1-ZST field in a `DispatchFromDyn` type",
10531053
);
1054-
op = op.extract_field(bx, idx);
1054+
op = op.extract_field(bx, idx.as_usize());
10551055
}
10561056

10571057
// Make sure that we've actually unwrapped the rcvr down

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+64-7
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout};
1515
use rustc_middle::ty::{self, adjustment::PointerCoercion, Instance, Ty, TyCtxt};
1616
use rustc_session::config::OptLevel;
1717
use rustc_span::{Span, DUMMY_SP};
18-
use rustc_target::abi::{self, FIRST_VARIANT};
18+
use rustc_target::abi::{self, FieldIdx, FIRST_VARIANT};
1919

2020
impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
2121
#[instrument(level = "trace", skip(self, bx))]
@@ -720,12 +720,44 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
720720
OperandRef { val: OperandValue::Immediate(static_), layout }
721721
}
722722
mir::Rvalue::Use(ref operand) => self.codegen_operand(bx, operand),
723-
mir::Rvalue::Repeat(..) | mir::Rvalue::Aggregate(..) => {
723+
mir::Rvalue::Repeat(..) => {
724724
// According to `rvalue_creates_operand`, only ZST
725-
// aggregate rvalues are allowed to be operands.
725+
// repat rvalues are allowed to be operands.
726726
let ty = rvalue.ty(self.mir, self.cx.tcx());
727727
OperandRef::zero_sized(self.cx.layout_of(self.monomorphize(ty)))
728728
}
729+
mir::Rvalue::Aggregate(ref kind, ref fields) => {
730+
let ty = rvalue.ty(self.mir, self.cx.tcx());
731+
let ty = self.monomorphize(ty);
732+
let layout = self.cx.layout_of(self.monomorphize(ty));
733+
match **kind {
734+
_ if layout.is_zst() => OperandRef::zero_sized(layout),
735+
mir::AggregateKind::Tuple => {
736+
debug_assert_eq!(
737+
fields.len(),
738+
2,
739+
"We should only get pairs, but got {rvalue:?}"
740+
);
741+
let a = self.codegen_operand(bx, &fields[FieldIdx::ZERO]);
742+
let b = self.codegen_operand(bx, &fields[FieldIdx::from_u32(1)]);
743+
let val = OperandValue::Pair(a.immediate(), b.immediate());
744+
OperandRef { val, layout }
745+
}
746+
mir::AggregateKind::Adt(..) => {
747+
let (field_idx, _) = layout
748+
.non_1zst_field(self.cx)
749+
.expect("only transparent non-ZST structs should get here");
750+
let field = self.codegen_operand(bx, &fields[field_idx]);
751+
// While the size is the same, since the struct is transparent,
752+
// calling transmute here handles the `i1`-vs-`i8` issues for `bool`.
753+
let Some(val) = self.codegen_transmute_operand(bx, field, layout) else {
754+
bug!("Couldn't transmute {field:?} to {layout:?}");
755+
};
756+
OperandRef { val, layout }
757+
}
758+
_ => bug!("Unexpected in codegen_rvalue_operand: {rvalue:?}"),
759+
}
760+
}
729761
mir::Rvalue::ShallowInitBox(ref operand, content_ty) => {
730762
let operand = self.codegen_operand(bx, operand);
731763
let val = operand.immediate();
@@ -1032,12 +1064,37 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
10321064
mir::Rvalue::ThreadLocalRef(_) |
10331065
mir::Rvalue::Use(..) => // (*)
10341066
true,
1035-
mir::Rvalue::Repeat(..) |
1036-
mir::Rvalue::Aggregate(..) => {
1067+
mir::Rvalue::Repeat(..) => {
10371068
let ty = rvalue.ty(self.mir, self.cx.tcx());
10381069
let ty = self.monomorphize(ty);
1039-
// For ZST this can be `OperandValueKind::ZeroSized`.
1040-
self.cx.spanned_layout_of(ty, span).is_zst()
1070+
let layout = self.cx.spanned_layout_of(ty, span);
1071+
layout.is_zst()
1072+
}
1073+
mir::Rvalue::Aggregate(ref kind, ref fields) => {
1074+
let ty = rvalue.ty(self.mir, self.cx.tcx());
1075+
let ty = self.monomorphize(ty);
1076+
let layout = self.cx.spanned_layout_of(ty, span);
1077+
match **kind {
1078+
// OperandValue::ZeroSized is easy
1079+
_ if layout.is_zst() => true,
1080+
// 2-Tuple of scalars is an easy scalar pair
1081+
mir::AggregateKind::Tuple => {
1082+
fields.len() == 2
1083+
&& self.cx.is_backend_scalar_pair(layout)
1084+
&& fields.iter().all(|field| {
1085+
let field_ty = field.ty(self.mir, self.cx.tcx());
1086+
let field_ty = self.monomorphize(field_ty);
1087+
let field_layout = self.cx.spanned_layout_of(field_ty, span);
1088+
self.cx.is_backend_immediate(field_layout)
1089+
})
1090+
}
1091+
// If a non-union is transparent, we can pass it along
1092+
mir::AggregateKind::Adt(_, _, _, _, None) => {
1093+
ty.ty_adt_def().is_some_and(|def| def.repr().transparent())
1094+
&& !self.cx.is_backend_ref(layout)
1095+
}
1096+
_ => false,
1097+
}
10411098
}
10421099
}
10431100

compiler/rustc_const_eval/src/interpret/terminator.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
793793
let (idx, _) = receiver.layout.non_1zst_field(self).expect(
794794
"not exactly one non-1-ZST field in a `DispatchFromDyn` type",
795795
);
796-
receiver = self.project_field(&receiver, idx)?;
796+
receiver = self.project_field(&receiver, idx.as_usize())?;
797797
}
798798
}
799799
};

compiler/rustc_target/src/abi/mod.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
279279

280280
/// Finds the one field that is not a 1-ZST.
281281
/// Returns `None` if there are multiple non-1-ZST fields or only 1-ZST-fields.
282-
pub fn non_1zst_field<C>(&self, cx: &C) -> Option<(usize, Self)>
282+
pub fn non_1zst_field<C>(&self, cx: &C) -> Option<(FieldIdx, Self)>
283283
where
284284
Ty: TyAbiInterface<'a, C> + Copy,
285285
{
@@ -293,8 +293,12 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
293293
// More than one non-1-ZST field.
294294
return None;
295295
}
296-
found = Some((field_idx, field));
296+
297+
// Arrays are the only things with more than FieldIdx::MAX fields,
298+
// but only 1-element arrays can ever return non-None here.
299+
found = Some((FieldIdx::from_usize(field_idx), field));
297300
}
301+
298302
found
299303
}
300304
}
+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
//@ compile-flags: -O -C no-prepopulate-passes
2+
3+
#![crate_type = "lib"]
4+
5+
#[repr(transparent)]
6+
pub struct Transparent32(u32);
7+
8+
// CHECK: i32 @make_transparent(i32 noundef %x)
9+
#[no_mangle]
10+
pub fn make_transparent(x: u32) -> Transparent32 {
11+
// CHECK-NOT: alloca
12+
// CHECK: ret i32 %x
13+
let a = Transparent32(x);
14+
a
15+
}
16+
17+
#[repr(transparent)]
18+
pub struct TransparentPair((), (u16, u16), ());
19+
20+
// CHECK: { i16, i16 } @make_transparent_pair(i16 noundef %x.0, i16 noundef %x.1)
21+
#[no_mangle]
22+
pub fn make_transparent_pair(x: (u16, u16)) -> TransparentPair {
23+
// CHECK-NOT: alloca
24+
// CHECK: %[[TEMP0:.+]] = insertvalue { i16, i16 } poison, i16 %x.0, 0
25+
// CHECK: %[[TEMP1:.+]] = insertvalue { i16, i16 } %[[TEMP0]], i16 %x.1, 1
26+
// CHECK: ret { i16, i16 } %[[TEMP1]]
27+
let a = TransparentPair((), x, ());
28+
a
29+
}
30+
31+
// CHECK-LABEL: { i32, i32 } @make_2_tuple(i32 noundef %x)
32+
#[no_mangle]
33+
pub fn make_2_tuple(x: u32) -> (u32, u32) {
34+
// CHECK-NOT: alloca
35+
// CHECK: %[[TEMP0:.+]] = insertvalue { i32, i32 } poison, i32 %x, 0
36+
// CHECK: %[[TEMP1:.+]] = insertvalue { i32, i32 } %[[TEMP0]], i32 %x, 1
37+
// CHECK: ret { i32, i32 } %[[TEMP1]]
38+
let pair = (x, x);
39+
pair
40+
}
41+
42+
// CHECK-LABEL: i8 @make_cell_of_bool(i1 noundef zeroext %b)
43+
#[no_mangle]
44+
pub fn make_cell_of_bool(b: bool) -> std::cell::Cell<bool> {
45+
// CHECK: %[[BYTE:.+]] = zext i1 %b to i8
46+
// CHECK: ret i8 %[[BYTE]]
47+
std::cell::Cell::new(b)
48+
}
49+
50+
// CHECK-LABLE: { i8, i16 } @make_cell_of_bool_and_short(i1 noundef zeroext %b, i16 noundef %s)
51+
#[no_mangle]
52+
pub fn make_cell_of_bool_and_short(b: bool, s: u16) -> std::cell::Cell<(bool, u16)> {
53+
// CHECK-NOT: alloca
54+
// CHECK: %[[BYTE:.+]] = zext i1 %b to i8
55+
// CHECK: %[[TEMP0:.+]] = insertvalue { i8, i16 } poison, i8 %[[BYTE]], 0
56+
// CHECK: %[[TEMP1:.+]] = insertvalue { i8, i16 } %[[TEMP0]], i16 %s, 1
57+
// CHECK: ret { i8, i16 } %[[TEMP1]]
58+
std::cell::Cell::new((b, s))
59+
}
60+
61+
// CHECK-LABEL: { i1, i1 } @make_tuple_of_bools(i1 noundef zeroext %a, i1 noundef zeroext %b)
62+
#[no_mangle]
63+
pub fn make_tuple_of_bools(a: bool, b: bool) -> (bool, bool) {
64+
// CHECK-NOT: alloca
65+
// CHECK: %[[TEMP0:.+]] = insertvalue { i1, i1 } poison, i1 %a, 0
66+
// CHECK: %[[TEMP1:.+]] = insertvalue { i1, i1 } %[[TEMP0]], i1 %b, 1
67+
// CHECK: ret { i1, i1 } %[[TEMP1]]
68+
(a, b)
69+
}

0 commit comments

Comments
 (0)