diff options
| author | bors <bors@rust-lang.org> | 2022-11-23 10:01:48 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-11-23 10:01:48 +0000 |
| commit | 4e0d0d757e2f1b61ec809420b006545a9f8974c0 (patch) | |
| tree | 2bc6f5a32e16c8b7dd42ba0b3f963da53fceed23 | |
| parent | 3f2b2eee8f46f2252d2919d7a57bf3068d7df285 (diff) | |
| parent | c1f392dbc09d9fef086f84bbcbecebc0cde8df81 (diff) | |
| download | rust-4e0d0d757e2f1b61ec809420b006545a9f8974c0.tar.gz rust-4e0d0d757e2f1b61ec809420b006545a9f8974c0.zip | |
Auto merge of #102750 - the8472:opt-field-order, r=wesleywiser
optimize field ordering by grouping m*2^n-sized fields with equivalently aligned ones
```rust
use std::ptr::addr_of;
use std::mem;
struct Foo {
word: u32,
byte: u8,
ary: [u8; 4]
}
fn main() {
let foo: Foo = unsafe { mem::zeroed() };
println!("base: {:p}\nword: {:p}\nbyte: {:p}\nary: {:p}", &foo, addr_of!(foo.word), addr_of!(foo.byte), addr_of!(foo.ary));
}
```
prints
```
base: 0x7fffc1a8a668
word: 0x7fffc1a8a668
byte: 0x7fffc1a8a66c
ary: 0x7fffc1a8a66d
```
I.e. the `u8` in the middle causes the array to sit at an odd offset, which might prevent optimizations, especially on architectures where unaligned loads are costly.
Note that this will make field ordering niche-dependent, i.e. a `Bar<T>` with `T=char` and `T=u32` may result in different field order, this may break some code that makes invalid assumptions about `repr(Rust)` types.
| -rw-r--r-- | compiler/rustc_hir/src/hir.rs | 9 | ||||
| -rw-r--r-- | compiler/rustc_ty_utils/src/layout.rs | 28 | ||||
| -rw-r--r-- | src/test/codegen/issue-37945.rs | 4 | ||||
| -rw-r--r-- | src/test/codegen/mem-replace-direct-memcpy.rs | 4 | ||||
| -rw-r--r-- | src/test/codegen/slice-iter-len-eq-zero.rs | 2 | ||||
| -rw-r--r-- | src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr | 20 | ||||
| -rw-r--r-- | src/test/ui/stats/hir-stats.rs | 1 | ||||
| -rw-r--r-- | src/test/ui/stats/hir-stats.stderr | 20 | ||||
| -rw-r--r-- | src/test/ui/structs-enums/type-sizes.rs | 21 |
9 files changed, 78 insertions, 31 deletions
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index bd5b93293a9..473a04f33a9 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/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 92e8542795f..07af3dc5164 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -138,8 +138,18 @@ 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 { + // 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 + } }; // If `-Z randomize-layout` was enabled for the type definition we can shuffle @@ -160,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(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| 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) + }); } } diff --git a/src/test/codegen/issue-37945.rs b/src/test/codegen/issue-37945.rs index 12fa1e9e56b..fe54375bbf6 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<f32>) -> 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<f32>) -> bool { pub fn is_empty_2(xs: Iter<f32>) -> 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 4318e926e47..e8bbf0e1bbd 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 1124028253d..894b0ec3de4 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 bfabe2d12f7..20d4c418e87 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 a24b3ada57e..0b89d0b160b 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 297245f0198..012bc848d4b 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 diff --git a/src/test/ui/structs-enums/type-sizes.rs b/src/test/ui/structs-enums/type-sizes.rs index 7a23f13630a..63e2f3150c0 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<X> { _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::<u8>(), 1 as usize); assert_eq!(size_of::<u32>(), 4 as usize); @@ -249,4 +262,12 @@ pub fn main() { assert_eq!(size_of::<EnumManyVariant<Option<NicheU16>>>(), 4); assert_eq!(size_of::<EnumManyVariant<Option2<NicheU16,u8>>>(), 6); assert_eq!(size_of::<EnumManyVariant<Option<(NicheU16,u8)>>>(), 6); + + + let v = Reorder4 {a: 0, b: 0, ary: [0; 4]}; + assert_eq!(size_of::<Reorder4>(), 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::<Reorder2>(), 10); + assert!((&v.ary).as_ptr().is_aligned_to(2), "[u8; 6] should group with align-2 fields"); } |
