Skip to content

Commit

Permalink
Deduce new constraints also on inputs. (#2330)
Browse files Browse the repository at this point in the history
  • Loading branch information
chriseth authored Jan 13, 2025
1 parent f795d41 commit fe4e2b2
Showing 1 changed file with 28 additions and 26 deletions.
54 changes: 28 additions & 26 deletions executor/src/witgen/machines/fixed_lookup_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,16 +281,27 @@ impl<'a, T: FieldElement> Machine<'a, T> for FixedLookup<'a, T> {
.collect_vec();

// Now only consider the index entries that match the input range constraints,
// see that the result is unique and determine new output range constraints.
let values_matching_input_constraints = index
// see that the result is unique and determine new range constraints.
let items_matching_input_constraints = index
.iter()
.filter(|(key, _)| matches_range_constraint(key, &input_range_constraints))
.map(|(_, value)| {
let (_, values) = value.get()?;
Some(values)
.filter(|(inputs, _)| matches_range_constraint(inputs, &input_range_constraints))
.map(|(inputs, value)| {
let (_, outputs) = value.get()?;
// Now re-order the items according to the connection (instead of
// by input/output).
let mut inputs = inputs.iter();
let mut outputs = outputs.iter();
let unpartition = |is_known| {
if is_known {
*inputs.next().unwrap()
} else {
*outputs.next().unwrap()
}
};
Some(known_arguments.iter().map(unpartition).collect_vec())
});
let mut new_range_constraints: Option<Vec<(T, T, T::Integer)>> = None;
for values in values_matching_input_constraints {
for values in items_matching_input_constraints {
// If any value is None, it means the lookup does not have a unique answer,
// and thus we cannot process the call.
let values = values?;
Expand All @@ -303,8 +314,8 @@ impl<'a, T: FieldElement> Machine<'a, T> for FixedLookup<'a, T> {
// Reduce range constraint by disjunction.
Some(mut acc) => {
for ((min, max, mask), v) in acc.iter_mut().zip_eq(values) {
*min = (*min).min(*v);
*max = (*max).max(*v);
*min = (*min).min(v);
*max = (*max).max(v);
*mask |= v.to_integer();
}
acc
Expand All @@ -320,25 +331,16 @@ impl<'a, T: FieldElement> Machine<'a, T> for FixedLookup<'a, T> {
// no way to signal this in the return type, yet.
// TODO(#2324): change this.
// We just return the input range constraints to signal "everything allright".
log::trace!("Call to FixedLookup resulted in no match.");
range_constraints.to_vec()
}
Some(new_output_range_constraints) => {
let mut new_output_range_constraints = new_output_range_constraints.into_iter();
known_arguments
.iter()
.enumerate()
.map(|(i, is_known)| {
if is_known {
// Just copy the input range constraint.
range_constraints[i].clone()
} else {
let (min, max, mask) = new_output_range_constraints.next().unwrap();
RangeConstraint::from_range(min, max)
.conjunction(&RangeConstraint::from_mask(mask))
}
})
.collect()
}
Some(new_range_constraints) => new_range_constraints
.into_iter()
.map(|(min, max, mask)| {
RangeConstraint::from_range(min, max)
.conjunction(&RangeConstraint::from_mask(mask))
})
.collect(),
})
}

Expand Down

1 comment on commit fe4e2b2

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: fe4e2b2 Previous: 64de47c Ratio
executor-benchmark/keccak 13684042652 ns/iter (± 424981947) 9185463295 ns/iter (± 110997725) 1.49

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.