Skip to content

Commit

Permalink
Rollup merge of rust-lang#129751 - RalfJung:interpret-visit-field-ord…
Browse files Browse the repository at this point in the history
…er, r=compiler-errors

interpret/visitor: make memory order iteration slightly more efficient

Finally I know enough about RPIT to write this iterator signature correctly. :D

This means memory-order iteration now needs an allocation, but it avoids quadratic complexity (where it has to do a linear scan n times to find the n-th field in memory order), so that seems like a win overall. The changed code only affects Miri; the rustc changes are NOPs.
  • Loading branch information
workingjubilee authored Aug 30, 2024
2 parents a9531e3 + de34a91 commit 1b5c216
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 17 deletions.
19 changes: 10 additions & 9 deletions compiler/rustc_const_eval/src/interpret/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ pub trait ValueVisitor<'tcx, M: Machine<'tcx>>: Sized {
}

/// This function provides the chance to reorder the order in which fields are visited for
/// `FieldsShape::Aggregate`: The order of fields will be
/// `(0..num_fields).map(aggregate_field_order)`.
/// `FieldsShape::Aggregate`.
///
/// The default means we iterate in source declaration order; alternative this can do an inverse
/// lookup in `memory_index` to use memory field order instead.
/// The default means we iterate in source declaration order; alternatively this can do some
/// work with `memory_index` to iterate in memory order.
#[inline(always)]
fn aggregate_field_order(_memory_index: &IndexVec<FieldIdx, u32>, idx: usize) -> usize {
idx
fn aggregate_field_iter(
memory_index: &IndexVec<FieldIdx, u32>,
) -> impl Iterator<Item = FieldIdx> + 'static {
memory_index.indices()
}

// Recursive actions, ready to be overloaded.
Expand Down Expand Up @@ -172,9 +173,9 @@ pub trait ValueVisitor<'tcx, M: Machine<'tcx>>: Sized {
&FieldsShape::Union(fields) => {
self.visit_union(v, fields)?;
}
FieldsShape::Arbitrary { offsets, memory_index } => {
for idx in 0..offsets.len() {
let idx = Self::aggregate_field_order(memory_index, idx);
FieldsShape::Arbitrary { memory_index, .. } => {
for idx in Self::aggregate_field_iter(memory_index) {
let idx = idx.as_usize();
let field = self.ecx().project_field(v, idx)?;
self.visit_field(v, idx, &field)?;
}
Expand Down
13 changes: 5 additions & 8 deletions src/tools/miri/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,14 +630,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
self.ecx
}

fn aggregate_field_order(memory_index: &IndexVec<FieldIdx, u32>, idx: usize) -> usize {
// We need to do an *inverse* lookup: find the field that has position `idx` in memory order.
for (src_field, &mem_pos) in memory_index.iter_enumerated() {
if mem_pos as usize == idx {
return src_field.as_usize();
}
}
panic!("invalid `memory_index`, could not find {}-th field in memory order", idx);
fn aggregate_field_iter(
memory_index: &IndexVec<FieldIdx, u32>,
) -> impl Iterator<Item = FieldIdx> + 'static {
let inverse_memory_index = memory_index.invert_bijective_mapping();
inverse_memory_index.into_iter()
}

// Hook to detect `UnsafeCell`.
Expand Down

0 comments on commit 1b5c216

Please sign in to comment.