Skip to content

Commit

Permalink
Auto merge of rust-lang#123886 - scottmcm:more-rvalue-operands, r=<try>
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
bors committed Apr 13, 2024
2 parents 9782770 + dd8ffd0 commit 8dfd58a
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 13 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/vtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub(crate) fn get_ptr_and_method_ref<'tcx>(
.layout()
.non_1zst_field(fx)
.expect("not exactly one non-1-ZST field in a `DispatchFromDyn` type");
arg = arg.value_field(fx, FieldIdx::new(idx));
arg = arg.value_field(fx, idx);
}
}

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 @@ -1019,7 +1019,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let (idx, _) = op.layout.non_1zst_field(bx).expect(
"not exactly one non-1-ZST field in a `DispatchFromDyn` type",
);
op = op.extract_field(bx, idx);
op = op.extract_field(bx, idx.as_usize());
}

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

// Make sure that we've actually unwrapped the rcvr down
Expand Down
74 changes: 67 additions & 7 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout};
use rustc_middle::ty::{self, adjustment::PointerCoercion, Instance, Ty, TyCtxt};
use rustc_session::config::OptLevel;
use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::{self, FIRST_VARIANT};
use rustc_target::abi::{self, FieldIdx, FIRST_VARIANT};

impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
#[instrument(level = "trace", skip(self, bx))]
Expand Down Expand Up @@ -720,12 +720,47 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
OperandRef { val: OperandValue::Immediate(static_), layout }
}
mir::Rvalue::Use(ref operand) => self.codegen_operand(bx, operand),
mir::Rvalue::Repeat(..) | mir::Rvalue::Aggregate(..) => {
mir::Rvalue::Repeat(..) => {
// According to `rvalue_creates_operand`, only ZST
// aggregate rvalues are allowed to be operands.
// repat rvalues are allowed to be operands.
let ty = rvalue.ty(self.mir, self.cx.tcx());
OperandRef::zero_sized(self.cx.layout_of(self.monomorphize(ty)))
}
mir::Rvalue::Aggregate(ref kind, ref fields) => {
let ty = rvalue.ty(self.mir, self.cx.tcx());
let ty = self.monomorphize(ty);
let layout = self.cx.layout_of(self.monomorphize(ty));
match **kind {
_ if layout.is_zst() => OperandRef::zero_sized(layout),
mir::AggregateKind::Tuple => {
debug_assert_eq!(
fields.len(),
2,
"We should only get pairs, but got {rvalue:?}"
);
let a = self.codegen_operand(bx, &fields[FieldIdx::ZERO]);
let b = self.codegen_operand(bx, &fields[FieldIdx::from_u32(1)]);
let val = OperandValue::Pair(a.immediate(), b.immediate());
OperandRef { val, layout }
}
mir::AggregateKind::Adt(..) => {
let (field, _) = layout
.non_1zst_field(self.cx)
.expect("only transparent non-ZST structs should get here");
let val = match self.codegen_operand(bx, &fields[field]).val {
OperandValue::Immediate(a) => {
OperandValue::Immediate(bx.from_immediate(a))
}
OperandValue::Pair(a, b) => {
OperandValue::Pair(bx.from_immediate(a), bx.from_immediate(b))
}
other => bug!("Unexpected operand {other:?}"),
};
OperandRef { val, layout }
}
_ => bug!("Unexpected in codegen_rvalue_operand: {rvalue:?}"),
}
}
mir::Rvalue::ShallowInitBox(ref operand, content_ty) => {
let operand = self.codegen_operand(bx, operand);
let val = operand.immediate();
Expand Down Expand Up @@ -1032,12 +1067,37 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::Rvalue::ThreadLocalRef(_) |
mir::Rvalue::Use(..) => // (*)
true,
mir::Rvalue::Repeat(..) |
mir::Rvalue::Aggregate(..) => {
mir::Rvalue::Repeat(..) => {
let ty = rvalue.ty(self.mir, self.cx.tcx());
let ty = self.monomorphize(ty);
let layout = self.cx.spanned_layout_of(ty, span);
layout.is_zst()
}
mir::Rvalue::Aggregate(ref kind, ref fields) => {
let ty = rvalue.ty(self.mir, self.cx.tcx());
let ty = self.monomorphize(ty);
// For ZST this can be `OperandValueKind::ZeroSized`.
self.cx.spanned_layout_of(ty, span).is_zst()
let layout = self.cx.spanned_layout_of(ty, span);
match **kind {
// OperandValue::ZeroSized is easy
_ if layout.is_zst() => true,
// 2-Tuple of scalars is an easy scalar pair
mir::AggregateKind::Tuple => {
fields.len() == 2
&& self.cx.is_backend_scalar_pair(layout)
&& fields.iter().all(|field| {
let field_ty = field.ty(self.mir, self.cx.tcx());
let field_ty = self.monomorphize(field_ty);
let field_layout = self.cx.spanned_layout_of(field_ty, span);
self.cx.is_backend_immediate(field_layout)
})
}
// If a non-union is transparent, we can pass it along
mir::AggregateKind::Adt(_, _, _, _, None) => {
ty.ty_adt_def().is_some_and(|def| def.repr().transparent())
&& !self.cx.is_backend_ref(layout)
}
_ => false,
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let (idx, _) = receiver.layout.non_1zst_field(self).expect(
"not exactly one non-1-ZST field in a `DispatchFromDyn` type",
);
receiver = self.project_field(&receiver, idx)?;
receiver = self.project_field(&receiver, idx.as_usize())?;
}
}
};
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_target/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {

/// Finds the one field that is not a 1-ZST.
/// Returns `None` if there are multiple non-1-ZST fields or only 1-ZST-fields.
pub fn non_1zst_field<C>(&self, cx: &C) -> Option<(usize, Self)>
pub fn non_1zst_field<C>(&self, cx: &C) -> Option<(FieldIdx, Self)>
where
Ty: TyAbiInterface<'a, C> + Copy,
{
Expand All @@ -293,8 +293,12 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
// More than one non-1-ZST field.
return None;
}
found = Some((field_idx, field));

// Arrays are the only things with more than FieldIdx::MAX fields,
// but only 1-element arrays can ever return non-None here.
found = Some((FieldIdx::from_usize(field_idx), field));
}

found
}
}
69 changes: 69 additions & 0 deletions tests/codegen/transparent-aggregates.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//@ compile-flags: -O -C no-prepopulate-passes

#![crate_type = "lib"]

#[repr(transparent)]
pub struct Transparent32(u32);

// CHECK: i32 @make_transparent(i32 noundef %x)
#[no_mangle]
pub fn make_transparent(x: u32) -> Transparent32 {
// CHECK-NOT: alloca
// CHECK: ret i32 %x
let a = Transparent32(x);
a
}

#[repr(transparent)]
pub struct TransparentPair((), (u16, u16), ());

// CHECK: { i16, i16 } @make_transparent_pair(i16 noundef %x.0, i16 noundef %x.1)
#[no_mangle]
pub fn make_transparent_pair(x: (u16, u16)) -> TransparentPair {
// CHECK-NOT: alloca
// CHECK: %[[TEMP0:.+]] = insertvalue { i16, i16 } poison, i16 %x.0, 0
// CHECK: %[[TEMP1:.+]] = insertvalue { i16, i16 } %[[TEMP0]], i16 %x.1, 1
// CHECK: ret { i16, i16 } %[[TEMP1]]
let a = TransparentPair((), x, ());
a
}

// CHECK-LABEL: { i32, i32 } @make_2_tuple(i32 noundef %x)
#[no_mangle]
pub fn make_2_tuple(x: u32) -> (u32, u32) {
// CHECK-NOT: alloca
// CHECK: %[[TEMP0:.+]] = insertvalue { i32, i32 } poison, i32 %x, 0
// CHECK: %[[TEMP1:.+]] = insertvalue { i32, i32 } %[[TEMP0]], i32 %x, 1
// CHECK: ret { i32, i32 } %[[TEMP1]]
let pair = (x, x);
pair
}

// CHECK-LABEL: i8 @make_cell_of_bool(i1 noundef zeroext %b)
#[no_mangle]
pub fn make_cell_of_bool(b: bool) -> std::cell::Cell<bool> {
// CHECK: %[[BYTE:.+]] = zext i1 %b to i8
// CHECK: ret i8 %[[BYTE]]
std::cell::Cell::new(b)
}

// CHECK-LABLE: { i8, i16 } @make_cell_of_bool_and_short(i1 noundef zeroext %b, i16 noundef %s)
#[no_mangle]
pub fn make_cell_of_bool_and_short(b: bool, s: u16) -> std::cell::Cell<(bool, u16)> {
// CHECK-NOT: alloca
// CHECK: %[[BYTE:.+]] = zext i1 %b to i8
// CHECK: %[[TEMP0:.+]] = insertvalue { i8, i16 } poison, i8 %[[BYTE]], 0
// CHECK: %[[TEMP1:.+]] = insertvalue { i8, i16 } %[[TEMP0]], i16 %s, 1
// CHECK: ret { i8, i16 } %[[TEMP1]]
std::cell::Cell::new((b, s))
}

// CHECK-LABEL: { i1, i1 } @make_tuple_of_bools(i1 noundef zeroext %a, i1 noundef zeroext %b)
#[no_mangle]
pub fn make_tuple_of_bools(a: bool, b: bool) -> (bool, bool) {
// CHECK-NOT: alloca
// CHECK: %[[TEMP0:.+]] = insertvalue { i1, i1 } poison, i1 %a, 0
// CHECK: %[[TEMP1:.+]] = insertvalue { i1, i1 } %[[TEMP0]], i1 %b, 1
// CHECK: ret { i1, i1 } %[[TEMP1]]
(a, b)
}

0 comments on commit 8dfd58a

Please sign in to comment.