From 4b16700be849f8981f58d8d17b6727814d90f458 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Fri, 12 Jun 2026 16:57:23 +0900 Subject: [PATCH] codegen: add OperandValue::Uninit to skip stores for entirely-uninit constants MIR GVN propagates MaybeUninit::uninit() as `const ` in aggregate constructions and codegen emits a memcpy from an `[N x i8] undef` global for each such field, which LLVM materializes as zero-initialization. Instead of special-casing `all_bytes_uninit` at each call site, add a new `OperandValue::Uninit` variant that `eval_mir_constant_to_operand` returns for any all-bytes-uninit constant. `store_with_flags` is a no-op for this variant, so the fix applies uniformly across `Rvalue::Use`, `Rvalue::Repeat`, and `Rvalue::Aggregate` without per-site checks. The variant propagates through field extraction and transmutes, and is handled appropriately at the remaining call sites (function arguments, discriminant reads, debug info, etc.). --- .../rustc_codegen_gcc/src/intrinsic/mod.rs | 2 +- compiler/rustc_codegen_llvm/src/intrinsic.rs | 3 +- compiler/rustc_codegen_ssa/src/base.rs | 2 +- compiler/rustc_codegen_ssa/src/mir/block.rs | 16 ++++++++-- .../rustc_codegen_ssa/src/mir/constant.rs | 6 +++- .../rustc_codegen_ssa/src/mir/debuginfo.rs | 2 +- compiler/rustc_codegen_ssa/src/mir/operand.rs | 23 ++++++++++++-- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 31 +++++-------------- tests/codegen-llvm/uninit-aggregate-field.rs | 30 ++++++++++++++++++ 9 files changed, 81 insertions(+), 34 deletions(-) create mode 100644 tests/codegen-llvm/uninit-aggregate-field.rs diff --git a/compiler/rustc_codegen_gcc/src/intrinsic/mod.rs b/compiler/rustc_codegen_gcc/src/intrinsic/mod.rs index 78a4c7e88c895..5e0a1de970c6f 100644 --- a/compiler/rustc_codegen_gcc/src/intrinsic/mod.rs +++ b/compiler/rustc_codegen_gcc/src/intrinsic/mod.rs @@ -639,7 +639,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc for arg in args { match arg.val { - OperandValue::ZeroSized => {} + OperandValue::ZeroSized | OperandValue::Uninit => {} OperandValue::Immediate(_) => call_args.push(arg.immediate()), OperandValue::Pair(a, b) => { call_args.push(a); diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 7ac35af5820ea..610d68957775a 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -977,6 +977,7 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { for arg in args { match arg.val { OperandValue::ZeroSized => {} + OperandValue::Uninit => {} OperandValue::Immediate(a) => llargs.push(a), OperandValue::Pair(a, b) => { llargs.push(a); @@ -1945,7 +1946,7 @@ fn get_args_from_tuple<'ll, 'tcx>( result } - OperandValue::ZeroSized => vec![], + OperandValue::ZeroSized | OperandValue::Uninit => vec![], } } diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 2a5e421282f4e..3648734d27f45 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -286,7 +286,7 @@ pub(crate) fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( let (base, info) = match bx.load_operand(src).val { OperandValue::Pair(base, info) => unsize_ptr(bx, base, src_ty, dst_ty, Some(info)), OperandValue::Immediate(base) => unsize_ptr(bx, base, src_ty, dst_ty, None), - OperandValue::Ref(..) | OperandValue::ZeroSized => bug!(), + OperandValue::Ref(..) | OperandValue::ZeroSized | OperandValue::Uninit => bug!(), }; OperandValue::Pair(base, info).store(bx, dst); } diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 00cfe1c845d7a..94adfd0d78737 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -21,7 +21,7 @@ use rustc_target::callconv::{ArgAbi, ArgAttributes, CastTarget, FnAbi, PassMode} use tracing::{debug, info}; use super::operand::OperandRef; -use super::operand::OperandValue::{self, Immediate, Pair, Ref, ZeroSized}; +use super::operand::OperandValue::{self, Immediate, Pair, Ref, Uninit, ZeroSized}; use super::place::{PlaceRef, PlaceValue}; use super::{CachedLlbb, FunctionCx, LocalRef}; use crate::base::{self, is_call_from_compiler_builtins_to_upstream_monomorphization}; @@ -599,6 +599,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { place_val.llval } ZeroSized => bug!("ZST return value shouldn't be in PassMode::Cast"), + OperandValue::Uninit => { + bug!("uninit return value shouldn't be in PassMode::Cast") + } }; if self.fn_abi.conv == CanonAbi::Arm(ArmCall::CCmseNonSecureEntry) { @@ -1775,7 +1778,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // Force by-ref if we have to load through a cast pointer. let (mut llval, align, by_ref) = match op.val { - Immediate(_) | Pair(..) => match arg.mode { + Immediate(_) | Pair(..) | Uninit => match arg.mode { PassMode::Indirect { attrs, .. } => { // Indirect argument may have higher alignment requirements than the type's // alignment. This can happen, e.g. when passing types with <4 byte alignment @@ -1795,7 +1798,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { op.store_with_annotation(bx, scratch); (scratch.val.llval, scratch.val.align, true) } - PassMode::Direct(_) => (op.immediate(), arg.layout.align.abi, false), + PassMode::Direct(_) => { + if let Uninit = op.val { + let ibty = bx.cx().immediate_backend_type(arg.layout); + (bx.cx().const_undef(ibty), arg.layout.align.abi, false) + } else { + (op.immediate(), arg.layout.align.abi, false) + } + } PassMode::Ignore | PassMode::Pair(..) => unreachable!("handled above"), }, Ref(op_place_val) => match arg.mode { diff --git a/compiler/rustc_codegen_ssa/src/mir/constant.rs b/compiler/rustc_codegen_ssa/src/mir/constant.rs index d4747c1cbafc9..f5efc69df173f 100644 --- a/compiler/rustc_codegen_ssa/src/mir/constant.rs +++ b/compiler/rustc_codegen_ssa/src/mir/constant.rs @@ -6,7 +6,7 @@ use rustc_middle::{bug, mir, span_bug}; use super::FunctionCx; use crate::errors; -use crate::mir::operand::OperandRef; +use crate::mir::operand::{OperandRef, OperandValue}; use crate::traits::*; impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { @@ -17,6 +17,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ) -> OperandRef<'tcx, Bx::Value> { let val = self.eval_mir_constant(constant); let ty = self.monomorphize(constant.ty()); + if val.all_bytes_uninit(self.cx.tcx()) { + let layout = bx.layout_of(ty); + return OperandRef { val: OperandValue::Uninit, layout, move_annotation: None }; + } OperandRef::from_const(bx, val, ty) } diff --git a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs index 2a55cbedfb3b8..37261a1f8669b 100644 --- a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs @@ -386,7 +386,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx.set_var_name(a, &(name.clone() + ".0")); bx.set_var_name(b, &(name.clone() + ".1")); } - OperandValue::ZeroSized => { + OperandValue::ZeroSized | OperandValue::Uninit => { // These never have a value to talk about } }, diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index c87ea83eacf62..7ef283510ff6f 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs @@ -69,6 +69,12 @@ pub enum OperandValue { /// `is_zst` on its `Layout` returns `true`. Note however that /// these values can still require alignment. ZeroSized, + /// A value for which all bytes are entirely uninitialized. + /// + /// Storing this value is a no-op; it propagates through field extraction. + /// Used to avoid emitting memcpys from uninit globals (which LLVM may + /// otherwise materialize as zero-fills) for `const ` operands. + Uninit, } impl OperandValue { @@ -78,7 +84,7 @@ impl OperandValue { match self { OperandValue::Immediate(llptr) => Some((llptr, None)), OperandValue::Pair(llptr, llextra) => Some((llptr, Some(llextra))), - OperandValue::Ref(_) | OperandValue::ZeroSized => None, + OperandValue::Ref(_) | OperandValue::ZeroSized | OperandValue::Uninit => None, } } @@ -114,6 +120,7 @@ impl OperandValue { OperandValue::Immediate(_) => cx.is_backend_immediate(ty), OperandValue::Pair(_, _) => cx.is_backend_scalar_pair(ty), OperandValue::Ref(_) => cx.is_backend_ref(ty), + OperandValue::Uninit => true, } } } @@ -375,7 +382,9 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { ); } - let val = if field.is_zst() { + let val = if let OperandValue::Uninit = self.val { + OperandValue::Uninit + } else if field.is_zst() { OperandValue::ZeroSized } else if field.size == self.layout.size { assert_eq!(offset.bytes(), 0); @@ -471,6 +480,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> { // Read the tag/niche-encoded discriminant from memory. let tag_op = match self.val { OperandValue::ZeroSized => bug!(), + OperandValue::Uninit => return bx.cx().const_poison(cast_to), OperandValue::Immediate(_) | OperandValue::Pair(_, _) => { self.extract_field(fx, bx, tag_field.as_usize()) } @@ -725,12 +735,15 @@ impl<'a, 'tcx, V: CodegenObject> OperandRefBuilder<'tcx, V> { field: FieldIdx, field_operand: OperandRef<'tcx, V>, ) { - if let OperandValue::ZeroSized = field_operand.val { + if matches!(field_operand.val, OperandValue::ZeroSized | OperandValue::Uninit) { // A ZST never adds any state, so just ignore it. // This special-casing is worth it because of things like // `Result` where `Ok(never)` is legal to write, // but the type shows as FieldShape::Primitive so we can't // actually look at the layout for the field being set. + // + // Likewise, an uninit field does not contribute any value; + // the builder's unset slots will produce `const_undef` in `build()`. return; } @@ -947,6 +960,10 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue { // Avoid generating stores of zero-sized values, because the only way to have a // zero-sized value is through `undef`/`poison`, and the store itself is useless. } + OperandValue::Uninit => { + // Storing an entirely uninit value is a no-op: the destination is left + // uninitialized, which is valid since the value itself is uninit. + } OperandValue::Ref(val) => { assert!(dest.layout.is_sized(), "cannot directly store unsized values"); if val.llextra.is_some() { diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 5dc4617717c11..dc3e4ceed6f84 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -24,12 +24,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ) { match *rvalue { mir::Rvalue::Use(ref operand, with_retag) => { - if let mir::Operand::Constant(const_op) = operand { - let val = self.eval_mir_constant(&const_op); - if val.all_bytes_uninit(self.cx.tcx()) { - return; - } - } let cg_operand = self.codegen_operand(bx, operand); // Crucially, we do *not* use `OperandValue::Ref` for types with // `BackendRepr::Scalar | BackendRepr::ScalarPair`. This ensures we match the MIR @@ -100,6 +94,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { OperandValue::ZeroSized => { bug!("unsized coercion on a ZST rvalue"); } + OperandValue::Uninit => { + bug!("unsized coercion on an uninit rvalue"); + } } } @@ -118,25 +115,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { return; } - // When the element is a const with all bytes uninit, emit a single memset that - // writes undef to the entire destination. - if let mir::Operand::Constant(const_op) = elem { - let val = self.eval_mir_constant(const_op); - if val.all_bytes_uninit(self.cx.tcx()) { - let size = bx.const_usize(dest.layout.size.bytes()); - bx.memset( - dest.val.llval, - bx.const_undef(bx.type_i8()), - size, - dest.val.align, - MemFlags::empty(), - ); - return; - } - } - let cg_elem = self.codegen_operand(bx, elem); + if let OperandValue::Uninit = cg_elem.val { + return; + } + let try_init_all_same = |bx: &mut Bx, v| { let start = dest.val.llval; let size = bx.const_usize(dest.layout.size.bytes()); @@ -292,6 +276,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let cx = bx.cx(); match (operand.val, operand.layout.backend_repr, cast.backend_repr) { _ if cast.is_zst() => OperandValue::ZeroSized, + (OperandValue::Uninit, _, _) => OperandValue::Uninit, (OperandValue::Ref(source_place_val), abi::BackendRepr::Memory { .. }, _) => { assert_eq!(source_place_val.llextra, None); // The existing alignment is part of `source_place_val`, diff --git a/tests/codegen-llvm/uninit-aggregate-field.rs b/tests/codegen-llvm/uninit-aggregate-field.rs new file mode 100644 index 0000000000000..f36ea05f0c8fd --- /dev/null +++ b/tests/codegen-llvm/uninit-aggregate-field.rs @@ -0,0 +1,30 @@ +// Regression test for https://github.com/rust-lang/rust/issues/157743 +// +// At opt-level >= 1, MIR GVN inlines MaybeUninit::uninit() and propagates the +// result as `const ` in aggregate constructions. Without the fix, this +// caused codegen to emit a memcpy from an `[N x i8] undef` global constant for +// the uninit field, which LLVM would materialize as zero-initialization. +// +// The fix skips emitting any IR for entirely-uninit constant aggregate fields. + +//@ compile-flags: -C no-prepopulate-passes -C opt-level=2 + +#![crate_type = "lib"] + +use std::mem::MaybeUninit; + +pub struct Inner { + cap: usize, + data: MaybeUninit<[u64; 2]>, +} + +// CHECK-LABEL: @make_inner +// The non-uninit field must be stored. +// CHECK: store i{{(32|64)}} +// The entirely-uninit `data` field must not cause a memcpy from an undef global. +// CHECK-NOT: call void @llvm.memcpy +// CHECK: ret void +#[no_mangle] +pub fn make_inner(cap: usize) -> Inner { + Inner { cap, data: MaybeUninit::uninit() } +}