From be8e67d93c2daafcb006d7dc55b4b270c99d77f3 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Thu, 16 Feb 2023 01:50:57 +0100 Subject: [PATCH 1/8] refactor: extract function --- compiler/rustc_abi/src/layout.rs | 434 ++++++++++++++++--------------- 1 file changed, 220 insertions(+), 214 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index f3af031ade4..a76ac5f98e6 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -49,220 +49,7 @@ pub trait LayoutCalculator { repr: &ReprOptions, kind: StructKind, ) -> Option { - let pack = repr.pack; - let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align }; - let mut inverse_memory_index: IndexVec = fields.indices().collect(); - let optimize = !repr.inhibit_struct_field_reordering_opt(); - if optimize { - let end = - if let StructKind::MaybeUnsized = kind { fields.len() - 1 } else { fields.len() }; - let optimizing = &mut inverse_memory_index.raw[..end]; - let effective_field_align = |layout: Layout<'_>| { - if let Some(pack) = pack { - // return the packed alignment in bytes - layout.align().abi.min(pack).bytes() - } else { - // returns log2(effective-align). - // This is ok since `pack` applies to all fields equally. - // The calculation assumes that size is an integer multiple of align, except for ZSTs. - // - // group [u8; 4] with align-4 or [u8; 6] with align-2 fields - layout.align().abi.bytes().max(layout.size().bytes()).trailing_zeros() as u64 - } - }; - - // If `-Z randomize-layout` was enabled for the type definition we can shuffle - // the field ordering to try and catch some code making assumptions about layouts - // we don't guarantee - if repr.can_randomize_type_layout() && cfg!(feature = "randomize") { - #[cfg(feature = "randomize")] - { - // `ReprOptions.layout_seed` is a deterministic seed that we can use to - // randomize field ordering with - let mut rng = - Xoshiro128StarStar::seed_from_u64(repr.field_shuffle_seed.as_u64()); - - // Shuffle the ordering of the fields - optimizing.shuffle(&mut rng); - } - // Otherwise we just leave things alone and actually optimize the type's fields - } else { - match kind { - StructKind::AlwaysSized | StructKind::MaybeUnsized => { - optimizing.sort_by_key(|&x| { - // Place ZSTs first to avoid "interesting offsets", - // especially with only one or two non-ZST fields. - // Then place largest alignments first, largest niches within an alignment group last - let f = fields[x]; - let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); - (!f.0.is_zst(), cmp::Reverse(effective_field_align(f)), niche_size) - }); - } - - StructKind::Prefixed(..) => { - // Sort in ascending alignment so that the layout stays optimal - // regardless of the prefix. - // And put the largest niche in an alignment group at the end - // so it can be used as discriminant in jagged enums - optimizing.sort_by_key(|&x| { - let f = fields[x]; - let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); - (effective_field_align(f), niche_size) - }); - } - } - - // FIXME(Kixiron): We can always shuffle fields within a given alignment class - // regardless of the status of `-Z randomize-layout` - } - } - // inverse_memory_index holds field indices by increasing memory offset. - // That is, if field 5 has offset 0, the first element of inverse_memory_index is 5. - // We now write field offsets to the corresponding offset slot; - // field 5 with offset 0 puts 0 in offsets[5]. - // At the bottom of this function, we invert `inverse_memory_index` to - // produce `memory_index` (see `invert_mapping`). - let mut sized = true; - let mut offsets = IndexVec::from_elem(Size::ZERO, &fields); - let mut offset = Size::ZERO; - let mut largest_niche = None; - let mut largest_niche_available = 0; - if let StructKind::Prefixed(prefix_size, prefix_align) = kind { - let prefix_align = - if let Some(pack) = pack { prefix_align.min(pack) } else { prefix_align }; - align = align.max(AbiAndPrefAlign::new(prefix_align)); - offset = prefix_size.align_to(prefix_align); - } - for &i in &inverse_memory_index { - let field = &fields[i]; - if !sized { - self.delay_bug(&format!( - "univariant: field #{} comes after unsized field", - offsets.len(), - )); - } - - if field.0.is_unsized() { - sized = false; - } - - // Invariant: offset < dl.obj_size_bound() <= 1<<61 - let field_align = if let Some(pack) = pack { - field.align().min(AbiAndPrefAlign::new(pack)) - } else { - field.align() - }; - offset = offset.align_to(field_align.abi); - align = align.max(field_align); - - debug!("univariant offset: {:?} field: {:#?}", offset, field); - offsets[i] = offset; - - if let Some(mut niche) = field.largest_niche() { - let available = niche.available(dl); - if available > largest_niche_available { - largest_niche_available = available; - niche.offset += offset; - largest_niche = Some(niche); - } - } - - offset = offset.checked_add(field.size(), dl)?; - } - if let Some(repr_align) = repr.align { - align = align.max(AbiAndPrefAlign::new(repr_align)); - } - debug!("univariant min_size: {:?}", offset); - let min_size = offset; - // As stated above, inverse_memory_index holds field indices by increasing offset. - // This makes it an already-sorted view of the offsets vec. - // To invert it, consider: - // If field 5 has offset 0, offsets[0] is 5, and memory_index[5] should be 0. - // Field 5 would be the first element, so memory_index is i: - // Note: if we didn't optimize, it's already right. - let memory_index = if optimize { - inverse_memory_index.invert_bijective_mapping() - } else { - debug_assert!(inverse_memory_index.iter().copied().eq(fields.indices())); - inverse_memory_index.into_iter().map(FieldIdx::as_u32).collect() - }; - let size = min_size.align_to(align.abi); - let mut abi = Abi::Aggregate { sized }; - // Unpack newtype ABIs and find scalar pairs. - if sized && size.bytes() > 0 { - // All other fields must be ZSTs. - let mut non_zst_fields = fields.iter_enumerated().filter(|&(_, f)| !f.0.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(); - } - // But scalar pairs are Rust-specific and get - // treated as aggregates by C ABIs anyway. - Abi::ScalarPair(..) => { - abi = field.abi(); - } - _ => {} - } - } - } - - // Two non-ZST fields, and they're both scalars. - (Some((i, a)), Some((j, b)), None) => { - match (a.abi(), b.abi()) { - (Abi::Scalar(a), Abi::Scalar(b)) => { - // 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, b); - let pair_offsets = match pair.fields { - FieldsShape::Arbitrary { ref offsets, ref memory_index } => { - assert_eq!(memory_index.raw, [0, 1]); - offsets - } - _ => panic!(), - }; - if offsets[i] == pair_offsets[FieldIdx::from_usize(0)] - && offsets[j] == pair_offsets[FieldIdx::from_usize(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; - } - } - _ => {} - } - } - - _ => {} - } - } - if fields.iter().any(|f| f.abi().is_uninhabited()) { - abi = Abi::Uninhabited; - } - Some(LayoutS { - variants: Variants::Single { index: FIRST_VARIANT }, - fields: FieldsShape::Arbitrary { offsets, memory_index }, - abi, - largest_niche, - align, - size, - }) + univariant(self, dl, fields, repr, kind) } fn layout_of_never_type(&self) -> LayoutS { @@ -934,3 +721,222 @@ pub trait LayoutCalculator { }) } } + +fn univariant( + this: &(impl LayoutCalculator + ?Sized), + dl: &TargetDataLayout, + fields: &IndexSlice>, + repr: &ReprOptions, + kind: StructKind, +) -> Option { + let pack = repr.pack; + let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align }; + let mut inverse_memory_index: IndexVec = fields.indices().collect(); + let optimize = !repr.inhibit_struct_field_reordering_opt(); + if optimize { + let end = if let StructKind::MaybeUnsized = kind { fields.len() - 1 } else { fields.len() }; + let optimizing = &mut inverse_memory_index.raw[..end]; + let effective_field_align = |layout: Layout<'_>| { + if let Some(pack) = pack { + // return the packed alignment in bytes + layout.align().abi.min(pack).bytes() + } else { + // returns log2(effective-align). + // This is ok since `pack` applies to all fields equally. + // The calculation assumes that size is an integer multiple of align, except for ZSTs. + // + // group [u8; 4] with align-4 or [u8; 6] with align-2 fields + layout.align().abi.bytes().max(layout.size().bytes()).trailing_zeros() as u64 + } + }; + + // If `-Z randomize-layout` was enabled for the type definition we can shuffle + // the field ordering to try and catch some code making assumptions about layouts + // we don't guarantee + if repr.can_randomize_type_layout() && cfg!(feature = "randomize") { + #[cfg(feature = "randomize")] + { + // `ReprOptions.layout_seed` is a deterministic seed that we can use to + // randomize field ordering with + let mut rng = Xoshiro128StarStar::seed_from_u64(repr.field_shuffle_seed.as_u64()); + + // Shuffle the ordering of the fields + optimizing.shuffle(&mut rng); + } + // Otherwise we just leave things alone and actually optimize the type's fields + } else { + match kind { + StructKind::AlwaysSized | StructKind::MaybeUnsized => { + optimizing.sort_by_key(|&x| { + // Place ZSTs first to avoid "interesting offsets", + // especially with only one or two non-ZST fields. + // Then place largest alignments first, largest niches within an alignment group last + let f = fields[x]; + let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); + (!f.0.is_zst(), cmp::Reverse(effective_field_align(f)), niche_size) + }); + } + + StructKind::Prefixed(..) => { + // Sort in ascending alignment so that the layout stays optimal + // regardless of the prefix. + // And put the largest niche in an alignment group at the end + // so it can be used as discriminant in jagged enums + optimizing.sort_by_key(|&x| { + let f = fields[x]; + let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); + (effective_field_align(f), niche_size) + }); + } + } + + // FIXME(Kixiron): We can always shuffle fields within a given alignment class + // regardless of the status of `-Z randomize-layout` + } + } + // inverse_memory_index holds field indices by increasing memory offset. + // That is, if field 5 has offset 0, the first element of inverse_memory_index is 5. + // We now write field offsets to the corresponding offset slot; + // field 5 with offset 0 puts 0 in offsets[5]. + // At the bottom of this function, we invert `inverse_memory_index` to + // produce `memory_index` (see `invert_mapping`). + let mut sized = true; + let mut offsets = IndexVec::from_elem(Size::ZERO, &fields); + let mut offset = Size::ZERO; + let mut largest_niche = None; + let mut largest_niche_available = 0; + if let StructKind::Prefixed(prefix_size, prefix_align) = kind { + let prefix_align = + if let Some(pack) = pack { prefix_align.min(pack) } else { prefix_align }; + align = align.max(AbiAndPrefAlign::new(prefix_align)); + offset = prefix_size.align_to(prefix_align); + } + for &i in &inverse_memory_index { + let field = &fields[i]; + if !sized { + this.delay_bug(&format!( + "univariant: field #{} comes after unsized field", + offsets.len(), + )); + } + + if field.0.is_unsized() { + sized = false; + } + + // Invariant: offset < dl.obj_size_bound() <= 1<<61 + let field_align = if let Some(pack) = pack { + field.align().min(AbiAndPrefAlign::new(pack)) + } else { + field.align() + }; + offset = offset.align_to(field_align.abi); + align = align.max(field_align); + + debug!("univariant offset: {:?} field: {:#?}", offset, field); + offsets[i] = offset; + + if let Some(mut niche) = field.largest_niche() { + let available = niche.available(dl); + if available > largest_niche_available { + largest_niche_available = available; + niche.offset += offset; + largest_niche = Some(niche); + } + } + + offset = offset.checked_add(field.size(), dl)?; + } + if let Some(repr_align) = repr.align { + align = align.max(AbiAndPrefAlign::new(repr_align)); + } + debug!("univariant min_size: {:?}", offset); + let min_size = offset; + // As stated above, inverse_memory_index holds field indices by increasing offset. + // This makes it an already-sorted view of the offsets vec. + // To invert it, consider: + // If field 5 has offset 0, offsets[0] is 5, and memory_index[5] should be 0. + // Field 5 would be the first element, so memory_index is i: + // Note: if we didn't optimize, it's already right. + let memory_index = if optimize { + inverse_memory_index.invert_bijective_mapping() + } else { + debug_assert!(inverse_memory_index.iter().copied().eq(fields.indices())); + inverse_memory_index.into_iter().map(FieldIdx::as_u32).collect() + }; + let size = min_size.align_to(align.abi); + let mut abi = Abi::Aggregate { sized }; + // Unpack newtype ABIs and find scalar pairs. + if sized && size.bytes() > 0 { + // All other fields must be ZSTs. + let mut non_zst_fields = fields.iter_enumerated().filter(|&(_, f)| !f.0.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(); + } + // But scalar pairs are Rust-specific and get + // treated as aggregates by C ABIs anyway. + Abi::ScalarPair(..) => { + abi = field.abi(); + } + _ => {} + } + } + } + + // Two non-ZST fields, and they're both scalars. + (Some((i, a)), Some((j, b)), None) => { + match (a.abi(), b.abi()) { + (Abi::Scalar(a), Abi::Scalar(b)) => { + // 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 = this.scalar_pair(a, b); + let pair_offsets = match pair.fields { + FieldsShape::Arbitrary { ref offsets, ref memory_index } => { + assert_eq!(memory_index.raw, [0, 1]); + offsets + } + _ => panic!(), + }; + if offsets[i] == pair_offsets[FieldIdx::from_usize(0)] + && offsets[j] == pair_offsets[FieldIdx::from_usize(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; + } + } + _ => {} + } + } + + _ => {} + } + } + if fields.iter().any(|f| f.abi().is_uninhabited()) { + abi = Abi::Uninhabited; + } + Some(LayoutS { + variants: Variants::Single { index: FIRST_VARIANT }, + fields: FieldsShape::Arbitrary { offsets, memory_index }, + abi, + largest_niche, + align, + size, + }) +} From faf2da3e2f04f525784fd4d41375e96a8356f4e3 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Thu, 16 Feb 2023 01:53:47 +0100 Subject: [PATCH 2/8] try two different niche-placement strategies when layouting univariant structs --- compiler/rustc_abi/src/layout.rs | 76 +++++++++++++++++++++++++--- tests/ui/structs-enums/type-sizes.rs | 30 ++++++++++- 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index a76ac5f98e6..a833302d566 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -49,7 +49,42 @@ pub trait LayoutCalculator { repr: &ReprOptions, kind: StructKind, ) -> Option { - univariant(self, dl, fields, repr, kind) + let layout = univariant(self, dl, fields, repr, kind, true); + // Enums prefer niches close to the beginning or the end of the variants so that other (smaller) + // data-carrying variants can be packed into the space after/before the niche. + // If the default field ordering does not give us a niche at the front then we do a second + // run and bias niches to the right and then check which one is closer to one of the struct's + // edges. + if let Some(layout) = &layout { + if let Some(niche) = layout.largest_niche { + let head_space = niche.offset.bytes(); + let niche_length = niche.value.size(dl).bytes(); + let tail_space = layout.size.bytes() - head_space - niche_length; + + // This may end up doing redundant work if the niche is already in the last field + // (e.g. a trailing bool) and there is tail padding. But it's non-trivial to get + // the unpadded size so we try anyway. + if fields.len() > 1 && head_space != 0 && tail_space > 0 { + let alt_layout = univariant(self, dl, fields, repr, kind, false) + .expect("alt layout should always work"); + let niche = alt_layout + .largest_niche + .expect("alt layout should have a niche like the regular one"); + let alt_head_space = niche.offset.bytes(); + let alt_niche_len = niche.value.size(dl).bytes(); + + debug_assert_eq!(layout.size.bytes(), alt_layout.size.bytes()); + + let prefer_alt_layout = + alt_head_space > head_space && alt_head_space > tail_space; + + if prefer_alt_layout { + return Some(alt_layout); + } + } + } + } + layout } fn layout_of_never_type(&self) -> LayoutS { @@ -728,6 +763,7 @@ fn univariant( fields: &IndexSlice>, repr: &ReprOptions, kind: StructKind, + niche_bias_start: bool, ) -> Option { let pack = repr.pack; let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align }; @@ -768,12 +804,35 @@ fn univariant( match kind { StructKind::AlwaysSized | StructKind::MaybeUnsized => { optimizing.sort_by_key(|&x| { - // Place ZSTs first to avoid "interesting offsets", - // especially with only one or two non-ZST fields. - // Then place largest alignments first, largest niches within an alignment group last let f = fields[x]; + let field_size = f.size().bytes(); let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); - (!f.0.is_zst(), cmp::Reverse(effective_field_align(f)), niche_size) + let niche_size = if niche_bias_start { + u128::MAX - niche_size // large niche first + } else { + niche_size // large niche last + }; + let inner_niche_placement = if niche_bias_start { + f.largest_niche().map_or(0, |n| n.offset.bytes()) + } else { + f.largest_niche().map_or(0, |n| { + field_size - n.value.size(dl).bytes() - n.offset.bytes() + }) + }; + + ( + // Place ZSTs first to avoid "interesting offsets", especially with only one + // or two non-ZST fields. This helps Scalar/ScalarPair layouts. + !f.0.is_zst(), + // Then place largest alignments first. + cmp::Reverse(effective_field_align(f)), + // Then prioritize niche placement within alignment group according to + // `niche_bias_start`. + niche_size, + // Then among fields with equally-sized niches prefer the ones + // closer to the start/end of the field. + inner_niche_placement, + ) }); } @@ -838,7 +897,12 @@ fn univariant( if let Some(mut niche) = field.largest_niche() { let available = niche.available(dl); - if available > largest_niche_available { + let prefer_new_niche = if niche_bias_start { + available > largest_niche_available + } else { + available >= largest_niche_available + }; + if prefer_new_niche { largest_niche_available = available; niche.offset += offset; largest_niche = Some(niche); diff --git a/tests/ui/structs-enums/type-sizes.rs b/tests/ui/structs-enums/type-sizes.rs index 63e2f3150c0..4bae1e07d0a 100644 --- a/tests/ui/structs-enums/type-sizes.rs +++ b/tests/ui/structs-enums/type-sizes.rs @@ -4,9 +4,14 @@ #![allow(dead_code)] #![feature(never_type)] #![feature(pointer_is_aligned)] +#![feature(ptr_from_ref)] +#![feature(strict_provenance)] use std::mem::size_of; -use std::num::NonZeroU8; +use std::num::{NonZeroU8, NonZeroU16}; +use std::ptr; +use std::ptr::NonNull; +use std::borrow::Cow; struct t {a: u8, b: i8} struct u {a: u8, b: i8, c: u8} @@ -181,6 +186,17 @@ struct Reorder2 { ary: [u8; 6], } +// standins for std types which we want to be laid out in a reasonable way +struct RawVecDummy { + ptr: NonNull, + cap: usize, +} + +struct VecDummy { + r: RawVecDummy, + len: usize, +} + pub fn main() { assert_eq!(size_of::(), 1 as usize); assert_eq!(size_of::(), 4 as usize); @@ -270,4 +286,16 @@ pub fn main() { let v = Reorder2 {a: 0, b: 0, ary: [0; 6]}; assert_eq!(size_of::(), 10); assert!((&v.ary).as_ptr().is_aligned_to(2), "[u8; 6] should group with align-2 fields"); + + let v = VecDummy { r: RawVecDummy { ptr: NonNull::dangling(), cap: 0 }, len: 1 }; + assert_eq!(ptr::from_ref(&v), ptr::from_ref(&v.r.ptr).cast(), + "sort niches to the front where possible"); + + // Ideal layouts: (bool, u8, NonZeroU16) or (NonZeroU16, u8, bool) + // Currently the layout algorithm will choose the latter because it doesn't attempt + // to aggregate multiple smaller fields to move a niche before a higher-alignment one. + let b = BoolInTheMiddle( NonZeroU16::new(1).unwrap(), true, 0); + assert!(ptr::from_ref(&b.1).addr() > ptr::from_ref(&b.2).addr()); + + assert_eq!(size_of::>(), size_of::()); } From 4907dac54cc43d44bd6df87636e545756d110957 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sun, 5 Mar 2023 16:15:16 +0100 Subject: [PATCH 3/8] don't promote large fields to higher alignments if that would affect niche placement --- compiler/rustc_abi/src/layout.rs | 37 ++++++++++++++++++---------- tests/ui/structs-enums/type-sizes.rs | 18 ++++++++++++++ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index a833302d566..0b0fea4c500 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -772,19 +772,6 @@ fn univariant( if optimize { let end = if let StructKind::MaybeUnsized = kind { fields.len() - 1 } else { fields.len() }; let optimizing = &mut inverse_memory_index.raw[..end]; - let effective_field_align = |layout: Layout<'_>| { - if let Some(pack) = pack { - // return the packed alignment in bytes - layout.align().abi.min(pack).bytes() - } else { - // returns log2(effective-align). - // This is ok since `pack` applies to all fields equally. - // The calculation assumes that size is an integer multiple of align, except for ZSTs. - // - // group [u8; 4] with align-4 or [u8; 6] with align-2 fields - layout.align().abi.bytes().max(layout.size().bytes()).trailing_zeros() as u64 - } - }; // If `-Z randomize-layout` was enabled for the type definition we can shuffle // the field ordering to try and catch some code making assumptions about layouts @@ -801,6 +788,30 @@ fn univariant( } // Otherwise we just leave things alone and actually optimize the type's fields } else { + let max_field_align = fields.iter().map(|f| f.align().abi.bytes()).max().unwrap_or(1); + let any_niche = fields.iter().any(|f| f.largest_niche().is_some()); + let effective_field_align = |layout: Layout<'_>| { + if let Some(pack) = pack { + // return the packed alignment in bytes + layout.align().abi.min(pack).bytes() + } else { + // returns log2(effective-align). + // This is ok since `pack` applies to all fields equally. + // The calculation assumes that size is an integer multiple of align, except for ZSTs. + // + // group [u8; 4] with align-4 or [u8; 6] with align-2 fields + let align = layout.align().abi.bytes(); + let size = layout.size().bytes(); + let size_as_align = align.max(size).trailing_zeros(); + let size_as_align = if any_niche { + max_field_align.trailing_zeros().min(size_as_align) + } else { + size_as_align + }; + size_as_align as u64 + } + }; + match kind { StructKind::AlwaysSized | StructKind::MaybeUnsized => { optimizing.sort_by_key(|&x| { diff --git a/tests/ui/structs-enums/type-sizes.rs b/tests/ui/structs-enums/type-sizes.rs index 4bae1e07d0a..e5c6857a26a 100644 --- a/tests/ui/structs-enums/type-sizes.rs +++ b/tests/ui/structs-enums/type-sizes.rs @@ -186,6 +186,18 @@ struct Reorder2 { ary: [u8; 6], } +// We want the niche in the front, which means we can't treat the array as quasi-aligned more than +// 4 bytes even though we also want to place it at an 8-aligned offset where possible. +// So the ideal layout would look like: (char, u32, [u8; 8], u8) +// The current layout algorithm does (char, [u8; 8], u32, u8) +#[repr(align(8))] +struct ReorderWithNiche { + a: u32, + b: char, + c: u8, + ary: [u8; 8] +} + // standins for std types which we want to be laid out in a reasonable way struct RawVecDummy { ptr: NonNull, @@ -298,4 +310,10 @@ pub fn main() { assert!(ptr::from_ref(&b.1).addr() > ptr::from_ref(&b.2).addr()); assert_eq!(size_of::>(), size_of::()); + + let v = ReorderWithNiche {a: 0, b: ' ', c: 0, ary: [0; 8]}; + assert!((&v.ary).as_ptr().is_aligned_to(4), + "here [u8; 8] should group with _at least_ align-4 fields"); + assert_eq!(ptr::from_ref(&v), ptr::from_ref(&v.b).cast(), + "sort niches to the front where possible"); } From 351e208f4c24069c141b630f35c09ebe9107fd5a Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sun, 5 Mar 2023 16:18:19 +0100 Subject: [PATCH 4/8] add tracing for layout optimizations --- compiler/rustc_abi/src/layout.rs | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index 0b0fea4c500..f15fb877d51 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -1,4 +1,5 @@ use super::*; +use std::fmt::Write; use std::{borrow::Borrow, cmp, iter, ops::Bound}; #[cfg(feature = "randomize")] @@ -72,12 +73,30 @@ pub trait LayoutCalculator { .expect("alt layout should have a niche like the regular one"); let alt_head_space = niche.offset.bytes(); let alt_niche_len = niche.value.size(dl).bytes(); + let alt_tail_space = alt_layout.size.bytes() - alt_head_space - alt_niche_len; debug_assert_eq!(layout.size.bytes(), alt_layout.size.bytes()); let prefer_alt_layout = alt_head_space > head_space && alt_head_space > tail_space; + debug!( + "sz: {}, default_niche_at: {}+{}, default_tail_space: {}, alt_niche_at/head_space: {}+{}, alt_tail: {}, num_fields: {}, better: {}\n\ + layout: {}\n\ + alt_layout: {}\n", + layout.size.bytes(), + head_space, + niche_length, + tail_space, + alt_head_space, + alt_niche_len, + alt_tail_space, + layout.fields.count(), + prefer_alt_layout, + format_field_niches(&layout, &fields, &dl), + format_field_niches(&alt_layout, &fields, &dl), + ); + if prefer_alt_layout { return Some(alt_layout); } @@ -1015,3 +1034,28 @@ fn univariant( size, }) } + +fn format_field_niches( + layout: &LayoutS, + fields: &IndexSlice>, + dl: &TargetDataLayout, +) -> String { + let mut s = String::new(); + for i in layout.fields.index_by_increasing_offset() { + let offset = layout.fields.offset(i); + let f = fields[i.into()]; + write!(s, "[o{}a{}s{}", offset.bytes(), f.align().abi.bytes(), f.size().bytes()).unwrap(); + if let Some(n) = f.largest_niche() { + write!( + s, + " n{}b{}s{}", + n.offset.bytes(), + n.available(dl).ilog2(), + n.value.size(dl).bytes() + ) + .unwrap(); + } + write!(s, "] ").unwrap(); + } + s +} From 1a51ec686439b52bd652b32f8f7aaf7817faf5a7 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Thu, 16 Feb 2023 01:49:00 +0100 Subject: [PATCH 5/8] bless tests --- tests/codegen/issues/issue-86106.rs | 30 ++++------- .../future-sizes/async-awaiting-fut.stdout | 40 +++++++------- .../async-await/future-sizes/large-arg.stdout | 24 ++++----- .../consts/const-eval/raw-bytes.32bit.stderr | 4 +- .../consts/const-eval/raw-bytes.64bit.stderr | 4 +- ...-scalarpair-payload-might-be-uninit.stderr | 52 +++++++++---------- tests/ui/print_type_sizes/async.stdout | 8 +-- tests/ui/print_type_sizes/generator.stdout | 8 +-- 8 files changed, 77 insertions(+), 93 deletions(-) diff --git a/tests/codegen/issues/issue-86106.rs b/tests/codegen/issues/issue-86106.rs index 9ccbcb24f56..af621d79391 100644 --- a/tests/codegen/issues/issue-86106.rs +++ b/tests/codegen/issues/issue-86106.rs @@ -9,12 +9,9 @@ // CHECK-LABEL: define void @string_new #[no_mangle] pub fn string_new() -> String { - // CHECK-NOT: load i8 - // CHECK: store i{{32|64}} + // CHECK: store ptr inttoptr // CHECK-NEXT: getelementptr - // CHECK-NEXT: store ptr - // CHECK-NEXT: getelementptr - // CHECK-NEXT: store i{{32|64}} + // CHECK-NEXT: call void @llvm.memset // CHECK-NEXT: ret void String::new() } @@ -22,12 +19,9 @@ pub fn string_new() -> String { // CHECK-LABEL: define void @empty_to_string #[no_mangle] pub fn empty_to_string() -> String { - // CHECK-NOT: load i8 - // CHECK: store i{{32|64}} - // CHECK-NEXT: getelementptr - // CHECK-NEXT: store ptr - // CHECK-NEXT: getelementptr - // CHECK-NEXT: store i{{32|64}} + // CHECK: getelementptr + // CHECK-NEXT: call void @llvm.memset + // CHECK-NEXT: store ptr inttoptr // CHECK-NEXT: ret void "".to_string() } @@ -38,12 +32,9 @@ pub fn empty_to_string() -> String { // CHECK-LABEL: @empty_vec #[no_mangle] pub fn empty_vec() -> Vec { - // CHECK: store i{{32|64}} - // CHECK-NOT: load i8 + // CHECK: store ptr inttoptr // CHECK-NEXT: getelementptr - // CHECK-NEXT: store ptr - // CHECK-NEXT: getelementptr - // CHECK-NEXT: store i{{32|64}} + // CHECK-NEXT: call void @llvm.memset // CHECK-NEXT: ret void vec![] } @@ -51,12 +42,9 @@ pub fn empty_vec() -> Vec { // CHECK-LABEL: @empty_vec_clone #[no_mangle] pub fn empty_vec_clone() -> Vec { - // CHECK: store i{{32|64}} - // CHECK-NOT: load i8 + // CHECK: store ptr inttoptr // CHECK-NEXT: getelementptr - // CHECK-NEXT: store ptr - // CHECK-NEXT: getelementptr - // CHECK-NEXT: store i{{32|64}} + // CHECK-NEXT: call void @llvm.memset // CHECK-NEXT: ret void vec![].clone() } diff --git a/tests/ui/async-await/future-sizes/async-awaiting-fut.stdout b/tests/ui/async-await/future-sizes/async-awaiting-fut.stdout index eaf3e4b61e3..c0fbb0204b3 100644 --- a/tests/ui/async-await/future-sizes/async-awaiting-fut.stdout +++ b/tests/ui/async-await/future-sizes/async-awaiting-fut.stdout @@ -2,38 +2,34 @@ print-type-size type: `[async fn body@$DIR/async-awaiting-fut.rs:21:21: 24:2]`: print-type-size discriminant: 1 bytes print-type-size variant `Unresumed`: 0 bytes print-type-size variant `Suspend0`: 3077 bytes -print-type-size local `.__awaitee`: 3077 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size local `.__awaitee`: 3077 bytes print-type-size variant `Returned`: 0 bytes print-type-size variant `Panicked`: 0 bytes print-type-size type: `[async fn body@$DIR/async-awaiting-fut.rs:10:64: 19:2]`: 3077 bytes, alignment: 1 bytes print-type-size discriminant: 1 bytes -print-type-size variant `Unresumed`: 2051 bytes -print-type-size padding: 1026 bytes -print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes +print-type-size variant `Unresumed`: 1025 bytes +print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes print-type-size variant `Suspend0`: 2052 bytes -print-type-size local `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes -print-type-size local `..generator_field4`: 1 bytes +print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes print-type-size padding: 1 bytes -print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes +print-type-size local `.fut`: 1025 bytes, alignment: 1 bytes +print-type-size local `..generator_field4`: 1 bytes print-type-size local `.__awaitee`: 1 bytes print-type-size variant `Suspend1`: 3076 bytes -print-type-size padding: 1024 bytes +print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size padding: 1026 bytes print-type-size local `..generator_field4`: 1 bytes, alignment: 1 bytes -print-type-size padding: 1 bytes -print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes print-type-size local `.__awaitee`: 1025 bytes print-type-size variant `Suspend2`: 2052 bytes -print-type-size local `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes -print-type-size local `..generator_field4`: 1 bytes +print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes print-type-size padding: 1 bytes -print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes +print-type-size local `.fut`: 1025 bytes, alignment: 1 bytes +print-type-size local `..generator_field4`: 1 bytes print-type-size local `.__awaitee`: 1 bytes -print-type-size variant `Returned`: 2051 bytes -print-type-size padding: 1026 bytes -print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes -print-type-size variant `Panicked`: 2051 bytes -print-type-size padding: 1026 bytes -print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes +print-type-size variant `Returned`: 1025 bytes +print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size variant `Panicked`: 1025 bytes +print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/async-awaiting-fut.rs:10:64: 19:2]>`: 3077 bytes, alignment: 1 bytes print-type-size field `.value`: 3077 bytes print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/async-awaiting-fut.rs:10:64: 19:2]>`: 3077 bytes, alignment: 1 bytes @@ -43,11 +39,11 @@ print-type-size field `.value`: 3077 bytes print-type-size type: `[async fn body@$DIR/async-awaiting-fut.rs:8:35: 8:37]`: 1025 bytes, alignment: 1 bytes print-type-size discriminant: 1 bytes print-type-size variant `Unresumed`: 1024 bytes -print-type-size upvar `.arg`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.arg`: 1024 bytes print-type-size variant `Returned`: 1024 bytes -print-type-size upvar `.arg`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.arg`: 1024 bytes print-type-size variant `Panicked`: 1024 bytes -print-type-size upvar `.arg`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.arg`: 1024 bytes print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/async-awaiting-fut.rs:8:35: 8:37]>`: 1025 bytes, alignment: 1 bytes print-type-size field `.value`: 1025 bytes print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/async-awaiting-fut.rs:8:35: 8:37]>`: 1025 bytes, alignment: 1 bytes diff --git a/tests/ui/async-await/future-sizes/large-arg.stdout b/tests/ui/async-await/future-sizes/large-arg.stdout index 91db4b1531f..b5e95ddd710 100644 --- a/tests/ui/async-await/future-sizes/large-arg.stdout +++ b/tests/ui/async-await/future-sizes/large-arg.stdout @@ -2,20 +2,20 @@ print-type-size type: `[async fn body@$DIR/large-arg.rs:6:21: 8:2]`: 3076 bytes, print-type-size discriminant: 1 bytes print-type-size variant `Unresumed`: 0 bytes print-type-size variant `Suspend0`: 3075 bytes -print-type-size local `.__awaitee`: 3075 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size local `.__awaitee`: 3075 bytes print-type-size variant `Returned`: 0 bytes print-type-size variant `Panicked`: 0 bytes print-type-size type: `[async fn body@$DIR/large-arg.rs:10:30: 12:2]`: 3075 bytes, alignment: 1 bytes print-type-size discriminant: 1 bytes print-type-size variant `Unresumed`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size variant `Suspend0`: 3074 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size local `.__awaitee`: 2050 bytes print-type-size variant `Returned`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size variant `Panicked`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/large-arg.rs:10:30: 12:2]>`: 3075 bytes, alignment: 1 bytes print-type-size field `.value`: 3075 bytes print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/large-arg.rs:10:30: 12:2]>`: 3075 bytes, alignment: 1 bytes @@ -25,14 +25,14 @@ print-type-size field `.value`: 3075 bytes print-type-size type: `[async fn body@$DIR/large-arg.rs:13:26: 15:2]`: 2050 bytes, alignment: 1 bytes print-type-size discriminant: 1 bytes print-type-size variant `Unresumed`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size variant `Suspend0`: 2049 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size local `.__awaitee`: 1025 bytes print-type-size variant `Returned`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size variant `Panicked`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/large-arg.rs:13:26: 15:2]>`: 2050 bytes, alignment: 1 bytes print-type-size field `.value`: 2050 bytes print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/large-arg.rs:13:26: 15:2]>`: 2050 bytes, alignment: 1 bytes @@ -42,11 +42,11 @@ print-type-size field `.value`: 2050 bytes print-type-size type: `[async fn body@$DIR/large-arg.rs:16:26: 18:2]`: 1025 bytes, alignment: 1 bytes print-type-size discriminant: 1 bytes print-type-size variant `Unresumed`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size variant `Returned`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size variant `Panicked`: 1024 bytes -print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.t`: 1024 bytes print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/large-arg.rs:16:26: 18:2]>`: 1025 bytes, alignment: 1 bytes print-type-size field `.value`: 1025 bytes print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/large-arg.rs:16:26: 18:2]>`: 1025 bytes, alignment: 1 bytes diff --git a/tests/ui/consts/const-eval/raw-bytes.32bit.stderr b/tests/ui/consts/const-eval/raw-bytes.32bit.stderr index a0f8dd097c7..a93b561e5be 100644 --- a/tests/ui/consts/const-eval/raw-bytes.32bit.stderr +++ b/tests/ui/consts/const-eval/raw-bytes.32bit.stderr @@ -465,7 +465,7 @@ LL | const LAYOUT_INVALID_ZERO: Layout = unsafe { Layout::from_size_align_unchec | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 8, align: 4) { - 00 10 00 00 00 00 00 00 │ ........ + 00 00 00 00 00 10 00 00 │ ........ } error[E0080]: it is undefined behavior to use this value @@ -476,7 +476,7 @@ LL | const LAYOUT_INVALID_THREE: Layout = unsafe { Layout::from_size_align_unche | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 8, align: 4) { - 09 00 00 00 03 00 00 00 │ ........ + 03 00 00 00 09 00 00 00 │ ........ } error[E0080]: it is undefined behavior to use this value diff --git a/tests/ui/consts/const-eval/raw-bytes.64bit.stderr b/tests/ui/consts/const-eval/raw-bytes.64bit.stderr index 9706f3ec2e0..a32d4863a38 100644 --- a/tests/ui/consts/const-eval/raw-bytes.64bit.stderr +++ b/tests/ui/consts/const-eval/raw-bytes.64bit.stderr @@ -465,7 +465,7 @@ LL | const LAYOUT_INVALID_ZERO: Layout = unsafe { Layout::from_size_align_unchec | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 16, align: 8) { - 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................ + 00 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00 │ ................ } error[E0080]: it is undefined behavior to use this value @@ -476,7 +476,7 @@ LL | const LAYOUT_INVALID_THREE: Layout = unsafe { Layout::from_size_align_unche | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 16, align: 8) { - 09 00 00 00 00 00 00 00 03 00 00 00 00 00 00 00 │ ................ + 03 00 00 00 00 00 00 00 09 00 00 00 00 00 00 00 │ ................ } error[E0080]: it is undefined behavior to use this value 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 20d4c418e87..8c7c915350f 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 @@ -370,12 +370,6 @@ error: layout_of(NicheFirst) = Layout { pref: $PREF_ALIGN, }, abi: ScalarPair( - Union { - value: Int( - I8, - false, - ), - }, Initialized { value: Int( I8, @@ -383,10 +377,16 @@ error: layout_of(NicheFirst) = Layout { ), valid_range: 0..=4, }, + Union { + value: Int( + I8, + false, + ), + }, ), fields: Arbitrary { offsets: [ - Size(1 bytes), + Size(0 bytes), ], memory_index: [ 0, @@ -394,7 +394,7 @@ error: layout_of(NicheFirst) = Layout { }, largest_niche: Some( Niche { - offset: Size(1 bytes), + offset: Size(0 bytes), value: Int( I8, false, @@ -429,29 +429,29 @@ error: layout_of(NicheFirst) = Layout { I8, false, ), - valid_range: 0..=255, + valid_range: 0..=2, }, Initialized { value: Int( I8, false, ), - valid_range: 0..=2, + valid_range: 0..=255, }, ), fields: Arbitrary { offsets: [ - Size(1 bytes), Size(0 bytes), + Size(1 bytes), ], memory_index: [ - 1, 0, + 1, ], }, largest_niche: Some( Niche { - offset: Size(1 bytes), + offset: Size(0 bytes), value: Int( I8, false, @@ -514,12 +514,6 @@ error: layout_of(NicheSecond) = Layout { pref: $PREF_ALIGN, }, abi: ScalarPair( - Union { - value: Int( - I8, - false, - ), - }, Initialized { value: Int( I8, @@ -527,10 +521,16 @@ error: layout_of(NicheSecond) = Layout { ), valid_range: 0..=4, }, + Union { + value: Int( + I8, + false, + ), + }, ), fields: Arbitrary { offsets: [ - Size(1 bytes), + Size(0 bytes), ], memory_index: [ 0, @@ -538,7 +538,7 @@ error: layout_of(NicheSecond) = Layout { }, largest_niche: Some( Niche { - offset: Size(1 bytes), + offset: Size(0 bytes), value: Int( I8, false, @@ -573,29 +573,29 @@ error: layout_of(NicheSecond) = Layout { I8, false, ), - valid_range: 0..=255, + valid_range: 0..=2, }, Initialized { value: Int( I8, false, ), - valid_range: 0..=2, + valid_range: 0..=255, }, ), fields: Arbitrary { offsets: [ - Size(0 bytes), Size(1 bytes), + Size(0 bytes), ], memory_index: [ - 0, 1, + 0, ], }, largest_niche: Some( Niche { - offset: Size(1 bytes), + offset: Size(0 bytes), value: Int( I8, false, diff --git a/tests/ui/print_type_sizes/async.stdout b/tests/ui/print_type_sizes/async.stdout index 8fe936efc89..1c6887412be 100644 --- a/tests/ui/print_type_sizes/async.stdout +++ b/tests/ui/print_type_sizes/async.stdout @@ -1,15 +1,15 @@ print-type-size type: `[async fn body@$DIR/async.rs:8:36: 11:2]`: 16386 bytes, alignment: 1 bytes print-type-size discriminant: 1 bytes print-type-size variant `Unresumed`: 8192 bytes -print-type-size upvar `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.arg`: 8192 bytes print-type-size variant `Suspend0`: 16385 bytes -print-type-size upvar `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.arg`: 8192 bytes print-type-size local `.arg`: 8192 bytes print-type-size local `.__awaitee`: 1 bytes print-type-size variant `Returned`: 8192 bytes -print-type-size upvar `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.arg`: 8192 bytes print-type-size variant `Panicked`: 8192 bytes -print-type-size upvar `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.arg`: 8192 bytes print-type-size type: `std::mem::ManuallyDrop<[u8; 8192]>`: 8192 bytes, alignment: 1 bytes print-type-size field `.value`: 8192 bytes print-type-size type: `std::mem::MaybeUninit<[u8; 8192]>`: 8192 bytes, alignment: 1 bytes diff --git a/tests/ui/print_type_sizes/generator.stdout b/tests/ui/print_type_sizes/generator.stdout index 7c58d6ce5ff..2dcadde9ec2 100644 --- a/tests/ui/print_type_sizes/generator.stdout +++ b/tests/ui/print_type_sizes/generator.stdout @@ -1,10 +1,10 @@ print-type-size type: `[generator@$DIR/generator.rs:10:5: 10:14]`: 8193 bytes, alignment: 1 bytes print-type-size discriminant: 1 bytes print-type-size variant `Unresumed`: 8192 bytes -print-type-size upvar `.array`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.array`: 8192 bytes print-type-size variant `Suspend0`: 8192 bytes -print-type-size upvar `.array`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.array`: 8192 bytes print-type-size variant `Returned`: 8192 bytes -print-type-size upvar `.array`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.array`: 8192 bytes print-type-size variant `Panicked`: 8192 bytes -print-type-size upvar `.array`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size upvar `.array`: 8192 bytes From 67a835d755a97770edb320d315d542859b11f854 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Wed, 5 Apr 2023 21:15:32 +0200 Subject: [PATCH 6/8] fix codegen test --- tests/codegen/issues/issue-103840.rs | 1 + tests/codegen/issues/issue-105386-ub-in-debuginfo.rs | 3 ++- tests/codegen/issues/issue-86106.rs | 5 +++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/codegen/issues/issue-103840.rs b/tests/codegen/issues/issue-103840.rs index f19d7031bb3..da64692d27d 100644 --- a/tests/codegen/issues/issue-103840.rs +++ b/tests/codegen/issues/issue-103840.rs @@ -1,4 +1,5 @@ // compile-flags: -O +// min-llvm-version: 16.0 #![crate_type = "lib"] pub fn foo(t: &mut Vec) { diff --git a/tests/codegen/issues/issue-105386-ub-in-debuginfo.rs b/tests/codegen/issues/issue-105386-ub-in-debuginfo.rs index d54ac9e33bc..2ee4d7cca0e 100644 --- a/tests/codegen/issues/issue-105386-ub-in-debuginfo.rs +++ b/tests/codegen/issues/issue-105386-ub-in-debuginfo.rs @@ -19,4 +19,5 @@ pub fn outer_function(x: S, y: S) -> usize { // CHECK-NOT: [[ptr_tmp:%.*]] = getelementptr inbounds %"[closure@{{.*.rs}}:9:23: 9:25]", ptr [[spill]] // CHECK-NOT: [[load:%.*]] = load ptr, ptr // CHECK: call void @llvm.lifetime.start{{.*}}({{.*}}, ptr [[spill]]) -// CHECK: call void @llvm.memcpy{{.*}}(ptr {{align .*}} [[spill]], ptr {{align .*}} %x +// CHECK: [[inner:%.*]] = getelementptr inbounds %"{{.*}}", ptr [[spill]] +// CHECK: call void @llvm.memcpy{{.*}}(ptr {{align .*}} [[inner]], ptr {{align .*}} %x diff --git a/tests/codegen/issues/issue-86106.rs b/tests/codegen/issues/issue-86106.rs index af621d79391..c0be7fab2f3 100644 --- a/tests/codegen/issues/issue-86106.rs +++ b/tests/codegen/issues/issue-86106.rs @@ -1,4 +1,5 @@ // min-llvm-version: 15.0 +// only-64bit llvm appears to use stores instead of memset on 32bit // compile-flags: -C opt-level=3 -Z merge-functions=disabled // The below two functions ensure that both `String::new()` and `"".to_string()` @@ -19,9 +20,9 @@ pub fn string_new() -> String { // CHECK-LABEL: define void @empty_to_string #[no_mangle] pub fn empty_to_string() -> String { - // CHECK: getelementptr + // CHECK: store ptr inttoptr + // CHECK-NEXT: getelementptr // CHECK-NEXT: call void @llvm.memset - // CHECK-NEXT: store ptr inttoptr // CHECK-NEXT: ret void "".to_string() } From afe106cdc8b5dbcbeedb292b87dc7d7ae58964f1 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Wed, 19 Apr 2023 22:14:28 +0200 Subject: [PATCH 7/8] [review] add comments, turn flag into enum --- compiler/rustc_abi/src/layout.rs | 67 +++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index f15fb877d51..a49c9e58297 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -50,7 +50,7 @@ pub trait LayoutCalculator { repr: &ReprOptions, kind: StructKind, ) -> Option { - let layout = univariant(self, dl, fields, repr, kind, true); + let layout = univariant(self, dl, fields, repr, kind, NicheBias::Start); // Enums prefer niches close to the beginning or the end of the variants so that other (smaller) // data-carrying variants can be packed into the space after/before the niche. // If the default field ordering does not give us a niche at the front then we do a second @@ -66,7 +66,7 @@ pub trait LayoutCalculator { // (e.g. a trailing bool) and there is tail padding. But it's non-trivial to get // the unpadded size so we try anyway. if fields.len() > 1 && head_space != 0 && tail_space > 0 { - let alt_layout = univariant(self, dl, fields, repr, kind, false) + let alt_layout = univariant(self, dl, fields, repr, kind, NicheBias::End) .expect("alt layout should always work"); let niche = alt_layout .largest_niche @@ -776,13 +776,19 @@ pub trait LayoutCalculator { } } +/// Determines towards which end of a struct layout optimizations will try to place the best niches. +enum NicheBias { + Start, + End, +} + fn univariant( this: &(impl LayoutCalculator + ?Sized), dl: &TargetDataLayout, fields: &IndexSlice>, repr: &ReprOptions, kind: StructKind, - niche_bias_start: bool, + niche_bias: NicheBias, ) -> Option { let pack = repr.pack; let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align }; @@ -809,7 +815,10 @@ fn univariant( } else { let max_field_align = fields.iter().map(|f| f.align().abi.bytes()).max().unwrap_or(1); let any_niche = fields.iter().any(|f| f.largest_niche().is_some()); - let effective_field_align = |layout: Layout<'_>| { + + // Calculates a sort key to group fields by their alignment or possibly some size-derived + // pseudo-alignment. + let alignment_group_key = |layout: Layout<'_>| { if let Some(pack) = pack { // return the packed alignment in bytes layout.align().abi.min(pack).bytes() @@ -818,10 +827,13 @@ fn univariant( // This is ok since `pack` applies to all fields equally. // The calculation assumes that size is an integer multiple of align, except for ZSTs. // - // group [u8; 4] with align-4 or [u8; 6] with align-2 fields let align = layout.align().abi.bytes(); let size = layout.size().bytes(); + // group [u8; 4] with align-4 or [u8; 6] with align-2 fields let size_as_align = align.max(size).trailing_zeros(); + // Given `A(u8, [u8; 16])` and `B(bool, [u8; 16])` we want to bump the array + // to the front in the first case (for aligned loads) but keep the bool in front + // in the second case for its niches. let size_as_align = if any_niche { max_field_align.trailing_zeros().min(size_as_align) } else { @@ -833,21 +845,29 @@ fn univariant( match kind { StructKind::AlwaysSized | StructKind::MaybeUnsized => { + // Currently `LayoutS` only exposes a single niche so sorting is usually sufficient + // to get one niche into the preferred position. If it ever supported multiple niches + // then a more advanced pick-and-pack approach could provide better results. + // But even for the single-niche cache it's not optimal. E.g. for + // A(u32, (bool, u8), u16) it would be possible to move the bool to the front + // but it would require packing the tuple together with the u16 to build a 4-byte + // group so that the u32 can be placed after it without padding. This kind + // of packing can't be achieved by sorting. optimizing.sort_by_key(|&x| { let f = fields[x]; let field_size = f.size().bytes(); let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); - let niche_size = if niche_bias_start { - u128::MAX - niche_size // large niche first - } else { - niche_size // large niche last + let niche_size_key = match niche_bias { + // large niche first + NicheBias::Start => !niche_size, + // large niche last + NicheBias::End => niche_size, }; - let inner_niche_placement = if niche_bias_start { - f.largest_niche().map_or(0, |n| n.offset.bytes()) - } else { - f.largest_niche().map_or(0, |n| { - field_size - n.value.size(dl).bytes() - n.offset.bytes() - }) + let inner_niche_offset_key = match niche_bias { + NicheBias::Start => f.largest_niche().map_or(0, |n| n.offset.bytes()), + NicheBias::End => f.largest_niche().map_or(0, |n| { + !(field_size - n.value.size(dl).bytes() - n.offset.bytes()) + }), }; ( @@ -855,13 +875,13 @@ fn univariant( // or two non-ZST fields. This helps Scalar/ScalarPair layouts. !f.0.is_zst(), // Then place largest alignments first. - cmp::Reverse(effective_field_align(f)), + cmp::Reverse(alignment_group_key(f)), // Then prioritize niche placement within alignment group according to // `niche_bias_start`. - niche_size, + niche_size_key, // Then among fields with equally-sized niches prefer the ones // closer to the start/end of the field. - inner_niche_placement, + inner_niche_offset_key, ) }); } @@ -874,7 +894,7 @@ fn univariant( optimizing.sort_by_key(|&x| { let f = fields[x]; let niche_size = f.largest_niche().map_or(0, |n| n.available(dl)); - (effective_field_align(f), niche_size) + (alignment_group_key(f), niche_size) }); } } @@ -927,10 +947,11 @@ fn univariant( if let Some(mut niche) = field.largest_niche() { let available = niche.available(dl); - let prefer_new_niche = if niche_bias_start { - available > largest_niche_available - } else { - available >= largest_niche_available + // Pick up larger niches. + let prefer_new_niche = match niche_bias { + NicheBias::Start => available > largest_niche_available, + // if there are several niches of the same size then pick the last one + NicheBias::End => available >= largest_niche_available, }; if prefer_new_niche { largest_niche_available = available; From 61fb5a91b794d7ab9c5f923b26c92cfc473b976b Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sat, 22 Apr 2023 19:24:39 +0200 Subject: [PATCH 8/8] layout-alignment-promotion logic should depend on the niche-bias For start-biased layout we want to avoid overpromoting so that the niche doesn't get pushed back. For end-biased layout we want to avoid promoting fields that may contain one of the niches of interest. --- compiler/rustc_abi/src/layout.rs | 29 +++++++++++++++++++++------- tests/ui/structs-enums/type-sizes.rs | 19 ++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index a49c9e58297..b4597d5bc78 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -794,7 +794,7 @@ fn univariant( let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align }; let mut inverse_memory_index: IndexVec = fields.indices().collect(); let optimize = !repr.inhibit_struct_field_reordering_opt(); - if optimize { + if optimize && fields.len() > 1 { let end = if let StructKind::MaybeUnsized = kind { fields.len() - 1 } else { fields.len() }; let optimizing = &mut inverse_memory_index.raw[..end]; @@ -814,7 +814,12 @@ fn univariant( // Otherwise we just leave things alone and actually optimize the type's fields } else { let max_field_align = fields.iter().map(|f| f.align().abi.bytes()).max().unwrap_or(1); - let any_niche = fields.iter().any(|f| f.largest_niche().is_some()); + let largest_niche_size = fields + .iter() + .filter_map(|f| f.largest_niche()) + .map(|n| n.available(dl)) + .max() + .unwrap_or(0); // Calculates a sort key to group fields by their alignment or possibly some size-derived // pseudo-alignment. @@ -829,13 +834,23 @@ fn univariant( // let align = layout.align().abi.bytes(); let size = layout.size().bytes(); + let niche_size = layout.largest_niche().map(|n| n.available(dl)).unwrap_or(0); // group [u8; 4] with align-4 or [u8; 6] with align-2 fields let size_as_align = align.max(size).trailing_zeros(); - // Given `A(u8, [u8; 16])` and `B(bool, [u8; 16])` we want to bump the array - // to the front in the first case (for aligned loads) but keep the bool in front - // in the second case for its niches. - let size_as_align = if any_niche { - max_field_align.trailing_zeros().min(size_as_align) + let size_as_align = if largest_niche_size > 0 { + match niche_bias { + // Given `A(u8, [u8; 16])` and `B(bool, [u8; 16])` we want to bump the array + // to the front in the first case (for aligned loads) but keep the bool in front + // in the second case for its niches. + NicheBias::Start => max_field_align.trailing_zeros().min(size_as_align), + // When moving niches towards the end of the struct then for + // A((u8, u8, u8, bool), (u8, bool, u8)) we want to keep the first tuple + // in the align-1 group because its bool can be moved closer to the end. + NicheBias::End if niche_size == largest_niche_size => { + align.trailing_zeros() + } + NicheBias::End => size_as_align, + } } else { size_as_align }; diff --git a/tests/ui/structs-enums/type-sizes.rs b/tests/ui/structs-enums/type-sizes.rs index e5c6857a26a..406e5c8441e 100644 --- a/tests/ui/structs-enums/type-sizes.rs +++ b/tests/ui/structs-enums/type-sizes.rs @@ -198,6 +198,18 @@ struct ReorderWithNiche { ary: [u8; 8] } +#[repr(C)] +struct EndNiche8([u8; 7], bool); + +#[repr(C)] +struct MiddleNiche4(u8, u8, bool, u8); + +struct ReorderEndNiche { + a: EndNiche8, + b: MiddleNiche4, +} + + // standins for std types which we want to be laid out in a reasonable way struct RawVecDummy { ptr: NonNull, @@ -316,4 +328,11 @@ pub fn main() { "here [u8; 8] should group with _at least_ align-4 fields"); assert_eq!(ptr::from_ref(&v), ptr::from_ref(&v.b).cast(), "sort niches to the front where possible"); + + // Neither field has a niche at the beginning so the layout algorithm should try move niches to + // the end which means the 8-sized field shouldn't be alignment-promoted before the 4-sized one. + let v = ReorderEndNiche { a: EndNiche8([0; 7], false), b: MiddleNiche4(0, 0, false, 0) }; + assert!(ptr::from_ref(&v.a).addr() > ptr::from_ref(&v.b).addr()); + + }