From 0586c63e070981af7df53e2f778d3e50293d8103 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 9 Jul 2025 23:59:00 -0700 Subject: [PATCH 1/2] Allow `Rvalue::Repeat` to return true in `rvalue_creates_operand` too The conversation in 143502 made be realize how easy this is to handle, since the only possibilty is ZSTs -- everything else ends up with the destination being `LocalKind::Memory` and thus doesn't call `codegen_rvalue_operand` at all. This gets us perilously close to a world where `rvalue_creates_operand` only ever returns true. I'll try out such a world next :) --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 16 ++++++-- tests/codegen/repeat-operand.rs | 39 ++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 tests/codegen/repeat-operand.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 2587e89417a6a..e872f8434e56e 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -630,7 +630,17 @@ 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(..) => bug!("{rvalue:?} in codegen_rvalue_operand"), + mir::Rvalue::Repeat(ref elem, len_const) => { + // All arrays have `BackendRepr::Memory`, so only the ZST cases + // end up here. Anything else forces the destination local to be + // `Memory`, and thus ends up handled in `codegen_rvalue` instead. + let operand = self.codegen_operand(bx, elem); + let array_ty = Ty::new_array_with_const_len(bx.tcx(), operand.layout.ty, len_const); + let array_ty = self.monomorphize(array_ty); + let array_layout = bx.layout_of(array_ty); + assert!(array_layout.is_zst()); + OperandRef { val: OperandValue::ZeroSized, layout: array_layout } + } mir::Rvalue::Aggregate(ref kind, ref fields) => { let (variant_index, active_field_index) = match **kind { mir::AggregateKind::Adt(_, variant_index, _, _, active_field_index) => { @@ -1000,12 +1010,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { mir::Rvalue::NullaryOp(..) | mir::Rvalue::ThreadLocalRef(_) | mir::Rvalue::Use(..) | + mir::Rvalue::Repeat(..) | // (*) mir::Rvalue::Aggregate(..) | // (*) mir::Rvalue::WrapUnsafeBinder(..) => // (*) true, - // Arrays are always aggregates, so it's not worth checking anything here. - // (If it's really `[(); N]` or `[T; 0]` and we use the place path, fine.) - mir::Rvalue::Repeat(..) => false, } // (*) this is only true if the type is suitable diff --git a/tests/codegen/repeat-operand.rs b/tests/codegen/repeat-operand.rs new file mode 100644 index 0000000000000..6464b35ddcad3 --- /dev/null +++ b/tests/codegen/repeat-operand.rs @@ -0,0 +1,39 @@ +//@ compile-flags: -Copt-level=1 -Cno-prepopulate-passes + +// This test is here to hit the `Rvalue::Repeat` case in `codegen_rvalue_operand`. +// It only applies when the resulting array is a ZST, so the test is written in +// such a way as to keep MIR optimizations from seeing that fact and removing +// the local and statement altogether. (At the time of writing, no other codegen +// test hit that code path, nor did a stage 2 build of the compiler.) + +#![crate_type = "lib"] + +#[repr(transparent)] +pub struct Wrapper([T; N]); + +// CHECK-LABEL: define {{.+}}do_repeat{{.+}}() +// CHECK-NEXT: start: +// CHECK-NOT: alloca +// CHECK-NEXT: ret void +// CHECK-LABEL: define {{.+}}do_repeat{{.+}}(i32 noundef %x) +// CHECK-NEXT: start: +// CHECK-NOT: alloca +// CHECK-NEXT: ret void +#[inline(never)] +pub fn do_repeat(x: T) -> Wrapper { + Wrapper([x; N]) +} + +// CHECK-LABEL: @trigger_repeat_zero_len +#[no_mangle] +pub fn trigger_repeat_zero_len() -> Wrapper { + // CHECK: call void {{.+}}do_repeat{{.+}}(i32 noundef 4) + do_repeat(4) +} + +// CHECK-LABEL: @trigger_repeat_zst +#[no_mangle] +pub fn trigger_repeat_zst() -> Wrapper<(), 8> { + // CHECK: call void {{.+}}do_repeat{{.+}}() + do_repeat(()) +} From 4b8f869c638e6d5090420ff46bd14e7b7d909690 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 19 Jul 2025 20:49:52 -0700 Subject: [PATCH 2/2] Split repeat-operand codegen test Looks like the output it's looking for can be in different orders, so separate tests will fix that. --- ...-operand.rs => repeat-operand-zero-len.rs} | 11 -------- tests/codegen/repeat-operand-zst-elem.rs | 28 +++++++++++++++++++ 2 files changed, 28 insertions(+), 11 deletions(-) rename tests/codegen/{repeat-operand.rs => repeat-operand-zero-len.rs} (77%) create mode 100644 tests/codegen/repeat-operand-zst-elem.rs diff --git a/tests/codegen/repeat-operand.rs b/tests/codegen/repeat-operand-zero-len.rs similarity index 77% rename from tests/codegen/repeat-operand.rs rename to tests/codegen/repeat-operand-zero-len.rs index 6464b35ddcad3..b4cec42a07c5c 100644 --- a/tests/codegen/repeat-operand.rs +++ b/tests/codegen/repeat-operand-zero-len.rs @@ -11,10 +11,6 @@ #[repr(transparent)] pub struct Wrapper([T; N]); -// CHECK-LABEL: define {{.+}}do_repeat{{.+}}() -// CHECK-NEXT: start: -// CHECK-NOT: alloca -// CHECK-NEXT: ret void // CHECK-LABEL: define {{.+}}do_repeat{{.+}}(i32 noundef %x) // CHECK-NEXT: start: // CHECK-NOT: alloca @@ -30,10 +26,3 @@ pub fn trigger_repeat_zero_len() -> Wrapper { // CHECK: call void {{.+}}do_repeat{{.+}}(i32 noundef 4) do_repeat(4) } - -// CHECK-LABEL: @trigger_repeat_zst -#[no_mangle] -pub fn trigger_repeat_zst() -> Wrapper<(), 8> { - // CHECK: call void {{.+}}do_repeat{{.+}}() - do_repeat(()) -} diff --git a/tests/codegen/repeat-operand-zst-elem.rs b/tests/codegen/repeat-operand-zst-elem.rs new file mode 100644 index 0000000000000..c3637759afa34 --- /dev/null +++ b/tests/codegen/repeat-operand-zst-elem.rs @@ -0,0 +1,28 @@ +//@ compile-flags: -Copt-level=1 -Cno-prepopulate-passes + +// This test is here to hit the `Rvalue::Repeat` case in `codegen_rvalue_operand`. +// It only applies when the resulting array is a ZST, so the test is written in +// such a way as to keep MIR optimizations from seeing that fact and removing +// the local and statement altogether. (At the time of writing, no other codegen +// test hit that code path, nor did a stage 2 build of the compiler.) + +#![crate_type = "lib"] + +#[repr(transparent)] +pub struct Wrapper([T; N]); + +// CHECK-LABEL: define {{.+}}do_repeat{{.+}}() +// CHECK-NEXT: start: +// CHECK-NOT: alloca +// CHECK-NEXT: ret void +#[inline(never)] +pub fn do_repeat(x: T) -> Wrapper { + Wrapper([x; N]) +} + +// CHECK-LABEL: @trigger_repeat_zst_elem +#[no_mangle] +pub fn trigger_repeat_zst_elem() -> Wrapper<(), 8> { + // CHECK: call void {{.+}}do_repeat{{.+}}() + do_repeat(()) +}