Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interpret/visitor: make memory order iteration slightly more efficient #129751

Merged
merged 1 commit into from
Aug 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading