Auto merge of #73453 - erikdesjardins:tuplayout, r=eddyb

Ignore ZST offsets when deciding whether to use Scalar/ScalarPair layout

This is important because Scalar/ScalarPair layout previously would not be used if any ZST had nonzero offset.
For example, before this change, only `((), u128)` would be laid out like `u128`, not `(u128, ())`.

Fixes #63244

2 files changed, 74 insertions, 74 deletions

diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs
index 7c3b80c9c8f..91609b22615 100644
--- a/compiler/rustc_codegen_ssa/src/mir/place.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/place.rs
@@ -93,15 +93,33 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
         let effective_field_align = self.align.restrict_for_offset(offset);
 
         let mut simple = || {
-            // Unions and newtypes only use an offset of 0.
-            let llval = if offset.bytes() == 0 {
-                self.llval
-            } else if let Abi::ScalarPair(ref a, ref b) = self.layout.abi {
-                // Offsets have to match either first or second field.
-                assert_eq!(offset, a.value.size(bx.cx()).align_to(b.value.align(bx.cx()).abi));
-                bx.struct_gep(self.llval, 1)
-            } else {
-                bx.struct_gep(self.llval, bx.cx().backend_field_index(self.layout, ix))
+            let llval = match self.layout.abi {
+                _ if offset.bytes() == 0 => {
+                    // Unions and newtypes only use an offset of 0.
+                    // Also handles the first field of Scalar, ScalarPair, and Vector layouts.
+                    self.llval
+                }
+                Abi::ScalarPair(ref a, ref b)
+                    if offset == a.value.size(bx.cx()).align_to(b.value.align(bx.cx()).abi) =>
+                {
+                    // Offset matches second field.
+                    bx.struct_gep(self.llval, 1)
+                }
+                Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } if field.is_zst() => {
+                    // ZST fields are not included in Scalar, ScalarPair, and Vector layouts, so manually offset the pointer.
+                    let byte_ptr = bx.pointercast(self.llval, bx.cx().type_i8p());
+                    bx.gep(byte_ptr, &[bx.const_usize(offset.bytes())])
+                }
+                Abi::Scalar(_) | Abi::ScalarPair(..) => {
+                    // All fields of Scalar and ScalarPair layouts must have been handled by this point.
+                    // Vector layouts have additional fields for each element of the vector, so don't panic in that case.
+                    bug!(
+                        "offset of non-ZST field `{:?}` does not match layout `{:#?}`",
+                        field,
+                        self.layout
+                    );
+                }
+                _ => bx.struct_gep(self.llval, bx.cx().backend_field_index(self.layout, ix)),
             };
             PlaceRef {
                 // HACK(eddyb): have to bitcast pointers until LLVM removes pointee types.
diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs
index cb79b089d94..0fda1473f64 100644
--- a/compiler/rustc_middle/src/ty/layout.rs
+++ b/compiler/rustc_middle/src/ty/layout.rs
@@ -390,78 +390,60 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
 
         // Unpack newtype ABIs and find scalar pairs.
         if sized && size.bytes() > 0 {
-            // All other fields must be ZSTs, and we need them to all start at 0.
-            let mut zst_offsets = offsets.iter().enumerate().filter(|&(i, _)| fields[i].is_zst());
-            if zst_offsets.all(|(_, o)| o.bytes() == 0) {
-                let mut non_zst_fields = fields.iter().enumerate().filter(|&(_, f)| !f.is_zst());
-
-                match (non_zst_fields.next(), non_zst_fields.next(), non_zst_fields.next()) {
-                    // We have exactly one non-ZST field.
-                    (Some((i, field)), None, None) => {
-                        // Field fills the struct and it has a scalar or scalar pair ABI.
-                        if offsets[i].bytes() == 0
-                            && align.abi == field.align.abi
-                            && size == field.size
-                        {
-                            match field.abi {
-                                // For plain scalars, or vectors of them, we can't unpack
-                                // newtypes for `#[repr(C)]`, as that affects C ABIs.
-                                Abi::Scalar(_) | Abi::Vector { .. } if optimize => {
-                                    abi = field.abi.clone();
-                                }
-                                // But scalar pairs are Rust-specific and get
-                                // treated as aggregates by C ABIs anyway.
-                                Abi::ScalarPair(..) => {
-                                    abi = field.abi.clone();
-                                }
-                                _ => {}
+            // All other fields must be ZSTs.
+            let mut non_zst_fields = fields.iter().enumerate().filter(|&(_, f)| !f.is_zst());
+
+            match (non_zst_fields.next(), non_zst_fields.next(), non_zst_fields.next()) {
+                // We have exactly one non-ZST field.
+                (Some((i, field)), None, None) => {
+                    // Field fills the struct and it has a scalar or scalar pair ABI.
+                    if offsets[i].bytes() == 0 && align.abi == field.align.abi && size == field.size
+                    {
+                        match field.abi {
+                            // For plain scalars, or vectors of them, we can't unpack
+                            // newtypes for `#[repr(C)]`, as that affects C ABIs.
+                            Abi::Scalar(_) | Abi::Vector { .. } if optimize => {
+                                abi = field.abi.clone();
                             }
+                            // But scalar pairs are Rust-specific and get
+                            // treated as aggregates by C ABIs anyway.
+                            Abi::ScalarPair(..) => {
+                                abi = field.abi.clone();
+                            }
+                            _ => {}
                         }
                     }
+                }
 
-                    // Two non-ZST fields, and they're both scalars.
-                    (
-                        Some((
-                            i,
-                            &TyAndLayout {
-                                layout: &Layout { abi: Abi::Scalar(ref a), .. }, ..
-                            },
-                        )),
-                        Some((
-                            j,
-                            &TyAndLayout {
-                                layout: &Layout { abi: Abi::Scalar(ref b), .. }, ..
-                            },
-                        )),
-                        None,
-                    ) => {
-                        // Order by the memory placement, not source order.
-                        let ((i, a), (j, b)) = if offsets[i] < offsets[j] {
-                            ((i, a), (j, b))
-                        } else {
-                            ((j, b), (i, a))
-                        };
-                        let pair = self.scalar_pair(a.clone(), b.clone());
-                        let pair_offsets = match pair.fields {
-                            FieldsShape::Arbitrary { ref offsets, ref memory_index } => {
-                                assert_eq!(memory_index, &[0, 1]);
-                                offsets
-                            }
-                            _ => bug!(),
-                        };
-                        if offsets[i] == pair_offsets[0]
-                            && offsets[j] == pair_offsets[1]
-                            && align == pair.align
-                            && size == pair.size
-                        {
-                            // We can use `ScalarPair` only when it matches our
-                            // already computed layout (including `#[repr(C)]`).
-                            abi = pair.abi;
+                // Two non-ZST fields, and they're both scalars.
+                (
+                    Some((i, &TyAndLayout { layout: &Layout { abi: Abi::Scalar(ref a), .. }, .. })),
+                    Some((j, &TyAndLayout { layout: &Layout { abi: Abi::Scalar(ref b), .. }, .. })),
+                    None,
+                ) => {
+                    // Order by the memory placement, not source order.
+                    let ((i, a), (j, b)) =
+                        if offsets[i] < offsets[j] { ((i, a), (j, b)) } else { ((j, b), (i, a)) };
+                    let pair = self.scalar_pair(a.clone(), b.clone());
+                    let pair_offsets = match pair.fields {
+                        FieldsShape::Arbitrary { ref offsets, ref memory_index } => {
+                            assert_eq!(memory_index, &[0, 1]);
+                            offsets
                         }
+                        _ => bug!(),
+                    };
+                    if offsets[i] == pair_offsets[0]
+                        && offsets[j] == pair_offsets[1]
+                        && align == pair.align
+                        && size == pair.size
+                    {
+                        // We can use `ScalarPair` only when it matches our
+                        // already computed layout (including `#[repr(C)]`).
+                        abi = pair.abi;
                     }
-
-                    _ => {}
                 }
+
+                _ => {}
             }
         }