Auto merge of #74611 - Mark-Simulacrum:revert-74069-bad-niche, r=eddyb

Revert "Compare tagged/niche-filling layout and pick the best one"

Reverts rust-lang/rust#74069. It caused a performance regression, see https://github.com/rust-lang/rust/pull/74069#issuecomment-662166827. perf: https://perf.rust-lang.org/compare.html?start=d3df8512d2c2afc6d2e7d8b5b951dd7f2ad77b02&end=cfade73820883adf654fe34fd6b0b03a99458a51

r? @eddyb

cc @nnethercote

4 files changed, 7 insertions, 51 deletions

diff --git a/src/librustc_middle/lib.rs b/src/librustc_middle/lib.rs
index a68301385b7..b7dccb8d8ce 100644
--- a/src/librustc_middle/lib.rs
+++ b/src/librustc_middle/lib.rs
@@ -27,7 +27,6 @@
 #![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 dc775b15927..215f44819b5 100644
--- a/src/librustc_middle/ty/layout.rs
+++ b/src/librustc_middle/ty/layout.rs
@@ -876,8 +876,6 @@ 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;
@@ -974,7 +972,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
                             let largest_niche =
                                 Niche::from_scalar(dl, offset, niche_scalar.clone());
 
-                            niche_filling_layout = Some(Layout {
+                            return Ok(tcx.intern_layout(Layout {
                                 variants: Variants::Multiple {
                                     tag: niche_scalar,
                                     tag_encoding: TagEncoding::Niche {
@@ -993,7 +991,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
                                 largest_niche,
                                 size,
                                 align,
-                            });
+                            }));
                         }
                     }
                 }
@@ -1216,7 +1214,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
 
                 let largest_niche = Niche::from_scalar(dl, Size::ZERO, tag.clone());
 
-                let tagged_layout = Layout {
+                tcx.intern_layout(Layout {
                     variants: Variants::Multiple {
                         tag,
                         tag_encoding: TagEncoding::Direct,
@@ -1231,23 +1229,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
                     abi,
                     align,
                     size,
-                };
-
-                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 1894cd218ee..301edc0d086 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 `.pre`: 1 bytes
-print-type-size         field `.post`: 2 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     variant `None`: 0 bytes
+print-type-size     end padding: 1 bytes
 print-type-size type: `MyOption<Union1<std::num::NonZeroU32>>`: 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 73a11a5e743..6a3f3c98f12 100644
--- a/src/test/ui/type-sizes.rs
+++ b/src/test/ui/type-sizes.rs
@@ -5,7 +5,6 @@
 #![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}
@@ -103,23 +102,6 @@ enum Option2<A, B> {
     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<Option<CanBeNicheFilledButShouldnt>>` to fit into 8 bytes.
-pub enum CanBeNicheFilledButShouldnt {
-    A(NonZeroU8, u32),
-    B
-}
-pub enum AlwaysTaggedBecauseItHasNoNiche {
-    A(u8, u32),
-    B
-}
-
 pub fn main() {
     assert_eq!(size_of::<u8>(), 1 as usize);
     assert_eq!(size_of::<u32>(), 4 as usize);
@@ -163,11 +145,4 @@ pub fn main() {
     assert_eq!(size_of::<Option<Option<(&(), bool)>>>(), size_of::<(bool, &())>());
     assert_eq!(size_of::<Option<Option2<bool, &()>>>(), size_of::<(bool, &())>());
     assert_eq!(size_of::<Option<Option2<&(), bool>>>(), size_of::<(bool, &())>());
-
-    assert_eq!(size_of::<CanBeNicheFilledButShouldnt>(), 8);
-    assert_eq!(size_of::<Option<CanBeNicheFilledButShouldnt>>(), 8);
-    assert_eq!(size_of::<Option<Option<CanBeNicheFilledButShouldnt>>>(), 8);
-    assert_eq!(size_of::<AlwaysTaggedBecauseItHasNoNiche>(), 8);
-    assert_eq!(size_of::<Option<AlwaysTaggedBecauseItHasNoNiche>>(), 8);
-    assert_eq!(size_of::<Option<Option<AlwaysTaggedBecauseItHasNoNiche>>>(), 8);
 }