From 7e933b4e26947b04da70589110dacbdb2461e27d Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Sun, 11 Jun 2023 17:05:26 -0400 Subject: [PATCH] repr(align) <= 4 should still be byval --- compiler/rustc_abi/src/layout.rs | 18 +++++----- compiler/rustc_abi/src/lib.rs | 16 ++++----- .../src/abi/comments.rs | 2 +- compiler/rustc_middle/src/ty/layout.rs | 2 +- compiler/rustc_target/src/abi/call/x86.rs | 8 +++-- compiler/rustc_ty_utils/src/layout.rs | 10 +++--- tests/codegen/align-byval.rs | 11 +++--- tests/ui/layout/debug.stderr | 36 +++++++++---------- tests/ui/layout/hexagon-enum.stderr | 20 +++++------ ...-scalarpair-payload-might-be-uninit.stderr | 34 +++++++++--------- .../issue-96185-overaligned-enum.stderr | 24 +++++++++---- tests/ui/layout/thumb-enum.stderr | 20 +++++------ .../layout/zero-sized-array-enum-niche.stderr | 26 +++++++------- 13 files changed, 120 insertions(+), 107 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index efae34e95df..ff811be3c81 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -40,7 +40,7 @@ pub trait LayoutCalculator { largest_niche, align, size, - has_repr_align: false, + repr_align: None, } } @@ -123,7 +123,7 @@ pub trait LayoutCalculator { largest_niche: None, align: dl.i8_align, size: Size::ZERO, - has_repr_align: false, + repr_align: None, } } @@ -424,7 +424,7 @@ pub trait LayoutCalculator { largest_niche, size, align, - has_repr_align: repr.align.is_some(), + repr_align: repr.align, }; Some(TmpLayout { layout, variants: variant_layouts }) @@ -694,7 +694,7 @@ pub trait LayoutCalculator { abi, align, size, - has_repr_align: repr.align.is_some(), + repr_align: repr.align, }; let tagged_layout = TmpLayout { layout: tagged_layout, variants: layout_variants }; @@ -813,7 +813,7 @@ pub trait LayoutCalculator { largest_niche: None, align, size: size.align_to(align.abi), - has_repr_align: repr.align.is_some(), + repr_align: repr.align, }) } } @@ -1111,9 +1111,9 @@ fn univariant( abi = Abi::Uninhabited; } - let has_repr_align = repr.align.is_some() - || repr.transparent() - && layout_of_single_non_zst_field.map_or(false, |l| l.has_repr_align()); + let repr_align = repr.align.or_else(|| { + if repr.transparent() { layout_of_single_non_zst_field?.repr_align() } else { None } + }); Some(LayoutS { variants: Variants::Single { index: FIRST_VARIANT }, @@ -1122,7 +1122,7 @@ fn univariant( largest_niche, align, size, - has_repr_align, + repr_align, }) } diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index 4cf6289ae00..b1577add4d0 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1532,10 +1532,10 @@ pub struct LayoutS { pub align: AbiAndPrefAlign, pub size: Size, - /// True if the alignment was explicitly requested with `repr(align)`. + /// The alignment explicitly requested with `repr(align)`. /// Only used on i686-windows, where the argument passing ABI is different when alignment is - /// requested, even if the requested alignment is equal to or less than the natural alignment. - pub has_repr_align: bool, + /// requested, even if the requested alignment is equal to the natural alignment. + pub repr_align: Option, } impl LayoutS { @@ -1550,7 +1550,7 @@ impl LayoutS { largest_niche, size, align, - has_repr_align: false, + repr_align: None, } } } @@ -1560,7 +1560,7 @@ impl fmt::Debug for LayoutS { // This is how `Layout` used to print before it become // `Interned`. We print it like this to avoid having to update // expected output in a lot of tests. - let LayoutS { size, align, abi, fields, largest_niche, variants, has_repr_align } = self; + let LayoutS { size, align, abi, fields, largest_niche, variants, repr_align } = self; f.debug_struct("Layout") .field("size", size) .field("align", align) @@ -1568,7 +1568,7 @@ impl fmt::Debug for LayoutS { .field("fields", fields) .field("largest_niche", largest_niche) .field("variants", variants) - .field("has_repr_align", has_repr_align) + .field("repr_align", repr_align) .finish() } } @@ -1609,8 +1609,8 @@ impl<'a> Layout<'a> { self.0.0.size } - pub fn has_repr_align(self) -> bool { - self.0.0.has_repr_align + pub fn repr_align(self) -> Option { + self.0.0.repr_align } /// Whether the layout is from a type that implements [`std::marker::PointerLike`]. diff --git a/compiler/rustc_codegen_cranelift/src/abi/comments.rs b/compiler/rustc_codegen_cranelift/src/abi/comments.rs index f1ada7b7219..97f8452a468 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/comments.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/comments.rs @@ -83,7 +83,7 @@ pub(super) fn add_local_place_comments<'tcx>( let rustc_target::abi::LayoutS { size, align, - has_repr_align: _, + repr_align: _, abi: _, variants: _, fields: _, diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index cdf9cd8c809..ffebbab04a6 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -755,7 +755,7 @@ where largest_niche: None, align: tcx.data_layout.i8_align, size: Size::ZERO, - has_repr_align: false, + repr_align: None, }) } diff --git a/compiler/rustc_target/src/abi/call/x86.rs b/compiler/rustc_target/src/abi/call/x86.rs index 9d3f9413afd..e4292151065 100644 --- a/compiler/rustc_target/src/abi/call/x86.rs +++ b/compiler/rustc_target/src/abi/call/x86.rs @@ -63,8 +63,8 @@ where if t.is_like_msvc && arg.layout.is_adt() - && arg.layout.has_repr_align - && arg.layout.align.abi > align_4 + && let Some(repr_align) = arg.layout.repr_align + && repr_align > align_4 { // MSVC has special rules for overaligned arguments: https://reviews.llvm.org/D72114. // Summarized here: @@ -72,6 +72,10 @@ where // - 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:?}", + arg.layout.align.abi, + ); arg.make_indirect(); } else if arg.layout.is_aggregate() { // We need to compute the alignment of the `byval` argument. The rules can be found in diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index f693263ac95..4fbc6b9400f 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -258,7 +258,7 @@ fn layout_of_uncached<'tcx>( largest_niche, align: element.align, size, - has_repr_align: false, + repr_align: None, }) } ty::Slice(element) => { @@ -270,7 +270,7 @@ fn layout_of_uncached<'tcx>( largest_niche: None, align: element.align, size: Size::ZERO, - has_repr_align: false, + repr_align: None, }) } ty::Str => tcx.mk_layout(LayoutS { @@ -280,7 +280,7 @@ fn layout_of_uncached<'tcx>( largest_niche: None, align: dl.i8_align, size: Size::ZERO, - has_repr_align: false, + repr_align: None, }), // Odd unit types. @@ -434,7 +434,7 @@ fn layout_of_uncached<'tcx>( largest_niche: e_ly.largest_niche, size, align, - has_repr_align: false, + repr_align: None, }) } @@ -883,7 +883,7 @@ fn generator_layout<'tcx>( largest_niche: prefix.largest_niche, size, align, - has_repr_align: false, + repr_align: None, }); debug!("generator layout ({:?}): {:#?}", ty, layout); Ok(layout) diff --git a/tests/codegen/align-byval.rs b/tests/codegen/align-byval.rs index 2c64230bfd0..fc5f795bf67 100644 --- a/tests/codegen/align-byval.rs +++ b/tests/codegen/align-byval.rs @@ -71,17 +71,16 @@ pub struct ForceAlign8 { c: i64 } -// On i686-windows, this is passed by reference because alignment is requested, -// even though the requested alignment is less than the natural alignment. +// On i686-windows, this is passed on stack, because requested alignment is <=4. #[repr(C)] -#[repr(align(1))] +#[repr(align(4))] pub struct LowerFA8 { a: i64, b: i64, c: i64 } -// On i686-windows, this is passed on stack again, because the wrapper struct does not have +// On i686-windows, this is passed on stack, because the wrapper struct does not have // requested/forced alignment. #[repr(C)] pub struct WrappedFA8 { @@ -287,9 +286,7 @@ extern "C" { // i686-linux: declare void @lower_fa8({{.*}}byval(%LowerFA8) align 4{{.*}}) - // i686-windows: declare void @lower_fa8( - // i686-windows-NOT: byval - // i686-windows-SAME: align 8{{.*}}) + // i686-windows: declare void @lower_fa8({{.*}}byval(%LowerFA8) align 4{{.*}}) fn lower_fa8(x: LowerFA8); // m68k: declare void @wrapped_fa8({{.*}}byval(%WrappedFA8) align 8{{.*}}) diff --git a/tests/ui/layout/debug.stderr b/tests/ui/layout/debug.stderr index 4f27eaae8d8..99022fb11b5 100644 --- a/tests/ui/layout/debug.stderr +++ b/tests/ui/layout/debug.stderr @@ -53,7 +53,7 @@ error: layout_of(E) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(12 bytes), @@ -78,11 +78,11 @@ error: layout_of(E) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:7:1 | @@ -127,7 +127,7 @@ error: layout_of(S) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:10:1 | @@ -150,7 +150,7 @@ error: layout_of(U) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:13:1 | @@ -242,7 +242,7 @@ error: layout_of(std::result::Result) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(8 bytes), @@ -278,11 +278,11 @@ error: layout_of(std::result::Result) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:16:1 | @@ -309,7 +309,7 @@ error: layout_of(i32) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:19:1 | @@ -332,7 +332,7 @@ error: layout_of(V) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:22:1 | @@ -355,7 +355,7 @@ error: layout_of(W) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:28:1 | @@ -378,7 +378,7 @@ error: layout_of(Y) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:34:1 | @@ -401,7 +401,7 @@ error: layout_of(P1) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:41:1 | @@ -424,7 +424,7 @@ error: layout_of(P2) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:45:1 | @@ -447,7 +447,7 @@ error: layout_of(P3) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:53:1 | @@ -470,7 +470,7 @@ error: layout_of(P4) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:57:1 | @@ -498,7 +498,7 @@ error: layout_of(P5) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:61:1 | @@ -526,7 +526,7 @@ error: layout_of(std::mem::MaybeUninit) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, } --> $DIR/debug.rs:64:1 | diff --git a/tests/ui/layout/hexagon-enum.stderr b/tests/ui/layout/hexagon-enum.stderr index 08c4e547ff2..a907853585d 100644 --- a/tests/ui/layout/hexagon-enum.stderr +++ b/tests/ui/layout/hexagon-enum.stderr @@ -59,11 +59,11 @@ error: layout_of(A) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/hexagon-enum.rs:16:1 | @@ -131,11 +131,11 @@ error: layout_of(B) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/hexagon-enum.rs:20:1 | @@ -203,11 +203,11 @@ error: layout_of(C) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/hexagon-enum.rs:24:1 | @@ -275,11 +275,11 @@ error: layout_of(P) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/hexagon-enum.rs:28:1 | @@ -347,11 +347,11 @@ error: layout_of(T) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/hexagon-enum.rs:34:1 | diff --git a/tests/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr b/tests/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr index 5fae4d9a6a5..2db6ab9bc89 100644 --- a/tests/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr +++ b/tests/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr @@ -81,7 +81,7 @@ error: layout_of(MissingPayloadField) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(1 bytes), @@ -100,11 +100,11 @@ error: layout_of(MissingPayloadField) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:16:1 | @@ -196,7 +196,7 @@ error: layout_of(CommonPayloadField) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(2 bytes), @@ -232,11 +232,11 @@ error: layout_of(CommonPayloadField) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:25:1 | @@ -326,7 +326,7 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(2 bytes), @@ -361,11 +361,11 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:33:1 | @@ -471,7 +471,7 @@ error: layout_of(NicheFirst) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(0 bytes), @@ -490,7 +490,7 @@ error: layout_of(NicheFirst) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(0 bytes), @@ -509,11 +509,11 @@ error: layout_of(NicheFirst) = Layout { variants: Single { index: 2, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:41:1 | @@ -619,7 +619,7 @@ error: layout_of(NicheSecond) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(0 bytes), @@ -638,7 +638,7 @@ error: layout_of(NicheSecond) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(0 bytes), @@ -657,11 +657,11 @@ error: layout_of(NicheSecond) = Layout { variants: Single { index: 2, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:50:1 | diff --git a/tests/ui/layout/issue-96185-overaligned-enum.stderr b/tests/ui/layout/issue-96185-overaligned-enum.stderr index fe1d4392b67..3a0ad07347d 100644 --- a/tests/ui/layout/issue-96185-overaligned-enum.stderr +++ b/tests/ui/layout/issue-96185-overaligned-enum.stderr @@ -53,7 +53,9 @@ error: layout_of(Aligned1) = Layout { variants: Single { index: 0, }, - has_repr_align: true, + repr_align: Some( + Align(8 bytes), + ), }, Layout { size: Size(8 bytes), @@ -72,11 +74,15 @@ error: layout_of(Aligned1) = Layout { variants: Single { index: 1, }, - has_repr_align: true, + repr_align: Some( + Align(8 bytes), + ), }, ], }, - has_repr_align: true, + repr_align: Some( + Align(8 bytes), + ), } --> $DIR/issue-96185-overaligned-enum.rs:8:1 | @@ -144,7 +150,9 @@ error: layout_of(Aligned2) = Layout { variants: Single { index: 0, }, - has_repr_align: true, + repr_align: Some( + Align(1 bytes), + ), }, Layout { size: Size(1 bytes), @@ -163,11 +171,15 @@ error: layout_of(Aligned2) = Layout { variants: Single { index: 1, }, - has_repr_align: true, + repr_align: Some( + Align(1 bytes), + ), }, ], }, - has_repr_align: true, + repr_align: Some( + Align(1 bytes), + ), } --> $DIR/issue-96185-overaligned-enum.rs:16:1 | diff --git a/tests/ui/layout/thumb-enum.stderr b/tests/ui/layout/thumb-enum.stderr index 7aca1aa462e..27bba72fcfe 100644 --- a/tests/ui/layout/thumb-enum.stderr +++ b/tests/ui/layout/thumb-enum.stderr @@ -59,11 +59,11 @@ error: layout_of(A) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/thumb-enum.rs:16:1 | @@ -131,11 +131,11 @@ error: layout_of(B) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/thumb-enum.rs:20:1 | @@ -203,11 +203,11 @@ error: layout_of(C) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/thumb-enum.rs:24:1 | @@ -275,11 +275,11 @@ error: layout_of(P) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/thumb-enum.rs:28:1 | @@ -347,11 +347,11 @@ error: layout_of(T) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/thumb-enum.rs:34:1 | diff --git a/tests/ui/layout/zero-sized-array-enum-niche.stderr b/tests/ui/layout/zero-sized-array-enum-niche.stderr index 8342d64b6fe..a9a242943ba 100644 --- a/tests/ui/layout/zero-sized-array-enum-niche.stderr +++ b/tests/ui/layout/zero-sized-array-enum-niche.stderr @@ -57,7 +57,7 @@ error: layout_of(std::result::Result<[u32; 0], bool>) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(2 bytes), @@ -89,11 +89,11 @@ error: layout_of(std::result::Result<[u32; 0], bool>) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/zero-sized-array-enum-niche.rs:13:1 | @@ -159,7 +159,7 @@ error: layout_of(MultipleAlignments) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(4 bytes), @@ -182,7 +182,7 @@ error: layout_of(MultipleAlignments) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(2 bytes), @@ -214,11 +214,11 @@ error: layout_of(MultipleAlignments) = Layout { variants: Single { index: 2, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/zero-sized-array-enum-niche.rs:21:1 | @@ -284,7 +284,7 @@ error: layout_of(std::result::Result<[u32; 0], Packed>) = variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(3 bytes), @@ -316,11 +316,11 @@ error: layout_of(std::result::Result<[u32; 0], Packed>) = variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/zero-sized-array-enum-niche.rs:37:1 | @@ -390,7 +390,7 @@ error: layout_of(std::result::Result<[u32; 0], Packed>) = Layout { variants: Single { index: 0, }, - has_repr_align: false, + repr_align: None, }, Layout { size: Size(2 bytes), @@ -422,11 +422,11 @@ error: layout_of(std::result::Result<[u32; 0], Packed>) = Layout { variants: Single { index: 1, }, - has_repr_align: false, + repr_align: None, }, ], }, - has_repr_align: false, + repr_align: None, } --> $DIR/zero-sized-array-enum-niche.rs:44:1 |