i686-windows: make requested alignment > 4 special case apply transitively

Author: erikdesjardins

compiler/rustc_abi/src/layout.rs

@@ -40,7 +40,7 @@ pub trait LayoutCalculator {
             largest_niche,
             align,
             size,
-            repr_align: None,
+            max_repr_align: None,
             unadjusted_abi_align: align.abi,
         }
     }
@@ -124,7 +124,7 @@ pub trait LayoutCalculator {
             largest_niche: None,
             align: dl.i8_align,
             size: Size::ZERO,
-            repr_align: None,
+            max_repr_align: None,
             unadjusted_abi_align: dl.i8_align.abi,
         }
     }
@@ -293,6 +293,7 @@ pub trait LayoutCalculator {
             }
 
             let mut align = dl.aggregate_align;
+            let mut max_repr_align = repr.align;
             let mut unadjusted_abi_align = align.abi;
 
             let mut variant_layouts = variants
@@ -302,6 +303,7 @@ pub trait LayoutCalculator {
                     st.variants = Variants::Single { index: j };
 
                     align = align.max(st.align);
+                    max_repr_align = max_repr_align.max(st.max_repr_align);
                     unadjusted_abi_align = unadjusted_abi_align.max(st.unadjusted_abi_align);
 
                     Some(st)
@@ -429,7 +431,7 @@ pub trait LayoutCalculator {
                 largest_niche,
                 size,
                 align,
-                repr_align: repr.align,
+                max_repr_align,
                 unadjusted_abi_align,
             };
 
@@ -465,6 +467,7 @@ pub trait LayoutCalculator {
         let (min_ity, signed) = discr_range_of_repr(min, max); //Integer::repr_discr(tcx, ty, &repr, min, max);
 
         let mut align = dl.aggregate_align;
+        let mut max_repr_align = repr.align;
         let mut unadjusted_abi_align = align.abi;
 
         let mut size = Size::ZERO;
@@ -509,6 +512,7 @@ pub trait LayoutCalculator {
                 }
                 size = cmp::max(size, st.size);
                 align = align.max(st.align);
+                max_repr_align = max_repr_align.max(st.max_repr_align);
                 unadjusted_abi_align = unadjusted_abi_align.max(st.unadjusted_abi_align);
                 Some(st)
             })
@@ -703,7 +707,7 @@ pub trait LayoutCalculator {
             abi,
             align,
             size,
-            repr_align: repr.align,
+            max_repr_align,
             unadjusted_abi_align,
         };
 
@@ -744,6 +748,7 @@ pub trait LayoutCalculator {
         let dl = self.current_data_layout();
         let dl = dl.borrow();
         let mut align = if repr.pack.is_some() { dl.i8_align } else { dl.aggregate_align };
+        let mut max_repr_align = repr.align;
 
         // If all the non-ZST fields have the same ABI and union ABI optimizations aren't
         // disabled, we can use that common ABI for the union as a whole.
@@ -761,6 +766,7 @@ pub trait LayoutCalculator {
             assert!(field.0.is_sized());
 
             align = align.max(field.align());
+            max_repr_align = max_repr_align.max(field.max_repr_align());
             size = cmp::max(size, field.size());
 
             if field.0.is_zst() {
@@ -827,7 +833,7 @@ pub trait LayoutCalculator {
             largest_niche: None,
             align,
             size: size.align_to(align.abi),
-            repr_align: repr.align,
+            max_repr_align,
             unadjusted_abi_align,
         })
     }
@@ -849,6 +855,7 @@ fn univariant(
 ) -> Option<LayoutS> {
     let pack = repr.pack;
     let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align };
+    let mut max_repr_align = repr.align;
     let mut inverse_memory_index: IndexVec<u32, FieldIdx> = fields.indices().collect();
     let optimize = !repr.inhibit_struct_field_reordering_opt();
     if optimize && fields.len() > 1 {
@@ -1017,6 +1024,7 @@ fn univariant(
         };
         offset = offset.align_to(field_align.abi);
         align = align.max(field_align);
+        max_repr_align = max_repr_align.max(field.max_repr_align());
 
         debug!("univariant offset: {:?} field: {:#?}", offset, field);
         offsets[i] = offset;
@@ -1133,28 +1141,16 @@ fn univariant(
         abi = Abi::Uninhabited;
     }
 
-    let (repr_align, unadjusted_abi_align) = if repr.transparent() {
+    let unadjusted_abi_align = if repr.transparent() {
         match layout_of_single_non_zst_field {
-            Some(l) => (l.repr_align(), l.unadjusted_abi_align()),
+            Some(l) => l.unadjusted_abi_align(),
             None => {
                 // `repr(transparent)` with all ZST fields.
-                //
-                // Using `None` for `repr_align` here is technically incorrect, since one of
-                // the ZSTs could have `repr(align(1))`. It's an interesting question, if you have
-                // `#{repr(transparent)] struct Foo((), ZstWithReprAlign1)`, which of those ZSTs'
-                // ABIs is forwarded by `repr(transparent)`? The answer to that question determines
-                // whether we should use `None` or `Some(align 1)` here. Thanksfully, two things
-                // together mean this doesn't matter:
-                // - You're not allowed to have a `repr(transparent)` struct that contains
-                //   `repr(align)` > 1 ZSTs. See error E0691.
-                // - MSVC never treats requested align 1 differently from natural align 1.
-                //   (And the `repr_align` field is only used on i686-windows, see `LayoutS` docs.)
-                // So just use `None` for now.
-                (None, align.abi)
+                align.abi
             }
         }
     } else {
-        (repr.align, unadjusted_abi_align)
+        unadjusted_abi_align
     };
 
     Some(LayoutS {
@@ -1164,7 +1160,7 @@ fn univariant(
         largest_niche,
         align,
         size,
-        repr_align,
+        max_repr_align,
         unadjusted_abi_align,
     })
 }

compiler/rustc_abi/src/lib.rs

@@ -1532,10 +1532,10 @@ pub struct LayoutS {
     pub align: AbiAndPrefAlign,
     pub size: Size,
 
-    /// The alignment explicitly requested with `repr(align)`.
+    /// The largest alignment explicitly requested with `repr(align)` on this type or any field.
     /// Only used on i686-windows, where the argument passing ABI is different when alignment is
     /// requested, even if the requested alignment is equal to the natural alignment.
-    pub repr_align: Option<Align>,
+    pub max_repr_align: Option<Align>,
 
     /// The alignment the type would have, ignoring any `repr(align)` but including `repr(packed)`.
     /// Only used on aarch64-linux, where the argument passing ABI ignores the requested alignment
@@ -1555,7 +1555,7 @@ impl LayoutS {
             largest_niche,
             size,
             align,
-            repr_align: None,
+            max_repr_align: None,
             unadjusted_abi_align: align.abi,
         }
     }
@@ -1573,7 +1573,7 @@ impl fmt::Debug for LayoutS {
             fields,
             largest_niche,
             variants,
-            repr_align,
+            max_repr_align,
             unadjusted_abi_align,
         } = self;
         f.debug_struct("Layout")
@@ -1583,7 +1583,7 @@ impl fmt::Debug for LayoutS {
             .field("fields", fields)
             .field("largest_niche", largest_niche)
             .field("variants", variants)
-            .field("repr_align", repr_align)
+            .field("max_repr_align", max_repr_align)
             .field("unadjusted_abi_align", unadjusted_abi_align)
             .finish()
     }
@@ -1625,8 +1625,8 @@ impl<'a> Layout<'a> {
         self.0.0.size
     }
 
-    pub fn repr_align(self) -> Option<Align> {
-        self.0.0.repr_align
+    pub fn max_repr_align(self) -> Option<Align> {
+        self.0.0.max_repr_align
     }
 
     pub fn unadjusted_abi_align(self) -> Align {

compiler/rustc_middle/src/ty/layout.rs

@@ -755,7 +755,7 @@ where
                     largest_niche: None,
                     align: tcx.data_layout.i8_align,
                     size: Size::ZERO,
-                    repr_align: None,
+                    max_repr_align: None,
                     unadjusted_abi_align: tcx.data_layout.i8_align.abi,
                 })
             }

compiler/rustc_target/src/abi/call/x86.rs

@@ -63,17 +63,17 @@ where
 
         if t.is_like_msvc
             && arg.layout.is_adt()
-            && let Some(repr_align) = arg.layout.repr_align
-            && repr_align > align_4
+            && let Some(max_repr_align) = arg.layout.max_repr_align
+            && max_repr_align > align_4
         {
             // MSVC has special rules for overaligned arguments: https://reviews.llvm.org/D72114.
             // Summarized here:
             // - Arguments with _requested_ alignment > 4 are passed indirectly.
             // - For backwards compatibility, arguments with natural alignment > 4 are still passed
             //   on stack (via `byval`). For example, this includes `double`, `int64_t`,
             //   and structs containing them, provided they lack an explicit alignment attribute.
-            assert!(arg.layout.align.abi >= repr_align,
-                "abi alignment {:?} less than requested alignment {repr_align:?}",
+            assert!(arg.layout.align.abi >= max_repr_align,
+                "abi alignment {:?} less than requested alignment {max_repr_align:?}",
                 arg.layout.align.abi,
             );
             arg.make_indirect();

compiler/rustc_ty_utils/src/layout.rs

@@ -258,7 +258,7 @@ fn layout_of_uncached<'tcx>(
                 largest_niche,
                 align: element.align,
                 size,
-                repr_align: None,
+                max_repr_align: None,
                 unadjusted_abi_align: element.align.abi,
             })
         }
@@ -271,7 +271,7 @@ fn layout_of_uncached<'tcx>(
                 largest_niche: None,
                 align: element.align,
                 size: Size::ZERO,
-                repr_align: None,
+                max_repr_align: None,
                 unadjusted_abi_align: element.align.abi,
             })
         }
@@ -282,7 +282,7 @@ fn layout_of_uncached<'tcx>(
             largest_niche: None,
             align: dl.i8_align,
             size: Size::ZERO,
-            repr_align: None,
+            max_repr_align: None,
             unadjusted_abi_align: dl.i8_align.abi,
         }),
 
@@ -437,7 +437,7 @@ fn layout_of_uncached<'tcx>(
                 largest_niche: e_ly.largest_niche,
                 size,
                 align,
-                repr_align: None,
+                max_repr_align: None,
                 unadjusted_abi_align: align.abi,
             })
         }
@@ -887,7 +887,7 @@ fn generator_layout<'tcx>(
         largest_niche: prefix.largest_niche,
         size,
         align,
-        repr_align: None,
+        max_repr_align: None,
         unadjusted_abi_align: align.abi,
     });
     debug!("generator layout ({:?}): {:#?}", ty, layout);

tests/codegen/align-byval.rs

@@ -80,7 +80,7 @@ pub struct LowerFA8 {
     c: i64
 }
 
-// On i686-windows, this is passed on stack, because the wrapper struct does not have
+// On i686-windows, this is passed by reference, because it contains a field with
 // requested/forced alignment.
 #[repr(C)]
 pub struct WrappedFA8 {
@@ -301,7 +301,9 @@ extern "C" {
 
     // i686-linux: declare void @wrapped_fa8({{.*}}byval(%WrappedFA8) align 4{{.*}})
 
-    // i686-windows: declare void @wrapped_fa8({{.*}}byval(%WrappedFA8) align 4{{.*}})
+    // i686-windows: declare void @wrapped_fa8(
+    // i686-windows-NOT: byval
+    // i686-windows-SAME: align 8{{.*}})
     fn wrapped_fa8(x: WrappedFA8);
 
     // m68k: declare void @transparent_fa8({{.*}}byval(%TransparentFA8) align 8{{.*}})