Auto merge of #157797 - glandium:uninit, r=oli-obk

codegen: skip stores for entirely-uninit constant aggregate fields



MIR GVN (since #147827) propagates MaybeUninit::uninit() as `const <uninit>` in aggregate constructions. Without this fix, codegen would emit a memcpy from an `[N x i8] undef` global for each such field, which LLVM materializes as zero-initialization.

This mirrors the existing `all_bytes_uninit` skip already present for `Rvalue::Use` (added in #147827) into the `Rvalue::Aggregate` field loop.

Fixes: #157743

Author: bors

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

@@ -629,7 +629,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);

compiler/rustc_codegen_llvm/src/intrinsic.rs

@@ -962,6 +962,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);
@@ -1939,7 +1940,7 @@ fn get_args_from_tuple<'ll, 'tcx>(
             result
         }
 
-        OperandValue::ZeroSized => vec![],
+        OperandValue::ZeroSized | OperandValue::Uninit => vec![],
     }
 }
 

compiler/rustc_codegen_ssa/src/base.rs

@@ -287,7 +287,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);
         }

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};
@@ -611,6 +611,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) {
@@ -1800,7 +1803,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
@@ -1820,7 +1823,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 {

compiler/rustc_codegen_ssa/src/mir/constant.rs

@@ -6,7 +6,7 @@ use rustc_middle::{bug, mir, span_bug};
 
 use super::FunctionCx;
 use crate::diagnostics;
-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)
     }
 

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

@@ -390,7 +390,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
                     }
                 },

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -86,6 +86,12 @@ pub enum OperandValue<V> {
     /// `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 <uninit>` operands.
+    Uninit,
 }
 
 impl<V: CodegenObject> OperandValue<V> {
@@ -95,7 +101,7 @@ impl<V: CodegenObject> OperandValue<V> {
         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,
         }
     }
 
@@ -123,6 +129,7 @@ impl<V: CodegenObject> OperandValue<V> {
     #[must_use]
     pub(crate) fn is_expected_variant_for_type<'tcx>(&self, ty: TyAndLayout<'tcx>) -> bool {
         match (self, ty.backend_repr) {
+            (OperandValue::Uninit, _) => true,
             (OperandValue::ZeroSized, BackendRepr::Memory { .. }) => ty.is_zst(),
             (OperandValue::Ref(_), BackendRepr::Memory { .. }) => !ty.is_zst(),
             (
@@ -397,7 +404,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);
@@ -496,6 +505,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())
             }
@@ -778,12 +788,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;
         }
 
@@ -1019,6 +1032,10 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
                 // 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() {

compiler/rustc_codegen_ssa/src/mir/retag.rs

@@ -298,6 +298,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                                 builder.update_imm(offset, fst);
                                 builder.update_imm(offset + Size::from_bytes(1), snd)
                             }
+                            OperandValue::Uninit => {
+                                unreachable!("load_operand never produces Uninit")
+                            }
                         }
                     }
                 }

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -100,12 +100,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
@@ -176,6 +170,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");
+                    }
                 }
             }
 
@@ -194,25 +191,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());
@@ -372,6 +356,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`,

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 <uninit>` 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() }
+}