Skip to content

Commit 65c7090

Browse files
committed
union padding computation: add fast-path for ZST
Also avoid even tracking empty ranges, and add fast-path for arrays of scalars
1 parent 11bd99d commit 65c7090

File tree

3 files changed

+43
-12
lines changed

3 files changed

+43
-12
lines changed

Diff for: compiler/rustc_const_eval/src/interpret/validity.rs

+27-12
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,10 @@ pub struct RangeSet(Vec<(Size, Size)>);
220220

221221
impl RangeSet {
222222
fn add_range(&mut self, offset: Size, size: Size) {
223+
if size.bytes() == 0 {
224+
// No need to track empty ranges.
225+
return;
226+
}
223227
let v = &mut self.0;
224228
// We scan for a partition point where the left partition is all the elements that end
225229
// 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> {
938942
let layout_cx = LayoutCx { tcx: *ecx.tcx, param_env: ecx.param_env };
939943
return M::cached_union_data_range(ecx, layout.ty, || {
940944
let mut out = RangeSet(Vec::new());
941-
union_data_range_(&layout_cx, layout, Size::ZERO, &mut out);
945+
union_data_range_uncached(&layout_cx, layout, Size::ZERO, &mut out);
942946
out
943947
});
944948

945949
/// Helper for recursive traversal: add data ranges of the given type to `out`.
946-
fn union_data_range_<'tcx>(
950+
fn union_data_range_uncached<'tcx>(
947951
cx: &LayoutCx<'tcx, TyCtxt<'tcx>>,
948952
layout: TyAndLayout<'tcx>,
949953
base_offset: Size,
950954
out: &mut RangeSet,
951955
) {
956+
// If this is a ZST, we don't contain any data. In particular, this helps us to quickly
957+
// skip over huge arrays of ZST.
958+
if layout.is_zst() {
959+
return;
960+
}
952961
// Just recursively add all the fields of everything to the output.
953962
match &layout.fields {
954963
FieldsShape::Primitive => {
955964
out.add_range(base_offset, layout.size);
956965
}
957966
&FieldsShape::Union(fields) => {
958-
// Currently, all fields start at offset 0.
967+
// Currently, all fields start at offset 0 (relative to `base_offset`).
959968
for field in 0..fields.get() {
960969
let field = layout.field(cx, field);
961-
union_data_range_(cx, field, base_offset, out);
970+
union_data_range_uncached(cx, field, base_offset, out);
962971
}
963972
}
964973
&FieldsShape::Array { stride, count } => {
965974
let elem = layout.field(cx, 0);
966-
for idx in 0..count {
967-
// This repeats the same computation for every array elements... but the alternative
968-
// is to allocate temporary storage for a dedicated `out` set for the array element,
969-
// and replicating that N times. Is that better?
970-
union_data_range_(cx, elem, base_offset + idx * stride, out);
975+
976+
// Fast-path for large arrays of simple types that do not contain any padding.
977+
if elem.abi.is_scalar() {
978+
out.add_range(base_offset, elem.size * count);
979+
} else {
980+
for idx in 0..count {
981+
// This repeats the same computation for every array element... but the alternative
982+
// is to allocate temporary storage for a dedicated `out` set for the array element,
983+
// and replicating that N times. Is that better?
984+
union_data_range_uncached(cx, elem, base_offset + idx * stride, out);
985+
}
971986
}
972987
}
973988
FieldsShape::Arbitrary { offsets, .. } => {
974989
for (field, &offset) in offsets.iter_enumerated() {
975990
let field = layout.field(cx, field.as_usize());
976-
union_data_range_(cx, field, base_offset + offset, out);
991+
union_data_range_uncached(cx, field, base_offset + offset, out);
977992
}
978993
}
979994
}
@@ -985,7 +1000,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
9851000
Variants::Multiple { variants, .. } => {
9861001
for variant in variants.indices() {
9871002
let variant = layout.for_variant(cx, variant);
988-
union_data_range_(cx, variant, base_offset, out);
1003+
union_data_range_uncached(cx, variant, base_offset, out);
9891004
}
9901005
}
9911006
}
@@ -1185,7 +1200,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
11851200
// This is the size in bytes of the whole array. (This checks for overflow.)
11861201
let size = layout.size * len;
11871202
// If the size is 0, there is nothing to check.
1188-
// (`size` can only be 0 of `len` is 0, and empty arrays are always valid.)
1203+
// (`size` can only be 0 if `len` is 0, and empty arrays are always valid.)
11891204
if size == Size::ZERO {
11901205
return Ok(());
11911206
}

Diff for: compiler/rustc_middle/src/ty/sty.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1136,6 +1136,7 @@ impl<'tcx> Ty<'tcx> {
11361136
}
11371137

11381138
/// Tests if this is any kind of primitive pointer type (reference, raw pointer, fn pointer).
1139+
/// `Box` is *not* considered a pointer here!
11391140
#[inline]
11401141
pub fn is_any_ptr(self) -> bool {
11411142
self.is_ref() || self.is_unsafe_ptr() || self.is_fn_ptr()

Diff for: src/tools/miri/tests/pass/arrays.rs

+15
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,20 @@ fn debug() {
6161
println!("{:?}", array);
6262
}
6363

64+
fn huge_zst() {
65+
fn id<T>(x: T) -> T { x }
66+
67+
// A "huge" zero-sized array. Make sure we don't loop over it in any part of Miri.
68+
let val = [(); usize::MAX];
69+
id(val); // make a copy
70+
71+
let val = [val; 2];
72+
id(val);
73+
74+
// Also wrap it in a union (which, in particular, hits the logic for computing union padding).
75+
let _copy = std::mem::MaybeUninit::new(val);
76+
}
77+
6478
fn main() {
6579
assert_eq!(empty_array(), []);
6680
assert_eq!(index_unsafe(), 20);
@@ -73,4 +87,5 @@ fn main() {
7387
from();
7488
eq();
7589
debug();
90+
huge_zst();
7691
}

0 commit comments

Comments
 (0)