Skip to content

Commit 956b51f

Browse files
RalfJungRalfJung
committed
optimize validation iterating over the elements of an array
This is still roughly 45ns slower than the old state, because it now works with an MPlaceTy and uses the appropriate abstractions, instead of working with a ptr-align pair directly.
1 parent 6f5cf12 commit 956b51f

File tree

4 files changed

+72
-30
lines changed

4 files changed

+72
-30
lines changed

‎src/librustc_mir/interpret/memory.rs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
494494

495495
/// Byte accessors
496496
impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
497+
/// This checks alignment!
497498
fn get_bytes_unchecked(
498499
&self,
499500
ptr: Pointer,
@@ -514,6 +515,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
514515
Ok(&alloc.bytes[offset..offset + size.bytes() as usize])
515516
}
516517

518+
/// This checks alignment!
517519
fn get_bytes_unchecked_mut(
518520
&mut self,
519521
ptr: Pointer,
@@ -551,7 +553,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
551553
) -> EvalResult<'tcx, &mut [u8]> {
552554
assert_ne!(size.bytes(), 0);
553555
self.clear_relocations(ptr, size)?;
554-
self.mark_definedness(ptr.into(), size, true)?;
556+
self.mark_definedness(ptr, size, true)?;
555557
self.get_bytes_unchecked_mut(ptr, size, align)
556558
}
557559
}
@@ -749,9 +751,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
749751
Ok(())
750752
}
751753

754+
/// Read a *non-ZST* scalar
752755
pub fn read_scalar(&self, ptr: Pointer, ptr_align: Align, size: Size) -> EvalResult<'tcx, ScalarMaybeUndef> {
753756
self.check_relocation_edges(ptr, size)?; // Make sure we don't read part of a pointer as a pointer
754757
let endianness = self.endianness();
758+
// get_bytes_unchecked tests alignment
755759
let bytes = self.get_bytes_unchecked(ptr, size, ptr_align.min(self.int_align(size)))?;
756760
// Undef check happens *after* we established that the alignment is correct.
757761
// We must not return Ok() for unaligned pointers!
@@ -784,16 +788,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
784788
self.read_scalar(ptr, ptr_align, self.pointer_size())
785789
}
786790

791+
/// Write a *non-ZST* scalar
787792
pub fn write_scalar(
788793
&mut self,
789-
ptr: Scalar,
794+
ptr: Pointer,
790795
ptr_align: Align,
791796
val: ScalarMaybeUndef,
792797
type_size: Size,
793-
type_align: Align,
794798
) -> EvalResult<'tcx> {
795799
let endianness = self.endianness();
796-
self.check_align(ptr, ptr_align)?;
797800

798801
let val = match val {
799802
ScalarMaybeUndef::Scalar(scalar) => scalar,
@@ -806,12 +809,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
806809
val.offset.bytes() as u128
807810
}
808811

809-
Scalar::Bits { size: 0, .. } => {
810-
// nothing to do for ZSTs
811-
assert_eq!(type_size.bytes(), 0);
812-
return Ok(());
813-
}
814-
815812
Scalar::Bits { bits, size } => {
816813
assert_eq!(size as u64, type_size.bytes());
817814
assert_eq!(truncate(bits, Size::from_bytes(size.into())), bits,
@@ -820,10 +817,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
820817
},
821818
};
822819

823-
let ptr = ptr.to_ptr()?;
824-
825820
{
826-
let dst = self.get_bytes_mut(ptr, type_size, ptr_align.min(type_align))?;
821+
// get_bytes_mut checks alignment
822+
let dst = self.get_bytes_mut(ptr, type_size, ptr_align)?;
827823
write_target_uint(endianness, dst, bytes).unwrap();
828824
}
829825

@@ -843,7 +839,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
843839

844840
pub fn write_ptr_sized(&mut self, ptr: Pointer, ptr_align: Align, val: ScalarMaybeUndef) -> EvalResult<'tcx> {
845841
let ptr_size = self.pointer_size();
846-
self.write_scalar(ptr.into(), ptr_align, val, ptr_size, ptr_align)
842+
self.write_scalar(ptr.into(), ptr_align, val, ptr_size)
847843
}
848844

849845
fn int_align(&self, size: Size) -> Align {
@@ -959,14 +955,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
959955

960956
pub fn mark_definedness(
961957
&mut self,
962-
ptr: Scalar,
958+
ptr: Pointer,
963959
size: Size,
964960
new_state: bool,
965961
) -> EvalResult<'tcx> {
966962
if size.bytes() == 0 {
967963
return Ok(());
968964
}
969-
let ptr = ptr.to_ptr()?;
970965
let alloc = self.get_mut(ptr.alloc_id)?;
971966
alloc.undef_mask.set_range(
972967
ptr.offset,

‎src/librustc_mir/interpret/operand.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,22 @@ impl<'tcx> Value {
4141
Value::ScalarPair(val.into(), Scalar::Ptr(vtable).into())
4242
}
4343

44+
#[inline]
4445
pub fn to_scalar_or_undef(self) -> ScalarMaybeUndef {
4546
match self {
4647
Value::Scalar(val) => val,
4748
Value::ScalarPair(..) => bug!("Got a fat pointer where a scalar was expected"),
4849
}
4950
}
5051

52+
#[inline]
5153
pub fn to_scalar(self) -> EvalResult<'tcx, Scalar> {
5254
self.to_scalar_or_undef().not_undef()
5355
}
5456

5557
/// Convert the value into a pointer (or a pointer-sized integer).
58+
/// Throws away the second half of a ScalarPair!
59+
#[inline]
5660
pub fn to_scalar_ptr(self) -> EvalResult<'tcx, Scalar> {
5761
match self {
5862
Value::Scalar(ptr) |
@@ -89,6 +93,7 @@ pub struct ValTy<'tcx> {
8993

9094
impl<'tcx> ::std::ops::Deref for ValTy<'tcx> {
9195
type Target = Value;
96+
#[inline(always)]
9297
fn deref(&self) -> &Value {
9398
&self.value
9499
}
@@ -141,12 +146,14 @@ pub struct OpTy<'tcx> {
141146

142147
impl<'tcx> ::std::ops::Deref for OpTy<'tcx> {
143148
type Target = Operand;
149+
#[inline(always)]
144150
fn deref(&self) -> &Operand {
145151
&self.op
146152
}
147153
}
148154

149155
impl<'tcx> From<MPlaceTy<'tcx>> for OpTy<'tcx> {
156+
#[inline(always)]
150157
fn from(mplace: MPlaceTy<'tcx>) -> Self {
151158
OpTy {
152159
op: Operand::Indirect(*mplace),
@@ -156,6 +163,7 @@ impl<'tcx> From<MPlaceTy<'tcx>> for OpTy<'tcx> {
156163
}
157164

158165
impl<'tcx> From<ValTy<'tcx>> for OpTy<'tcx> {
166+
#[inline(always)]
159167
fn from(val: ValTy<'tcx>) -> Self {
160168
OpTy {
161169
op: Operand::Immediate(val.value),
@@ -192,14 +200,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
192200
return Ok(None);
193201
}
194202
let (ptr, ptr_align) = mplace.to_scalar_ptr_align();
195-
self.memory.check_align(ptr, ptr_align)?;
196203

197204
if mplace.layout.size.bytes() == 0 {
205+
// Not all ZSTs have a layout we would handle below, so just short-circuit them
206+
// all here.
207+
self.memory.check_align(ptr, ptr_align)?;
198208
return Ok(Some(Value::Scalar(Scalar::zst().into())));
199209
}
200210

201211
let ptr = ptr.to_ptr()?;
202-
203212
match mplace.layout.abi {
204213
layout::Abi::Scalar(..) => {
205214
let scalar = self.memory.read_scalar(ptr, ptr_align, mplace.layout.size)?;
@@ -264,7 +273,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
264273
// This decides which types we will use the Immediate optimization for, and hence should
265274
// match what `try_read_value` and `eval_place_to_op` support.
266275
if layout.is_zst() {
267-
return Ok(Operand::Immediate(Value::Scalar(ScalarMaybeUndef::Undef)));
276+
return Ok(Operand::Immediate(Value::Scalar(Scalar::zst().into())));
268277
}
269278

270279
Ok(match layout.abi {

‎src/librustc_mir/interpret/place.rs

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ pub struct PlaceTy<'tcx> {
5454

5555
impl<'tcx> ::std::ops::Deref for PlaceTy<'tcx> {
5656
type Target = Place;
57+
#[inline(always)]
5758
fn deref(&self) -> &Place {
5859
&self.place
5960
}
@@ -68,12 +69,14 @@ pub struct MPlaceTy<'tcx> {
6869

6970
impl<'tcx> ::std::ops::Deref for MPlaceTy<'tcx> {
7071
type Target = MemPlace;
72+
#[inline(always)]
7173
fn deref(&self) -> &MemPlace {
7274
&self.mplace
7375
}
7476
}
7577

7678
impl<'tcx> From<MPlaceTy<'tcx>> for PlaceTy<'tcx> {
79+
#[inline(always)]
7780
fn from(mplace: MPlaceTy<'tcx>) -> Self {
7881
PlaceTy {
7982
place: Place::Ptr(mplace.mplace),
@@ -160,14 +163,15 @@ impl<'tcx> PartialEq for MPlaceTy<'tcx> {
160163
impl<'tcx> Eq for MPlaceTy<'tcx> {}
161164

162165
impl<'tcx> OpTy<'tcx> {
166+
#[inline(always)]
163167
pub fn try_as_mplace(self) -> Result<MPlaceTy<'tcx>, Value> {
164168
match *self {
165169
Operand::Indirect(mplace) => Ok(MPlaceTy { mplace, layout: self.layout }),
166170
Operand::Immediate(value) => Err(value),
167171
}
168172
}
169173

170-
#[inline]
174+
#[inline(always)]
171175
pub fn to_mem_place(self) -> MPlaceTy<'tcx> {
172176
self.try_as_mplace().unwrap()
173177
}
@@ -311,6 +315,28 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
311315
Ok(MPlaceTy { mplace: MemPlace { ptr, align, extra }, layout: field_layout })
312316
}
313317

318+
// Iterates over all fields of an array. Much more efficient than doing the
319+
// same by repeatedly calling `mplace_array`.
320+
pub fn mplace_array_fields(
321+
&self,
322+
base: MPlaceTy<'tcx>,
323+
) -> EvalResult<'tcx, impl Iterator<Item=EvalResult<'tcx, MPlaceTy<'tcx>>> + 'a> {
324+
let len = base.len();
325+
let stride = match base.layout.fields {
326+
layout::FieldPlacement::Array { stride, .. } => stride,
327+
_ => bug!("mplace_array_fields: expected an array layout"),
328+
};
329+
let layout = base.layout.field(self, 0)?;
330+
let dl = &self.tcx.data_layout;
331+
Ok((0..len).map(move |i| {
332+
let ptr = base.ptr.ptr_offset(i * stride, dl)?;
333+
Ok(MPlaceTy {
334+
mplace: MemPlace { ptr, align: base.align, extra: PlaceExtra::None },
335+
layout
336+
})
337+
}))
338+
}
339+
314340
pub fn mplace_subslice(
315341
&self,
316342
base: MPlaceTy<'tcx>,
@@ -545,14 +571,22 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
545571
value: Value,
546572
dest: MPlaceTy<'tcx>,
547573
) -> EvalResult<'tcx> {
548-
assert_eq!(dest.extra, PlaceExtra::None);
574+
let (ptr, ptr_align) = dest.to_scalar_ptr_align();
549575
// Note that it is really important that the type here is the right one, and matches the type things are read at.
550576
// In case `src_val` is a `ScalarPair`, we don't do any magic here to handle padding properly, which is only
551577
// correct if we never look at this data with the wrong type.
578+
579+
// Nothing to do for ZSTs, other than checking alignment
580+
if dest.layout.size.bytes() == 0 {
581+
self.memory.check_align(ptr, ptr_align)?;
582+
return Ok(());
583+
}
584+
585+
let ptr = ptr.to_ptr()?;
552586
match value {
553587
Value::Scalar(scalar) => {
554588
self.memory.write_scalar(
555-
dest.ptr, dest.align, scalar, dest.layout.size, dest.layout.align
589+
ptr, ptr_align.min(dest.layout.align), scalar, dest.layout.size
556590
)
557591
}
558592
Value::ScalarPair(a_val, b_val) => {
@@ -562,12 +596,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
562596
};
563597
let (a_size, b_size) = (a.size(&self), b.size(&self));
564598
let (a_align, b_align) = (a.align(&self), b.align(&self));
565-
let a_ptr = dest.ptr;
566599
let b_offset = a_size.abi_align(b_align);
567-
let b_ptr = a_ptr.ptr_offset(b_offset, &self)?.into();
600+
let b_ptr = ptr.offset(b_offset, &self)?.into();
568601

569-
self.memory.write_scalar(a_ptr, dest.align, a_val, a_size, a_align)?;
570-
self.memory.write_scalar(b_ptr, dest.align, b_val, b_size, b_align)
602+
self.memory.write_scalar(ptr, ptr_align.min(a_align), a_val, a_size)?;
603+
self.memory.write_scalar(b_ptr, ptr_align.min(b_align), b_val, b_size)
571604
}
572605
}
573606
}
@@ -608,6 +641,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
608641
) -> EvalResult<'tcx, MPlaceTy<'tcx>> {
609642
let mplace = match place.place {
610643
Place::Local { frame, local } => {
644+
// FIXME: Consider not doing anything for a ZST, and just returning
645+
// a fake pointer?
646+
611647
// We need the layout of the local. We can NOT use the layout we got,
612648
// that might e.g. be a downcast variant!
613649
let local_layout = self.layout_of_local(frame, local)?;
@@ -707,6 +743,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
707743
}
708744

709745
/// Every place can be read from, so we can turm them into an operand
746+
#[inline(always)]
710747
pub fn place_to_op(&self, place: PlaceTy<'tcx>) -> EvalResult<'tcx, OpTy<'tcx>> {
711748
let op = match place.place {
712749
Place::Ptr(mplace) => {

‎src/librustc_mir/interpret/validity.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
124124
}
125125
}
126126

127-
/// This function checks the memory where `ptr` points to.
127+
/// This function checks the memory where `dest` points to. The place must be sized
128+
/// (i.e., dest.extra == PlaceExtra::None).
128129
/// It will error if the bits at the destination do not match the ones described by the layout.
129130
pub fn validate_mplace(
130131
&self,
@@ -205,11 +206,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
205206
// See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389
206207
Ok(())
207208
},
208-
layout::FieldPlacement::Array { count, .. } => {
209-
for i in 0..count {
209+
layout::FieldPlacement::Array { .. } => {
210+
for (i, field) in self.mplace_array_fields(dest)?.enumerate() {
211+
let field = field?;
210212
let mut path = path.clone();
211213
self.dump_field_name(&mut path, dest.layout.ty, i as usize, variant).unwrap();
212-
let field = self.mplace_field(dest, i)?;
213214
self.validate_mplace(field, path, seen, todo)?;
214215
}
215216
Ok(())

0 commit comments

Comments
 (0)