From 9f0cb566ea2f81392de6fb49b09c7e9b4ef4bf1d Mon Sep 17 00:00:00 2001 From: The 8472 Date: Thu, 6 Oct 2022 21:20:02 +0200 Subject: [PATCH 1/5] optimize field ordering by grouping power-of-two arrays with larger types --- compiler/rustc_ty_utils/src/layout.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 92e8542795fae..1505ce41b9630 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -138,8 +138,17 @@ fn univariant_uninterned<'tcx>( if optimize { let end = if let StructKind::MaybeUnsized = kind { fields.len() - 1 } else { fields.len() }; let optimizing = &mut inverse_memory_index[..end]; - let field_align = |f: &TyAndLayout<'_>| { - if let Some(pack) = pack { f.align.abi.min(pack) } else { f.align.abi } + let effective_field_align = |f: &TyAndLayout<'_>| { + if let Some(pack) = pack { + f.align.abi.min(pack) + } else if f.size.bytes().is_power_of_two() && f.size.bytes() >= f.align.abi.bytes() { + // Try to put fields which have a 2^n size and smaller alignment together with + // fields that have an alignment matching that size. + // E.g. group [u8; 4] with u32 fields + Align::from_bytes(f.align.abi.bytes()).unwrap_or(f.align.abi) + } else { + f.align.abi + } }; // If `-Z randomize-layout` was enabled for the type definition we can shuffle @@ -161,14 +170,14 @@ fn univariant_uninterned<'tcx>( // Place ZSTs first to avoid "interesting offsets", // especially with only one or two non-ZST fields. let f = &fields[x as usize]; - (!f.is_zst(), cmp::Reverse(field_align(f))) + (!f.is_zst(), cmp::Reverse(effective_field_align(f))) }); } StructKind::Prefixed(..) => { // Sort in ascending alignment so that the layout stays optimal // regardless of the prefix - optimizing.sort_by_key(|&x| field_align(&fields[x as usize])); + optimizing.sort_by_key(|&x| effective_field_align(&fields[x as usize])); } } From a3450d060d04b07c09775483b4d4bb7597429e5a Mon Sep 17 00:00:00 2001 From: The 8472 Date: Thu, 6 Oct 2022 23:34:50 +0200 Subject: [PATCH 2/5] group fields based on largest power of two dividing its size --- compiler/rustc_ty_utils/src/layout.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 1505ce41b9630..cf7a74dee9901 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -140,14 +140,10 @@ fn univariant_uninterned<'tcx>( let optimizing = &mut inverse_memory_index[..end]; let effective_field_align = |f: &TyAndLayout<'_>| { if let Some(pack) = pack { - f.align.abi.min(pack) - } else if f.size.bytes().is_power_of_two() && f.size.bytes() >= f.align.abi.bytes() { - // Try to put fields which have a 2^n size and smaller alignment together with - // fields that have an alignment matching that size. - // E.g. group [u8; 4] with u32 fields - Align::from_bytes(f.align.abi.bytes()).unwrap_or(f.align.abi) + f.align.abi.min(pack).bytes() } else { - f.align.abi + // group [u8; 4] with align-4 or [u8; 6] with align-2 fields + f.align.abi.bytes().max(f.size.bytes()).trailing_zeros() as u64 } }; From 97d8a9bdd3b364406577d7368f5c5203a0f9740a Mon Sep 17 00:00:00 2001 From: The 8472 Date: Fri, 7 Oct 2022 01:09:54 +0200 Subject: [PATCH 3/5] also sort fields by niche sizes to retain optimizations --- compiler/rustc_ty_utils/src/layout.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index cf7a74dee9901..07af3dc516478 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -140,8 +140,13 @@ fn univariant_uninterned<'tcx>( let optimizing = &mut inverse_memory_index[..end]; let effective_field_align = |f: &TyAndLayout<'_>| { if let Some(pack) = pack { + // return the packed alignment in bytes f.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 f.align.abi.bytes().max(f.size.bytes()).trailing_zeros() as u64 } @@ -165,15 +170,23 @@ fn univariant_uninterned<'tcx>( 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 as usize]; - (!f.is_zst(), cmp::Reverse(effective_field_align(f))) + let niche_size = f.largest_niche.map_or(0, |n| n.available(cx)); + (!f.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 - optimizing.sort_by_key(|&x| effective_field_align(&fields[x as usize])); + // 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 as usize]; + let niche_size = f.largest_niche.map_or(0, |n| n.available(cx)); + (effective_field_align(f), niche_size) + }); } } From a9128d89273be6f7b2bc58cd5b8bcd74382c7cd7 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Fri, 7 Oct 2022 01:10:15 +0200 Subject: [PATCH 4/5] fix tests, update size asserts --- compiler/rustc_hir/src/hir.rs | 9 ++++++++- src/test/codegen/issue-37945.rs | 4 ++-- src/test/codegen/mem-replace-direct-memcpy.rs | 4 ++-- src/test/codegen/slice-iter-len-eq-zero.rs | 2 +- ...-scalarpair-payload-might-be-uninit.stderr | 20 +++++++++---------- src/test/ui/stats/hir-stats.rs | 1 + src/test/ui/stats/hir-stats.stderr | 20 +++++++++---------- 7 files changed, 34 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index bd5b93293a9b6..473a04f33a9ad 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -3594,9 +3594,16 @@ mod size_asserts { static_assert_size!(Res, 12); static_assert_size!(Stmt<'_>, 32); static_assert_size!(StmtKind<'_>, 16); + // tidy-alphabetical-end + // FIXME: move the tidy directive to the end after the next bootstrap bump + #[cfg(bootstrap)] static_assert_size!(TraitItem<'_>, 88); + #[cfg(not(bootstrap))] + static_assert_size!(TraitItem<'_>, 80); + #[cfg(bootstrap)] static_assert_size!(TraitItemKind<'_>, 48); + #[cfg(not(bootstrap))] + static_assert_size!(TraitItemKind<'_>, 40); static_assert_size!(Ty<'_>, 48); static_assert_size!(TyKind<'_>, 32); - // tidy-alphabetical-end } diff --git a/src/test/codegen/issue-37945.rs b/src/test/codegen/issue-37945.rs index 12fa1e9e56b6a..fe54375bbf6aa 100644 --- a/src/test/codegen/issue-37945.rs +++ b/src/test/codegen/issue-37945.rs @@ -15,7 +15,7 @@ use std::slice::Iter; pub fn is_empty_1(xs: Iter) -> bool { // CHECK-LABEL: @is_empty_1( // CHECK-NEXT: start: -// CHECK-NEXT: [[A:%.*]] = icmp ne {{i32\*|ptr}} %xs.1, null +// CHECK-NEXT: [[A:%.*]] = icmp ne {{i32\*|ptr}} {{%xs.0|%xs.1}}, null // CHECK-NEXT: tail call void @llvm.assume(i1 [[A]]) // The order between %xs.0 and %xs.1 on the next line doesn't matter // and different LLVM versions produce different order. @@ -28,7 +28,7 @@ pub fn is_empty_1(xs: Iter) -> bool { pub fn is_empty_2(xs: Iter) -> bool { // CHECK-LABEL: @is_empty_2 // CHECK-NEXT: start: -// CHECK-NEXT: [[C:%.*]] = icmp ne {{i32\*|ptr}} %xs.1, null +// CHECK-NEXT: [[C:%.*]] = icmp ne {{i32\*|ptr}} {{%xs.0|%xs.1}}, null // CHECK-NEXT: tail call void @llvm.assume(i1 [[C]]) // The order between %xs.0 and %xs.1 on the next line doesn't matter // and different LLVM versions produce different order. diff --git a/src/test/codegen/mem-replace-direct-memcpy.rs b/src/test/codegen/mem-replace-direct-memcpy.rs index 4318e926e4791..e8bbf0e1bbd61 100644 --- a/src/test/codegen/mem-replace-direct-memcpy.rs +++ b/src/test/codegen/mem-replace-direct-memcpy.rs @@ -18,7 +18,7 @@ pub fn replace_byte(dst: &mut u8, src: u8) -> u8 { // CHECK-NOT: call void @llvm.memcpy // CHECK: ; core::mem::replace // CHECK-NOT: call void @llvm.memcpy -// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %dest, i{{.*}} 1, i1 false) +// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %{{.*}}, i{{.*}} 1, i1 false) // CHECK-NOT: call void @llvm.memcpy -// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %dest, {{i8\*|ptr}} align 1 %src{{.*}}, i{{.*}} 1, i1 false) +// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %{{.*}}, i{{.*}} 1, i1 false) // CHECK-NOT: call void @llvm.memcpy diff --git a/src/test/codegen/slice-iter-len-eq-zero.rs b/src/test/codegen/slice-iter-len-eq-zero.rs index 1124028253ded..894b0ec3de4f3 100644 --- a/src/test/codegen/slice-iter-len-eq-zero.rs +++ b/src/test/codegen/slice-iter-len-eq-zero.rs @@ -9,7 +9,7 @@ type Demo = [u8; 3]; #[no_mangle] pub fn slice_iter_len_eq_zero(y: std::slice::Iter<'_, Demo>) -> bool { // CHECK-NOT: sub - // CHECK: %2 = icmp eq {{i8\*|ptr}} %1, %0 + // CHECK: %2 = icmp eq {{i8\*|ptr}} {{%1|%0}}, {{%1|%0}} // CHECK: ret i1 %2 y.len() == 0 } diff --git a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr index bfabe2d12f7ff..20d4c418e879c 100644 --- a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr +++ b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr @@ -370,23 +370,23 @@ error: layout_of(NicheFirst) = Layout { pref: $PREF_ALIGN, }, abi: ScalarPair( - Initialized { + Union { value: Int( I8, false, ), - valid_range: 0..=4, }, - Union { + Initialized { value: Int( I8, false, ), + valid_range: 0..=4, }, ), fields: Arbitrary { offsets: [ - Size(0 bytes), + Size(1 bytes), ], memory_index: [ 0, @@ -394,7 +394,7 @@ error: layout_of(NicheFirst) = Layout { }, largest_niche: Some( Niche { - offset: Size(0 bytes), + offset: Size(1 bytes), value: Int( I8, false, @@ -429,29 +429,29 @@ error: layout_of(NicheFirst) = Layout { I8, false, ), - valid_range: 0..=2, + valid_range: 0..=255, }, Initialized { value: Int( I8, false, ), - valid_range: 0..=255, + valid_range: 0..=2, }, ), fields: Arbitrary { offsets: [ - Size(0 bytes), Size(1 bytes), + Size(0 bytes), ], memory_index: [ - 0, 1, + 0, ], }, largest_niche: Some( Niche { - offset: Size(0 bytes), + offset: Size(1 bytes), value: Int( I8, false, diff --git a/src/test/ui/stats/hir-stats.rs b/src/test/ui/stats/hir-stats.rs index a24b3ada57e59..0b89d0b160b3d 100644 --- a/src/test/ui/stats/hir-stats.rs +++ b/src/test/ui/stats/hir-stats.rs @@ -1,6 +1,7 @@ // check-pass // compile-flags: -Zhir-stats // only-x86_64 +// ignore-stage1 FIXME: remove after next bootstrap bump // The aim here is to include at least one of every different type of top-level // AST/HIR node reported by `-Zhir-stats`. diff --git a/src/test/ui/stats/hir-stats.stderr b/src/test/ui/stats/hir-stats.stderr index 297245f0198b5..012bc848d4bca 100644 --- a/src/test/ui/stats/hir-stats.stderr +++ b/src/test/ui/stats/hir-stats.stderr @@ -2,12 +2,12 @@ ast-stats-1 PRE EXPANSION AST STATS ast-stats-1 Name Accumulated Size Count Item Size ast-stats-1 ---------------------------------------------------------------- ast-stats-1 ExprField 48 ( 0.6%) 1 48 +ast-stats-1 GenericArgs 56 ( 0.8%) 1 56 +ast-stats-1 - AngleBracketed 56 ( 0.8%) 1 ast-stats-1 Crate 56 ( 0.8%) 1 56 ast-stats-1 Attribute 64 ( 0.9%) 2 32 ast-stats-1 - Normal 32 ( 0.4%) 1 ast-stats-1 - DocComment 32 ( 0.4%) 1 -ast-stats-1 GenericArgs 64 ( 0.9%) 1 64 -ast-stats-1 - AngleBracketed 64 ( 0.9%) 1 ast-stats-1 Local 72 ( 1.0%) 1 72 ast-stats-1 WherePredicate 72 ( 1.0%) 1 72 ast-stats-1 - BoundPredicate 72 ( 1.0%) 1 @@ -53,15 +53,15 @@ ast-stats-1 - Impl 184 ( 2.5%) 1 ast-stats-1 - Fn 368 ( 5.0%) 2 ast-stats-1 - Use 552 ( 7.4%) 3 ast-stats-1 ---------------------------------------------------------------- -ast-stats-1 Total 7_424 +ast-stats-1 Total 7_416 ast-stats-1 ast-stats-2 POST EXPANSION AST STATS ast-stats-2 Name Accumulated Size Count Item Size ast-stats-2 ---------------------------------------------------------------- ast-stats-2 ExprField 48 ( 0.6%) 1 48 +ast-stats-2 GenericArgs 56 ( 0.7%) 1 56 +ast-stats-2 - AngleBracketed 56 ( 0.7%) 1 ast-stats-2 Crate 56 ( 0.7%) 1 56 -ast-stats-2 GenericArgs 64 ( 0.8%) 1 64 -ast-stats-2 - AngleBracketed 64 ( 0.8%) 1 ast-stats-2 Local 72 ( 0.9%) 1 72 ast-stats-2 WherePredicate 72 ( 0.9%) 1 72 ast-stats-2 - BoundPredicate 72 ( 0.9%) 1 @@ -80,9 +80,9 @@ ast-stats-2 - Expr 96 ( 1.2%) 3 ast-stats-2 Param 160 ( 2.0%) 4 40 ast-stats-2 FnDecl 200 ( 2.5%) 5 40 ast-stats-2 Variant 240 ( 3.0%) 2 120 -ast-stats-2 GenericBound 288 ( 3.5%) 4 72 -ast-stats-2 - Trait 288 ( 3.5%) 4 -ast-stats-2 Block 288 ( 3.5%) 6 48 +ast-stats-2 GenericBound 288 ( 3.6%) 4 72 +ast-stats-2 - Trait 288 ( 3.6%) 4 +ast-stats-2 Block 288 ( 3.6%) 6 48 ast-stats-2 AssocItem 416 ( 5.1%) 4 104 ast-stats-2 - Type 208 ( 2.6%) 2 ast-stats-2 - Fn 208 ( 2.6%) 2 @@ -104,7 +104,7 @@ ast-stats-2 - Rptr 64 ( 0.8%) 1 ast-stats-2 - Ptr 64 ( 0.8%) 1 ast-stats-2 - ImplicitSelf 128 ( 1.6%) 2 ast-stats-2 - Path 640 ( 7.9%) 10 -ast-stats-2 Item 2_024 (24.9%) 11 184 +ast-stats-2 Item 2_024 (25.0%) 11 184 ast-stats-2 - Trait 184 ( 2.3%) 1 ast-stats-2 - Enum 184 ( 2.3%) 1 ast-stats-2 - ExternCrate 184 ( 2.3%) 1 @@ -113,7 +113,7 @@ ast-stats-2 - Impl 184 ( 2.3%) 1 ast-stats-2 - Fn 368 ( 4.5%) 2 ast-stats-2 - Use 736 ( 9.1%) 4 ast-stats-2 ---------------------------------------------------------------- -ast-stats-2 Total 8_120 +ast-stats-2 Total 8_112 ast-stats-2 hir-stats HIR STATS hir-stats Name Accumulated Size Count Item Size From c1f392dbc09d9fef086f84bbcbecebc0cde8df81 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Fri, 7 Oct 2022 20:35:34 +0200 Subject: [PATCH 5/5] add tests for field ordering optimization --- src/test/ui/structs-enums/type-sizes.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/test/ui/structs-enums/type-sizes.rs b/src/test/ui/structs-enums/type-sizes.rs index 7a23f13630a43..63e2f3150c013 100644 --- a/src/test/ui/structs-enums/type-sizes.rs +++ b/src/test/ui/structs-enums/type-sizes.rs @@ -3,6 +3,7 @@ #![allow(non_camel_case_types)] #![allow(dead_code)] #![feature(never_type)] +#![feature(pointer_is_aligned)] use std::mem::size_of; use std::num::NonZeroU8; @@ -168,6 +169,18 @@ pub enum EnumManyVariant { _F0, _F1, _F2, _F3, _F4, _F5, _F6, _F7, _F8, _F9, _FA, _FB, _FC, _FD, _FE, _FF, } +struct Reorder4 { + a: u32, + b: u8, + ary: [u8; 4], +} + +struct Reorder2 { + a: u16, + b: u8, + ary: [u8; 6], +} + pub fn main() { assert_eq!(size_of::(), 1 as usize); assert_eq!(size_of::(), 4 as usize); @@ -249,4 +262,12 @@ pub fn main() { assert_eq!(size_of::>>(), 4); assert_eq!(size_of::>>(), 6); assert_eq!(size_of::>>(), 6); + + + let v = Reorder4 {a: 0, b: 0, ary: [0; 4]}; + assert_eq!(size_of::(), 12); + assert!((&v.ary).as_ptr().is_aligned_to(4), "[u8; 4] should group with align-4 fields"); + 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"); }