From 8dc9d4d567dae3481a47ad677e263cb3bd57cdf5 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sun, 12 Apr 2020 00:39:43 +0300 Subject: [PATCH 1/3] [experiment] ty/layout: compute both niche-filling and tagged layouts for enums. --- src/librustc_middle/ty/layout.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/librustc_middle/ty/layout.rs b/src/librustc_middle/ty/layout.rs index 215f44819b5d1..010b29a060b6f 100644 --- a/src/librustc_middle/ty/layout.rs +++ b/src/librustc_middle/ty/layout.rs @@ -876,6 +876,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { .iter_enumerated() .all(|(i, v)| v.discr == ty::VariantDiscr::Relative(i.as_u32())); + let mut niche_filling_layout = None; + // Niche-filling enum optimization. if !def.repr.inhibit_enum_layout_opt() && no_explicit_discriminants { let mut dataful_variant = None; @@ -972,7 +974,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let largest_niche = Niche::from_scalar(dl, offset, niche_scalar.clone()); - return Ok(tcx.intern_layout(Layout { + niche_filling_layout = Some(Layout { variants: Variants::Multiple { tag: niche_scalar, tag_encoding: TagEncoding::Niche { @@ -991,7 +993,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { largest_niche, size, align, - })); + }); } } } @@ -1214,7 +1216,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let largest_niche = Niche::from_scalar(dl, Size::ZERO, tag.clone()); - tcx.intern_layout(Layout { + let tagged_layout = Layout { variants: Variants::Multiple { tag, tag_encoding: TagEncoding::Direct, @@ -1229,7 +1231,9 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { abi, align, size, - }) + }; + + tcx.intern_layout(niche_filling_layout.unwrap_or(tagged_layout)) } // Types with no meaningful known layout. From b34d3004914a1b6b2e7c91b2c41ac1af28745874 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Sun, 5 Jul 2020 14:29:30 -0400 Subject: [PATCH 2/3] compare tagged/niche-filling layout and pick the best one --- src/librustc_middle/lib.rs | 1 + src/librustc_middle/ty/layout.rs | 16 +++++++++++++++- .../ui/print_type_sizes/niche-filling.stdout | 6 +++--- src/test/ui/type-sizes.rs | 17 +++++++++++++++++ 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/librustc_middle/lib.rs b/src/librustc_middle/lib.rs index b7dccb8d8ce6d..a68301385b7a5 100644 --- a/src/librustc_middle/lib.rs +++ b/src/librustc_middle/lib.rs @@ -27,6 +27,7 @@ #![feature(bool_to_option)] #![feature(box_patterns)] #![feature(box_syntax)] +#![feature(cmp_min_max_by)] #![feature(const_fn)] #![feature(const_panic)] #![feature(const_fn_transmute)] diff --git a/src/librustc_middle/ty/layout.rs b/src/librustc_middle/ty/layout.rs index 010b29a060b6f..dc775b15927fa 100644 --- a/src/librustc_middle/ty/layout.rs +++ b/src/librustc_middle/ty/layout.rs @@ -1233,7 +1233,21 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { size, }; - tcx.intern_layout(niche_filling_layout.unwrap_or(tagged_layout)) + let best_layout = match (tagged_layout, niche_filling_layout) { + (tagged_layout, Some(niche_filling_layout)) => { + // Pick the smaller layout; otherwise, + // pick the layout with the larger niche; otherwise, + // pick tagged as it has simpler codegen. + cmp::min_by_key(tagged_layout, niche_filling_layout, |layout| { + let niche_size = + layout.largest_niche.as_ref().map_or(0, |n| n.available(dl)); + (layout.size, cmp::Reverse(niche_size)) + }) + } + (tagged_layout, None) => tagged_layout, + }; + + tcx.intern_layout(best_layout) } // Types with no meaningful known layout. diff --git a/src/test/ui/print_type_sizes/niche-filling.stdout b/src/test/ui/print_type_sizes/niche-filling.stdout index 301edc0d086b1..1894cd218ee34 100644 --- a/src/test/ui/print_type_sizes/niche-filling.stdout +++ b/src/test/ui/print_type_sizes/niche-filling.stdout @@ -8,12 +8,12 @@ print-type-size variant `Some`: 12 bytes print-type-size field `.0`: 12 bytes print-type-size variant `None`: 0 bytes print-type-size type: `EmbeddedDiscr`: 8 bytes, alignment: 4 bytes +print-type-size discriminant: 1 bytes print-type-size variant `Record`: 7 bytes -print-type-size field `.val`: 4 bytes -print-type-size field `.post`: 2 bytes print-type-size field `.pre`: 1 bytes +print-type-size field `.post`: 2 bytes +print-type-size field `.val`: 4 bytes print-type-size variant `None`: 0 bytes -print-type-size end padding: 1 bytes print-type-size type: `MyOption>`: 8 bytes, alignment: 4 bytes print-type-size discriminant: 4 bytes print-type-size variant `Some`: 4 bytes diff --git a/src/test/ui/type-sizes.rs b/src/test/ui/type-sizes.rs index 6a3f3c98f127a..0ae9bfcf0bd5a 100644 --- a/src/test/ui/type-sizes.rs +++ b/src/test/ui/type-sizes.rs @@ -5,6 +5,7 @@ #![feature(never_type)] use std::mem::size_of; +use std::num::NonZeroU8; struct t {a: u8, b: i8} struct u {a: u8, b: i8, c: u8} @@ -102,6 +103,15 @@ enum Option2 { None } +pub enum CanBeNicheFilledButShouldnt { + A(NonZeroU8, u32), + B +} +pub enum AlwaysTagged { + A(u8, u32), + B +} + pub fn main() { assert_eq!(size_of::(), 1 as usize); assert_eq!(size_of::(), 4 as usize); @@ -145,4 +155,11 @@ pub fn main() { assert_eq!(size_of::>>(), size_of::<(bool, &())>()); assert_eq!(size_of::>>(), size_of::<(bool, &())>()); assert_eq!(size_of::>>(), size_of::<(bool, &())>()); + + assert_eq!(size_of::(), 8); + assert_eq!(size_of::>(), 8); + assert_eq!(size_of::>>(), 8); + assert_eq!(size_of::(), 8); + assert_eq!(size_of::>(), 8); + assert_eq!(size_of::>>(), 8); } From 144b1592e8186ab7c24212058bb2c897260897e3 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Thu, 16 Jul 2020 18:54:26 -0400 Subject: [PATCH 3/3] document test changes --- src/test/ui/type-sizes.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/test/ui/type-sizes.rs b/src/test/ui/type-sizes.rs index 0ae9bfcf0bd5a..73a11a5e743f6 100644 --- a/src/test/ui/type-sizes.rs +++ b/src/test/ui/type-sizes.rs @@ -103,11 +103,19 @@ enum Option2 { None } +// Two layouts are considered for `CanBeNicheFilledButShouldnt`: +// Niche-filling: +// { u32 (4 bytes), NonZeroU8 + tag in niche (1 byte), padding (3 bytes) } +// Tagged: +// { tag (1 byte), NonZeroU8 (1 byte), padding (2 bytes), u32 (4 bytes) } +// Both are the same size (due to padding), +// but the tagged layout is better as the tag creates a niche with 254 invalid values, +// allowing types like `Option>` to fit into 8 bytes. pub enum CanBeNicheFilledButShouldnt { A(NonZeroU8, u32), B } -pub enum AlwaysTagged { +pub enum AlwaysTaggedBecauseItHasNoNiche { A(u8, u32), B } @@ -159,7 +167,7 @@ pub fn main() { assert_eq!(size_of::(), 8); assert_eq!(size_of::>(), 8); assert_eq!(size_of::>>(), 8); - assert_eq!(size_of::(), 8); - assert_eq!(size_of::>(), 8); - assert_eq!(size_of::>>(), 8); + assert_eq!(size_of::(), 8); + assert_eq!(size_of::>(), 8); + assert_eq!(size_of::>>(), 8); }