From 4d77d010962dc91e225437cbd508e1b10a9192cf Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Wed, 25 Mar 2020 22:42:13 +0100 Subject: [PATCH 1/4] Fix for #62691: use the largest niche across all fields fixes #62691 --- src/librustc/ty/layout.rs | 25 +++++++++++-------------- src/test/ui/type-sizes.rs | 7 +++++++ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index c54360e03933e..e63a9eac0e643 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -282,8 +282,6 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align }; - let mut sized = true; - let mut offsets = vec![Size::ZERO; fields.len()]; let mut inverse_memory_index: Vec = (0..fields.len() as u32).collect(); let mut optimize = !repr.inhibit_struct_field_reordering_opt(); @@ -320,6 +318,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // 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 = vec![Size::ZERO; fields.len()]; let mut offset = Size::ZERO; let mut largest_niche = None; let mut largest_niche_available = 0; @@ -907,18 +907,15 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let count = (niche_variants.end().as_u32() - niche_variants.start().as_u32() + 1) as u128; - // FIXME(#62691) use the largest niche across all fields, - // not just the first one. - for (field_index, &field) in variants[i].iter().enumerate() { - let niche = match &field.largest_niche { - Some(niche) => niche, - _ => continue, - }; - let (niche_start, niche_scalar) = match niche.reserve(self, count) { - Some(pair) => pair, - None => continue, - }; - + if let Some((field_index, niche, (niche_start, niche_scalar))) = variants[i] + .iter() + .enumerate() + .filter_map(|(i, &field)| { + let niche = field.largest_niche.as_ref()?; + Some((i, niche, niche.reserve(self, count)?)) + }) + .max_by_key(|(_, niche, _)| niche.available(dl)) + { let mut align = dl.aggregate_align; let st = variants .iter_enumerated() diff --git a/src/test/ui/type-sizes.rs b/src/test/ui/type-sizes.rs index 27433fd770b05..1d332cc3bf7ba 100644 --- a/src/test/ui/type-sizes.rs +++ b/src/test/ui/type-sizes.rs @@ -74,6 +74,11 @@ enum NicheFilledEnumWithAbsentVariant { C, } +enum Option2 { + Some(A, B), + None +} + pub fn main() { assert_eq!(size_of::(), 1 as usize); assert_eq!(size_of::(), 4 as usize); @@ -113,4 +118,6 @@ 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::>>(), size_of::<(bool, &())>()); } From 81006f644da0963aa24aa86d1df9a69a859efcef Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Wed, 25 Mar 2020 23:05:47 +0100 Subject: [PATCH 2/4] Optimize slightly by avoiding to call Niche::reserve when not needed --- src/librustc/ty/layout.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index e63a9eac0e643..e8c0ec89cc443 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -907,14 +907,25 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let count = (niche_variants.end().as_u32() - niche_variants.start().as_u32() + 1) as u128; - if let Some((field_index, niche, (niche_start, niche_scalar))) = variants[i] - .iter() - .enumerate() - .filter_map(|(i, &field)| { - let niche = field.largest_niche.as_ref()?; - Some((i, niche, niche.reserve(self, count)?)) + let mut niche_size = 0; + if let Some((field_index, niche, niche_start, niche_scalar)) = + variants[i].iter().enumerate().fold(None, |acc, (j, &field)| { + let niche = match &field.largest_niche { + Some(niche) => niche, + _ => return acc, + }; + let ns = niche.available(dl); + if ns <= niche_size { + return acc; + } + match niche.reserve(self, count) { + Some(pair) => { + niche_size = ns; + Some((j, niche, pair.0, pair.1)) + } + None => acc, + } }) - .max_by_key(|(_, niche, _)| niche.available(dl)) { let mut align = dl.aggregate_align; let st = variants From bb3e513a2cfdc01a35c5226c24b15200d635135e Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 26 Mar 2020 00:10:14 +0100 Subject: [PATCH 3/4] Revert previous commit and make the optimisation in a nicer way --- src/librustc/ty/layout.rs | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index e8c0ec89cc443..91d39334acb78 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -907,25 +907,12 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let count = (niche_variants.end().as_u32() - niche_variants.start().as_u32() + 1) as u128; - let mut niche_size = 0; - if let Some((field_index, niche, niche_start, niche_scalar)) = - variants[i].iter().enumerate().fold(None, |acc, (j, &field)| { - let niche = match &field.largest_niche { - Some(niche) => niche, - _ => return acc, - }; - let ns = niche.available(dl); - if ns <= niche_size { - return acc; - } - match niche.reserve(self, count) { - Some(pair) => { - niche_size = ns; - Some((j, niche, pair.0, pair.1)) - } - None => acc, - } - }) + if let Some((field_index, niche, (niche_start, niche_scalar))) = variants[i] + .iter() + .enumerate() + .filter_map(|(i, &field)| Some((i, field.largest_niche.as_ref()?))) + .max_by_key(|(_, niche)| niche.available(dl)) + .and_then(|(i, niche)| Some((i, niche, niche.reserve(self, count)?))) { let mut align = dl.aggregate_align; let st = variants From 0b00c20465c2cacf34b4d3d1a5f4d0427f384cb2 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 26 Mar 2020 13:14:25 +0100 Subject: [PATCH 4/4] Reorganize a bit the code and add a comment --- src/librustc/ty/layout.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 91d39334acb78..dc34f07665475 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -907,12 +907,18 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let count = (niche_variants.end().as_u32() - niche_variants.start().as_u32() + 1) as u128; - if let Some((field_index, niche, (niche_start, niche_scalar))) = variants[i] + + // Find the field with the largest niche + let niche_candidate = variants[i] .iter() .enumerate() - .filter_map(|(i, &field)| Some((i, field.largest_niche.as_ref()?))) - .max_by_key(|(_, niche)| niche.available(dl)) - .and_then(|(i, niche)| Some((i, niche, niche.reserve(self, count)?))) + .filter_map(|(j, &field)| Some((j, field.largest_niche.as_ref()?))) + .max_by_key(|(_, niche)| niche.available(dl)); + + if let Some((field_index, niche, (niche_start, niche_scalar))) = + niche_candidate.and_then(|(field_index, niche)| { + Some((field_index, niche, niche.reserve(self, count)?)) + }) { let mut align = dl.aggregate_align; let st = variants