Rollup merge of rust-lang#111318 - scottmcm:operand-value-poison, r=compiler-errors

Add a distinct `OperandValue::ZeroSized` variant for ZSTs

These tend to have special handling in a bunch of places anyway, so the variant helps remember that.  And I think it's easier to grok than `Aggregate`s sometimes being `Immediates` (after all, I previously got that wrong and caused rust-lang#109992).  As a minor bonus, it means we don't need to generate poison LLVM values for ZSTs to pass around in `OperandValue::Immediate`s.

Inspired by rust-lang#110021 (comment), so
r? `@compiler-errors`

Author: Dylan-DPC

compiler/rustc_codegen_gcc/src/builder.rs

@@ -758,7 +758,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
         assert_eq!(place.llextra.is_some(), place.layout.is_unsized());
 
         if place.layout.is_zst() {
-            return OperandRef::new_zst(self, place.layout);
+            return OperandRef::zero_sized(place.layout);
         }
 
         fn scalar_load_metadata<'a, 'gcc, 'tcx>(bx: &mut Builder<'a, 'gcc, 'tcx>, load: RValue<'gcc>, scalar: &abi::Scalar) {

compiler/rustc_codegen_gcc/src/type_of.rs

@@ -159,8 +159,7 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
     fn is_gcc_immediate(&self) -> bool {
         match self.abi {
             Abi::Scalar(_) | Abi::Vector { .. } => true,
-            Abi::ScalarPair(..) => false,
-            Abi::Uninhabited | Abi::Aggregate { .. } => self.is_zst(),
+            Abi::ScalarPair(..) | Abi::Uninhabited | Abi::Aggregate { .. } => false,
         }
     }
 

compiler/rustc_codegen_llvm/src/builder.rs

@@ -486,7 +486,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
         assert_eq!(place.llextra.is_some(), place.layout.is_unsized());
 
         if place.layout.is_zst() {
-            return OperandRef::new_zst(self, place.layout);
+            return OperandRef::zero_sized(place.layout);
         }
 
         #[instrument(level = "trace", skip(bx))]

compiler/rustc_codegen_llvm/src/type_of.rs

@@ -198,8 +198,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {
     fn is_llvm_immediate(&self) -> bool {
         match self.abi {
             Abi::Scalar(_) | Abi::Vector { .. } => true,
-            Abi::ScalarPair(..) => false,
-            Abi::Uninhabited | Abi::Aggregate { .. } => self.is_zst(),
+            Abi::ScalarPair(..) | Abi::Uninhabited | Abi::Aggregate { .. } => false,
         }
     }
 

compiler/rustc_codegen_ssa/src/base.rs

@@ -295,7 +295,7 @@ pub 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(..) => bug!(),
+                OperandValue::Ref(..) | OperandValue::ZeroSized => bug!(),
             };
             OperandValue::Pair(base, info).store(bx, dst);
         }

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -1,5 +1,5 @@
 use super::operand::OperandRef;
-use super::operand::OperandValue::{Immediate, Pair, Ref};
+use super::operand::OperandValue::{Immediate, Pair, Ref, ZeroSized};
 use super::place::PlaceRef;
 use super::{CachedLlbb, FunctionCx, LocalRef};
 
@@ -427,6 +427,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         assert_eq!(align, op.layout.align.abi, "return place is unaligned!");
                         llval
                     }
+                    ZeroSized => bug!("ZST return value shouldn't be in PassMode::Cast"),
                 };
                 let ty = bx.cast_backend_type(cast_ty);
                 let addr = bx.pointercast(llslot, bx.type_ptr_to(ty));
@@ -1386,6 +1387,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                     (llval, align, true)
                 }
             }
+            ZeroSized => match arg.mode {
+                PassMode::Indirect { .. } => {
+                    // Though `extern "Rust"` doesn't pass ZSTs, some ABIs pass
+                    // a pointer for `repr(C)` structs even when empty, so get
+                    // one from an `alloca` (which can be left uninitialized).
+                    let scratch = PlaceRef::alloca(bx, arg.layout);
+                    (scratch.llval, scratch.align, true)
+                }
+                _ => bug!("ZST {op:?} wasn't ignored, but was passed with abi {arg:?}"),
+            },
         };
 
         if by_ref && !arg.is_indirect() {

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

@@ -352,6 +352,9 @@ 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 => {
+                        // These never have a value to talk about
+                    }
                 },
                 LocalRef::PendingOperand => {}
             }

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -129,16 +129,13 @@ enum LocalRef<'tcx, V> {
     PendingOperand,
 }
 
-impl<'a, 'tcx, V: CodegenObject> LocalRef<'tcx, V> {
-    fn new_operand<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
-        bx: &mut Bx,
-        layout: TyAndLayout<'tcx>,
-    ) -> LocalRef<'tcx, V> {
+impl<'tcx, V: CodegenObject> LocalRef<'tcx, V> {
+    fn new_operand(layout: TyAndLayout<'tcx>) -> LocalRef<'tcx, V> {
         if layout.is_zst() {
             // Zero-size temporaries aren't always initialized, which
             // doesn't matter because they don't contain data, but
             // we need something in the operand.
-            LocalRef::Operand(OperandRef::new_zst(bx, layout))
+            LocalRef::Operand(OperandRef::zero_sized(layout))
         } else {
             LocalRef::PendingOperand
         }
@@ -249,7 +246,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                 }
             } else {
                 debug!("alloc: {:?} -> operand", local);
-                LocalRef::new_operand(&mut start_bx, layout)
+                LocalRef::new_operand(layout)
             }
         };
 
@@ -355,7 +352,7 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                 let local = |op| LocalRef::Operand(op);
                 match arg.mode {
                     PassMode::Ignore => {
-                        return local(OperandRef::new_zst(bx, arg.layout));
+                        return local(OperandRef::zero_sized(arg.layout));
                     }
                     PassMode::Direct(_) => {
                         let llarg = bx.get_param(llarg_idx);

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -45,6 +45,14 @@ pub enum OperandValue<V> {
     /// as returned by [`LayoutTypeMethods::scalar_pair_element_backend_type`]
     /// with `immediate: true`.
     Pair(V, V),
+    /// A value taking no bytes, and which therefore needs no LLVM value at all.
+    ///
+    /// If you ever need a `V` to pass to something, get a fresh poison value
+    /// from [`ConstMethods::const_poison`].
+    ///
+    /// An `OperandValue` *must* be this variant for any type for which
+    /// `is_zst` on its `Layout` returns `true`.
+    ZeroSized,
 }
 
 /// An `OperandRef` is an "SSA" reference to a Rust value, along with
@@ -71,15 +79,9 @@ impl<V: CodegenObject> fmt::Debug for OperandRef<'_, V> {
 }
 
 impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
-    pub fn new_zst<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
-        bx: &mut Bx,
-        layout: TyAndLayout<'tcx>,
-    ) -> OperandRef<'tcx, V> {
+    pub fn zero_sized(layout: TyAndLayout<'tcx>) -> OperandRef<'tcx, V> {
         assert!(layout.is_zst());
-        OperandRef {
-            val: OperandValue::Immediate(bx.const_poison(bx.immediate_backend_type(layout))),
-            layout,
-        }
+        OperandRef { val: OperandValue::ZeroSized, layout }
     }
 
     pub fn from_const<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
@@ -97,7 +99,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
                 let llval = bx.scalar_to_backend(x, scalar, bx.immediate_backend_type(layout));
                 OperandValue::Immediate(llval)
             }
-            ConstValue::ZeroSized => return OperandRef::new_zst(bx, layout),
+            ConstValue::ZeroSized => return OperandRef::zero_sized(layout),
             ConstValue::Slice { data, start, end } => {
                 let Abi::ScalarPair(a_scalar, _) = layout.abi else {
                     bug!("from_const: invalid ScalarPair layout: {:#?}", layout);
@@ -178,7 +180,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
                 );
                 OperandRef { val: OperandValue::Pair(a_val, b_val), layout }
             }
-            _ if layout.is_zst() => OperandRef::new_zst(bx, layout),
+            _ if layout.is_zst() => OperandRef::zero_sized(layout),
             _ => {
                 // Neither a scalar nor scalar pair. Load from a place
                 let init = bx.const_data_from_alloc(alloc);
@@ -216,6 +218,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
             OperandValue::Immediate(llptr) => (llptr, None),
             OperandValue::Pair(llptr, llextra) => (llptr, Some(llextra)),
             OperandValue::Ref(..) => bug!("Deref of by-Ref operand {:?}", self),
+            OperandValue::ZeroSized => bug!("Deref of ZST operand {:?}", self),
         };
         let layout = cx.layout_of(projected_ty);
         PlaceRef { llval: llptr, llextra, layout, align: layout.align.abi }
@@ -273,9 +276,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
 
         let mut val = match (self.val, self.layout.abi) {
             // If the field is ZST, it has no data.
-            _ if field.is_zst() => {
-                return OperandRef::new_zst(bx, field);
-            }
+            _ if field.is_zst() => OperandValue::ZeroSized,
 
             // Newtype of a scalar, scalar pair or vector.
             (OperandValue::Immediate(_) | OperandValue::Pair(..), _)
@@ -306,6 +307,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
         };
 
         match (&mut val, field.abi) {
+            (OperandValue::ZeroSized, _) => {}
             (
                 OperandValue::Immediate(llval),
                 Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. },
@@ -359,16 +361,18 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
 impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
     /// Returns an `OperandValue` that's generally UB to use in any way.
     ///
-    /// Depending on the `layout`, returns an `Immediate` or `Pair` containing
-    /// poison value(s), or a `Ref` containing a poison pointer.
+    /// Depending on the `layout`, returns `ZeroSized` for ZSTs, an `Immediate` or
+    /// `Pair` containing poison value(s), or a `Ref` containing a poison pointer.
     ///
     /// Supports sized types only.
     pub fn poison<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
         bx: &mut Bx,
         layout: TyAndLayout<'tcx>,
     ) -> OperandValue<V> {
         assert!(layout.is_sized());
-        if bx.cx().is_backend_immediate(layout) {
+        if layout.is_zst() {
+            OperandValue::ZeroSized
+        } else if bx.cx().is_backend_immediate(layout) {
             let ibty = bx.cx().immediate_backend_type(layout);
             OperandValue::Immediate(bx.const_poison(ibty))
         } else if bx.cx().is_backend_scalar_pair(layout) {
@@ -421,12 +425,11 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
         flags: MemFlags,
     ) {
         debug!("OperandRef::store: operand={:?}, dest={:?}", self, dest);
-        // Avoid generating stores of zero-sized values, because the only way to have a zero-sized
-        // value is through `undef`, and store itself is useless.
-        if dest.layout.is_zst() {
-            return;
-        }
         match self {
+            OperandValue::ZeroSized => {
+                // 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::Ref(r, None, source_align) => {
                 if flags.contains(MemFlags::NONTEMPORAL) {
                     // HACK(nox): This is inefficient but there is no nontemporal memcpy.
@@ -527,7 +530,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                             // checks in `codegen_consume` and `extract_field`.
                             let elem = o.layout.field(bx.cx(), 0);
                             if elem.is_zst() {
-                                o = OperandRef::new_zst(bx, elem);
+                                o = OperandRef::zero_sized(elem);
                             } else {
                                 return None;
                             }
@@ -561,7 +564,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 
         // ZSTs don't require any actual memory access.
         if layout.is_zst() {
-            return OperandRef::new_zst(bx, layout);
+            return OperandRef::zero_sized(layout);
         }
 
         if let Some(o) = self.maybe_codegen_consume_direct(bx, place_ref) {