From 65c70900ceda1c276c8eef1dad4e2c4f7a0756d0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Sep 2024 14:42:03 +0200 Subject: [PATCH] union padding computation: add fast-path for ZST Also avoid even tracking empty ranges, and add fast-path for arrays of scalars --- .../src/interpret/validity.rs | 39 +++++++++++++------ compiler/rustc_middle/src/ty/sty.rs | 1 + src/tools/miri/tests/pass/arrays.rs | 15 +++++++ 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index 806ebb9dfed4d..fb24f983ca9c3 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -220,6 +220,10 @@ pub struct RangeSet(Vec<(Size, Size)>); impl RangeSet { fn add_range(&mut self, offset: Size, size: Size) { + if size.bytes() == 0 { + // No need to track empty ranges. + return; + } let v = &mut self.0; // We scan for a partition point where the left partition is all the elements that end // strictly before we start. Those are elements that are too "low" to merge with us. @@ -938,42 +942,53 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> { let layout_cx = LayoutCx { tcx: *ecx.tcx, param_env: ecx.param_env }; return M::cached_union_data_range(ecx, layout.ty, || { let mut out = RangeSet(Vec::new()); - union_data_range_(&layout_cx, layout, Size::ZERO, &mut out); + union_data_range_uncached(&layout_cx, layout, Size::ZERO, &mut out); out }); /// Helper for recursive traversal: add data ranges of the given type to `out`. - fn union_data_range_<'tcx>( + fn union_data_range_uncached<'tcx>( cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, layout: TyAndLayout<'tcx>, base_offset: Size, out: &mut RangeSet, ) { + // If this is a ZST, we don't contain any data. In particular, this helps us to quickly + // skip over huge arrays of ZST. + if layout.is_zst() { + return; + } // Just recursively add all the fields of everything to the output. match &layout.fields { FieldsShape::Primitive => { out.add_range(base_offset, layout.size); } &FieldsShape::Union(fields) => { - // Currently, all fields start at offset 0. + // Currently, all fields start at offset 0 (relative to `base_offset`). for field in 0..fields.get() { let field = layout.field(cx, field); - union_data_range_(cx, field, base_offset, out); + union_data_range_uncached(cx, field, base_offset, out); } } &FieldsShape::Array { stride, count } => { let elem = layout.field(cx, 0); - for idx in 0..count { - // This repeats the same computation for every array elements... but the alternative - // is to allocate temporary storage for a dedicated `out` set for the array element, - // and replicating that N times. Is that better? - union_data_range_(cx, elem, base_offset + idx * stride, out); + + // Fast-path for large arrays of simple types that do not contain any padding. + if elem.abi.is_scalar() { + out.add_range(base_offset, elem.size * count); + } else { + for idx in 0..count { + // This repeats the same computation for every array element... but the alternative + // is to allocate temporary storage for a dedicated `out` set for the array element, + // and replicating that N times. Is that better? + union_data_range_uncached(cx, elem, base_offset + idx * stride, out); + } } } FieldsShape::Arbitrary { offsets, .. } => { for (field, &offset) in offsets.iter_enumerated() { let field = layout.field(cx, field.as_usize()); - union_data_range_(cx, field, base_offset + offset, out); + union_data_range_uncached(cx, field, base_offset + offset, out); } } } @@ -985,7 +1000,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> { Variants::Multiple { variants, .. } => { for variant in variants.indices() { let variant = layout.for_variant(cx, variant); - union_data_range_(cx, variant, base_offset, out); + union_data_range_uncached(cx, variant, base_offset, out); } } } @@ -1185,7 +1200,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt, // This is the size in bytes of the whole array. (This checks for overflow.) let size = layout.size * len; // If the size is 0, there is nothing to check. - // (`size` can only be 0 of `len` is 0, and empty arrays are always valid.) + // (`size` can only be 0 if `len` is 0, and empty arrays are always valid.) if size == Size::ZERO { return Ok(()); } diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 1f4f2c62d7084..730ba265b19d6 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1136,6 +1136,7 @@ impl<'tcx> Ty<'tcx> { } /// Tests if this is any kind of primitive pointer type (reference, raw pointer, fn pointer). + /// `Box` is *not* considered a pointer here! #[inline] pub fn is_any_ptr(self) -> bool { self.is_ref() || self.is_unsafe_ptr() || self.is_fn_ptr() diff --git a/src/tools/miri/tests/pass/arrays.rs b/src/tools/miri/tests/pass/arrays.rs index 61b44453e9bd9..b0c6f54cab87c 100644 --- a/src/tools/miri/tests/pass/arrays.rs +++ b/src/tools/miri/tests/pass/arrays.rs @@ -61,6 +61,20 @@ fn debug() { println!("{:?}", array); } +fn huge_zst() { + fn id(x: T) -> T { x } + + // A "huge" zero-sized array. Make sure we don't loop over it in any part of Miri. + let val = [(); usize::MAX]; + id(val); // make a copy + + let val = [val; 2]; + id(val); + + // Also wrap it in a union (which, in particular, hits the logic for computing union padding). + let _copy = std::mem::MaybeUninit::new(val); +} + fn main() { assert_eq!(empty_array(), []); assert_eq!(index_unsafe(), 20); @@ -73,4 +87,5 @@ fn main() { from(); eq(); debug(); + huge_zst(); }