Refactorings around process_lookup_direct (#2209)

This PR refactors a few things:
- `process_lookup_direct` no longer has a default implementation.
Eventually, we want all machines to implement it, so I figured it would
be better to explicitly panic in each machine.
- Refactored the implementation of
`FixedLookupMachine::process_plookup`, pulling some stuff out into a new
`CallerData` struct. This is similar to what @chriseth has done on
[`call_jit_from_block`](https://github.com/powdr-labs/powdr/compare/main...call_jit_from_block),
see the comment below.
- As a first test, I implemented `process_lookup_direct` for the
"large"-field memory machine (and `process_plookup` by wrapping
`process_lookup_direct`)
This commit is contained in:
Georg Wiese
2024-12-10 17:44:15 +01:00
committed by GitHub
parent 11c3c7023c
commit 0180542559
12 changed files with 239 additions and 129 deletions

View File

@@ -0,0 +1,73 @@
use itertools::Itertools;
use powdr_number::FieldElement;
use crate::witgen::{
machines::LookupCell,
processor::{Left, OuterQuery},
EvalError, EvalResult, EvalValue,
};
/// A representation of the caller's data.
///
/// Useful for implementing [Machine::process_plookup] in terms of [Machine::process_lookup_direct].
pub struct CallerData<'a, 'b, T> {
/// The raw data of the caller. Unknown values should be ignored.
data: Vec<T>,
/// The affine expressions of the caller.
left: &'b Left<'a, T>,
}
impl<'a, 'b, T: FieldElement> From<&'b OuterQuery<'a, '_, T>> for CallerData<'a, 'b, T> {
/// Builds a `CallerData` from an `OuterQuery`.
fn from(outer_query: &'b OuterQuery<'a, '_, T>) -> Self {
let data = outer_query
.left
.iter()
.map(|l| l.constant_value().unwrap_or_default())
.collect();
Self {
data,
left: &outer_query.left,
}
}
}
impl<'a, 'b, T: FieldElement> CallerData<'a, 'b, T> {
/// Returns the data as a list of `LookupCell`s, as expected by `Machine::process_lookup_direct`.
pub fn as_lookup_cells(&mut self) -> Vec<LookupCell<'_, T>> {
self.data
.iter_mut()
.zip_eq(self.left.iter())
.map(|(value, left)| match left.constant_value().is_some() {
true => LookupCell::Input(value),
false => LookupCell::Output(value),
})
.collect()
}
}
impl<'a, 'b, T: FieldElement> From<CallerData<'a, 'b, T>> for EvalResult<'a, T> {
/// Turns the result of a direct lookup into an `EvalResult`, as used by `Machine::process_plookup`.
///
/// Note that this function assumes that the lookup was successful and complete.
fn from(data: CallerData<'a, 'b, T>) -> EvalResult<'a, T> {
let mut result = EvalValue::complete(vec![]);
for (l, v) in data.left.iter().zip_eq(data.data.iter()) {
if !l.is_constant() {
let evaluated = l.clone() - (*v).into();
match evaluated.solve() {
Ok(constraints) => {
result.combine(constraints);
}
Err(_) => {
// Fail the whole lookup
return Err(EvalError::ConstraintUnsatisfiable(format!(
"Constraint is invalid ({l} != {v}).",
)));
}
}
}
}
Ok(result)
}
}

View File

@@ -1,3 +1,4 @@
pub mod caller_data;
pub mod column_map;
pub mod copy_constraints;
pub mod finalizable_data;

View File

@@ -2,7 +2,9 @@ use std::collections::{BTreeMap, HashMap};
use std::fmt::Display;
use std::iter::{self};
use super::{compute_size_and_log, ConnectionKind, EvalResult, FixedData, MachineParts};
use super::{
compute_size_and_log, ConnectionKind, EvalResult, FixedData, LookupCell, MachineParts,
};
use crate::witgen::affine_expression::AlgebraicVariable;
use crate::witgen::analysis::detect_connection_type_and_block_size;
@@ -139,6 +141,15 @@ impl<'a, T: FieldElement> Machine<'a, T> for BlockMachine<'a, T> {
self.parts.connections.keys().copied().collect()
}
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(
&mut self,
_mutable_state: &'b MutableState<'a, T, Q>,
_identity_id: u64,
_values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>> {
unimplemented!("Direct lookup not supported by machine {}.", self.name())
}
fn process_plookup<'b, Q: QueryCallback<T>>(
&mut self,
mutable_state: &'b MutableState<'a, T, Q>,

View File

@@ -3,7 +3,7 @@ use std::iter::once;
use itertools::Itertools;
use super::{ConnectionKind, Machine, MachineParts};
use super::{ConnectionKind, LookupCell, Machine, MachineParts};
use crate::witgen::data_structures::mutable_state::MutableState;
use crate::witgen::machines::compute_size_and_log;
use crate::witgen::rows::RowPair;
@@ -214,6 +214,15 @@ impl<'a, T: FieldElement> DoubleSortedWitnesses16<'a, T> {
}
impl<'a, T: FieldElement> Machine<'a, T> for DoubleSortedWitnesses16<'a, T> {
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(
&mut self,
_mutable_state: &'b MutableState<'a, T, Q>,
_identity_id: u64,
_values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>> {
unimplemented!("Direct lookup not supported by machine {}.", self.name())
}
fn identity_ids(&self) -> Vec<u64> {
self.selector_ids.keys().cloned().collect()
}

View File

@@ -3,13 +3,14 @@ use std::iter::once;
use itertools::Itertools;
use super::{Machine, MachineParts};
use super::{LookupCell, Machine, MachineParts};
use crate::witgen::data_structures::caller_data::CallerData;
use crate::witgen::data_structures::mutable_state::MutableState;
use crate::witgen::machines::compute_size_and_log;
use crate::witgen::processor::OuterQuery;
use crate::witgen::rows::RowPair;
use crate::witgen::util::try_to_simple_poly;
use crate::witgen::{EvalError, EvalResult, FixedData, QueryCallback};
use crate::witgen::{EvalValue, IncompleteCause};
use crate::witgen::{EvalError, EvalResult, EvalValue, FixedData, IncompleteCause, QueryCallback};
use powdr_number::{DegreeType, FieldElement, LargeInt};
@@ -184,6 +185,15 @@ impl<'a, T: FieldElement> DoubleSortedWitnesses32<'a, T> {
}
impl<'a, T: FieldElement> Machine<'a, T> for DoubleSortedWitnesses32<'a, T> {
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(
&mut self,
_mutable_state: &'b MutableState<'a, T, Q>,
identity_id: u64,
values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>> {
self.process_plookup_internal(identity_id, values)
}
fn identity_ids(&self) -> Vec<u64> {
self.selector_ids.keys().cloned().collect()
}
@@ -194,11 +204,33 @@ impl<'a, T: FieldElement> Machine<'a, T> for DoubleSortedWitnesses32<'a, T> {
fn process_plookup<Q: QueryCallback<T>>(
&mut self,
_mutable_state: &MutableState<'a, T, Q>,
mutable_state: &MutableState<'a, T, Q>,
identity_id: u64,
caller_rows: &RowPair<'_, 'a, T>,
) -> EvalResult<'a, T> {
self.process_plookup_internal(identity_id, caller_rows)
let connection = self.parts.connections[&identity_id];
let outer_query = OuterQuery::new(caller_rows, connection);
let mut data = CallerData::from(&outer_query);
if self.process_lookup_direct(mutable_state, identity_id, &mut data.as_lookup_cells())? {
Ok(EvalResult::from(data)?.report_side_effect())
} else {
// One of the required arguments was not set, find out which:
let data = data.as_lookup_cells();
Ok(EvalValue::incomplete(
IncompleteCause::NonConstantRequiredArgument(
match (&data[0], &data[1], &data[2], &data[3]) {
(LookupCell::Output(_), _, _, _) => "operation_id",
(_, LookupCell::Output(_), _, _) => "m_addr",
(_, _, LookupCell::Output(_), _) => "m_step",
// Note that for the mload operation, we'd expect this to be an output.
// But since process_lookup_direct returned false and all other arguments are set,
// we must have been in the mstore operation, in which case the value is required.
(_, _, _, LookupCell::Output(_)) => "m_value",
_ => unreachable!(),
},
),
))
}
}
fn take_witness_col_values<'b, Q: QueryCallback<T>>(
@@ -343,8 +375,8 @@ impl<'a, T: FieldElement> DoubleSortedWitnesses32<'a, T> {
fn process_plookup_internal(
&mut self,
identity_id: u64,
caller_rows: &RowPair<'_, 'a, T>,
) -> EvalResult<'a, T> {
values: &mut [LookupCell<'_, T>],
) -> Result<bool, EvalError<T>> {
// We blindly assume the lookup is of the form
// OP { operation_id, ADDR, STEP, X } is <selector> { operation_id, m_addr, m_step, m_value }
// Where:
@@ -352,66 +384,39 @@ impl<'a, T: FieldElement> DoubleSortedWitnesses32<'a, T> {
// - operation_id == 1: Write
// - operation_id == 2: Bootloader write
let args = self.parts.connections[&identity_id]
.left
.expressions
.iter()
.map(|e| caller_rows.evaluate(e).unwrap())
.collect::<Vec<_>>();
let operation_id = match args[0].constant_value() {
Some(v) => v,
None => {
return Ok(EvalValue::incomplete(
IncompleteCause::NonConstantRequiredArgument("operation_id"),
))
}
let operation_id = match values[0] {
LookupCell::Input(v) => v,
LookupCell::Output(_) => return Ok(false),
};
let addr = match values[1] {
LookupCell::Input(v) => v,
LookupCell::Output(_) => return Ok(false),
};
let step = match values[2] {
LookupCell::Input(v) => v,
LookupCell::Output(_) => return Ok(false),
};
let value_ptr = &mut values[3];
let selector_id = *self.selector_ids.get(&identity_id).unwrap();
let is_normal_write = operation_id == T::from(OPERATION_ID_WRITE);
let is_bootloader_write = operation_id == T::from(OPERATION_ID_BOOTLOADER_WRITE);
let is_normal_write = operation_id == &T::from(OPERATION_ID_WRITE);
let is_bootloader_write = operation_id == &T::from(OPERATION_ID_BOOTLOADER_WRITE);
let is_write = is_bootloader_write || is_normal_write;
let addr = match args[1].constant_value() {
Some(v) => v,
None => {
return Ok(EvalValue::incomplete(
IncompleteCause::NonConstantRequiredArgument("m_addr"),
))
}
};
if self.has_bootloader_write_column {
let is_initialized = self.is_initialized.get(&addr).cloned().unwrap_or_default();
let is_initialized = self.is_initialized.get(addr).cloned().unwrap_or_default();
if !is_initialized && !is_bootloader_write {
panic!("Memory address {addr:x} must be initialized with a bootloader write",);
}
self.is_initialized.insert(addr, true);
self.is_initialized.insert(*addr, true);
}
let step = args[2]
.constant_value()
.ok_or_else(|| format!("Step must be known but is: {}", args[2]))?;
let value_expr = &args[3];
log::trace!(
"Query addr={:x}, step={step}, write: {is_write}, value: {}",
addr.to_arbitrary_integer(),
value_expr
);
// TODO this does not check any of the failure modes
let mut assignments = EvalValue::complete(vec![]);
let has_side_effect = if is_write {
let value = match value_expr.constant_value() {
Some(v) => v,
None => {
return Ok(EvalValue::incomplete(
IncompleteCause::NonConstantRequiredArgument("m_value"),
))
}
let added_memory_access = if is_write {
let value = match value_ptr {
LookupCell::Input(v) => *v,
LookupCell::Output(_) => return Ok(false),
};
log::trace!(
@@ -419,31 +424,39 @@ impl<'a, T: FieldElement> DoubleSortedWitnesses32<'a, T> {
addr,
value
);
self.data.write(addr, value);
self.data.write(*addr, *value);
self.trace
.insert(
(addr, step),
(*addr, *step),
Operation {
is_normal_write,
is_bootloader_write,
value,
value: *value,
selector_id,
},
)
.is_none()
} else {
let value = self.data.read(addr);
let value = self.data.read(*addr);
log::trace!(
"Memory read: addr={:x}, step={step}, value={:x}",
addr,
value
);
let ass =
(value_expr.clone() - value.into()).solve_with_range_constraints(caller_rows)?;
assignments.combine(ass);
match value_ptr {
LookupCell::Input(v) => {
if *v != &value {
return Err(EvalError::ConstraintUnsatisfiable(format!(
"{v} != {value} (address 0x{addr:x}, time step)"
)));
}
}
LookupCell::Output(v) => **v = value,
};
self.trace
.insert(
(addr, step),
(*addr, *step),
Operation {
is_normal_write,
is_bootloader_write,
@@ -454,16 +467,15 @@ impl<'a, T: FieldElement> DoubleSortedWitnesses32<'a, T> {
.is_none()
};
assert!(
has_side_effect,
added_memory_access,
"Already had a memory access for address 0x{addr:x} and time step {step}!"
);
assignments = assignments.report_side_effect();
if self.trace.len() > (self.degree as usize) {
return Err(EvalError::RowsExhausted(self.name.clone()));
}
Ok(assignments)
Ok(true)
}
}

View File

@@ -11,7 +11,11 @@ use crate::witgen::processor::{OuterQuery, SolverState};
use crate::witgen::rows::{Row, RowIndex, RowPair};
use crate::witgen::sequence_iterator::{DefaultSequenceIterator, ProcessingSequenceIterator};
use crate::witgen::vm_processor::VmProcessor;
use crate::witgen::{AlgebraicVariable, EvalResult, EvalValue, FixedData, QueryCallback};
use crate::witgen::{
AlgebraicVariable, EvalError, EvalResult, EvalValue, FixedData, QueryCallback,
};
use super::LookupCell;
struct ProcessResult<'a, T: FieldElement> {
eval_value: EvalValue<AlgebraicVariable<'a>, T>,
@@ -31,6 +35,15 @@ pub struct DynamicMachine<'a, T: FieldElement> {
}
impl<'a, T: FieldElement> Machine<'a, T> for DynamicMachine<'a, T> {
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(
&mut self,
_mutable_state: &'b MutableState<'a, T, Q>,
_identity_id: u64,
_values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>> {
unimplemented!("Direct lookup not supported by machine {}.", self.name())
}
fn identity_ids(&self) -> Vec<u64> {
self.parts.identity_ids()
}

View File

@@ -9,6 +9,7 @@ use powdr_ast::analyzed::{AlgebraicReference, PolynomialType};
use powdr_number::{DegreeType, FieldElement};
use crate::witgen::affine_expression::{AffineExpression, AlgebraicVariable};
use crate::witgen::data_structures::caller_data::CallerData;
use crate::witgen::data_structures::multiplicity_counter::MultiplicityCounter;
use crate::witgen::data_structures::mutable_state::MutableState;
use crate::witgen::global_constraints::{GlobalConstraints, RangeConstraintSet};
@@ -198,68 +199,37 @@ impl<'a, T: FieldElement> FixedLookup<'a, T> {
}
}
fn process_plookup_internal<'b, Q: QueryCallback<T>>(
fn process_plookup_internal<Q: QueryCallback<T>>(
&mut self,
mutable_state: &'b MutableState<'a, T, Q>,
mutable_state: &MutableState<'a, T, Q>,
identity_id: u64,
rows: &RowPair<'_, 'a, T>,
left: &[AffineExpression<AlgebraicVariable<'a>, T>],
outer_query: OuterQuery<'a, '_, T>,
mut right: Peekable<impl Iterator<Item = &'a AlgebraicReference>>,
) -> EvalResult<'a, T> {
if left.len() == 1
&& !left.first().unwrap().is_constant()
if outer_query.left.len() == 1
&& !outer_query.left.first().unwrap().is_constant()
&& right.peek().unwrap().poly_id.ptype == PolynomialType::Constant
{
// Lookup of the form "c $ [ X ] in [ B ]". Might be a conditional range check.
return self.process_range_check(
rows,
left.first().unwrap(),
outer_query.left.first().unwrap(),
AlgebraicVariable::Column(right.peek().unwrap()),
);
}
// Split the left-hand-side into known input values and unknown output expressions.
let mut data = vec![T::zero(); left.len()];
let mut values = left
.iter()
.zip(&mut data)
.map(|(l, d)| {
if let Some(value) = l.constant_value() {
*d = value;
LookupCell::Input(d)
} else {
LookupCell::Output(d)
}
})
.collect::<Vec<_>>();
let mut values = CallerData::from(&outer_query);
if !self.process_lookup_direct(mutable_state, identity_id, &mut values)? {
if !self.process_lookup_direct(mutable_state, identity_id, &mut values.as_lookup_cells())? {
// multiple matches, we stop and learnt nothing
return Ok(EvalValue::incomplete(
IncompleteCause::MultipleLookupMatches,
));
};
let mut result = EvalValue::complete(vec![]);
for (l, v) in left.iter().zip(data) {
if !l.is_constant() {
let evaluated = l.clone() - v.into();
// TODO we could use bit constraints here
match evaluated.solve() {
Ok(constraints) => {
result.combine(constraints);
}
Err(_) => {
// Fail the whole lookup
return Err(EvalError::ConstraintUnsatisfiable(format!(
"Constraint is invalid ({l} != {v}).",
)));
}
}
}
}
Ok(result)
values.into()
}
fn process_range_check(
@@ -327,11 +297,11 @@ impl<'a, T: FieldElement> Machine<'a, T> for FixedLookup<'a, T> {
"FixedLookup"
}
fn process_plookup<'b, Q: crate::witgen::QueryCallback<T>>(
fn process_plookup<Q: crate::witgen::QueryCallback<T>>(
&mut self,
mutable_state: &'b MutableState<'a, T, Q>,
mutable_state: &MutableState<'a, T, Q>,
identity_id: u64,
caller_rows: &'b RowPair<'b, 'a, T>,
caller_rows: &RowPair<'_, 'a, T>,
) -> EvalResult<'a, T> {
let identity = self.connections[&identity_id];
let right = identity.right;
@@ -344,13 +314,7 @@ impl<'a, T: FieldElement> Machine<'a, T> for FixedLookup<'a, T> {
.peekable();
let outer_query = OuterQuery::new(caller_rows, identity);
self.process_plookup_internal(
mutable_state,
identity_id,
caller_rows,
&outer_query.left,
right,
)
self.process_plookup_internal(mutable_state, identity_id, caller_rows, outer_query, right)
}
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(

View File

@@ -91,12 +91,10 @@ pub trait Machine<'a, T: FieldElement>: Send + Sync {
/// An error is always unrecoverable.
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(
&mut self,
_mutable_state: &'b MutableState<'a, T, Q>,
_identity_id: u64,
_values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>> {
unimplemented!("Direct lookup not supported machine {}.", self.name())
}
mutable_state: &'b MutableState<'a, T, Q>,
identity_id: u64,
values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>>;
/// Returns the final values of the witness columns.
fn take_witness_col_values<'b, Q: QueryCallback<T>>(

View File

@@ -11,7 +11,9 @@ use crate::witgen::processor::SolverState;
use crate::witgen::rows::{Row, RowIndex, RowPair};
use crate::witgen::sequence_iterator::{DefaultSequenceIterator, ProcessingSequenceIterator};
use crate::witgen::vm_processor::VmProcessor;
use crate::witgen::{EvalResult, FixedData, QueryCallback};
use crate::witgen::{EvalError, EvalResult, FixedData, QueryCallback};
use super::LookupCell;
/// A machine responsible for second-phase witness generation.
/// For example, this might generate the witnesses for a bus accumulator or LogUp argument.
@@ -25,6 +27,15 @@ pub struct SecondStageMachine<'a, T: FieldElement> {
}
impl<'a, T: FieldElement> Machine<'a, T> for SecondStageMachine<'a, T> {
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(
&mut self,
_mutable_state: &'b MutableState<'a, T, Q>,
_identity_id: u64,
_values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>> {
unimplemented!("Direct lookup not supported by machine {}.", self.name())
}
fn identity_ids(&self) -> Vec<u64> {
Vec::new()
}

View File

@@ -1,7 +1,7 @@
use std::collections::{BTreeMap, HashMap};
use super::super::affine_expression::AffineExpression;
use super::{Connection, EvalResult, FixedData};
use super::{Connection, EvalResult, FixedData, LookupCell};
use super::{Machine, MachineParts};
use crate::witgen::affine_expression::AlgebraicVariable;
use crate::witgen::data_structures::mutable_state::MutableState;
@@ -9,7 +9,7 @@ use crate::witgen::evaluators::fixed_evaluator::FixedEvaluator;
use crate::witgen::evaluators::partial_expression_evaluator::PartialExpressionEvaluator;
use crate::witgen::evaluators::symbolic_evaluator::SymbolicEvaluator;
use crate::witgen::rows::RowPair;
use crate::witgen::{EvalValue, IncompleteCause, QueryCallback};
use crate::witgen::{EvalError, EvalValue, IncompleteCause, QueryCallback};
use crate::Identity;
use itertools::Itertools;
use num_traits::One;
@@ -195,6 +195,15 @@ fn check_constraint<T: FieldElement>(
}
impl<'a, T: FieldElement> Machine<'a, T> for SortedWitnesses<'a, T> {
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(
&mut self,
_mutable_state: &'b MutableState<'a, T, Q>,
_identity_id: u64,
_values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>> {
unimplemented!("Direct lookup not supported by machine {}.", self.name())
}
fn identity_ids(&self) -> Vec<u64> {
self.rhs_references.keys().cloned().collect()
}

View File

@@ -13,7 +13,7 @@ use crate::witgen::{
QueryCallback,
};
use super::{Connection, Machine, MachineParts};
use super::{Connection, LookupCell, Machine, MachineParts};
/// A memory machine with a fixed address space, and each address can only have one
/// value during the lifetime of the program.
@@ -242,6 +242,15 @@ impl<'a, T: FieldElement> WriteOnceMemory<'a, T> {
}
impl<'a, T: FieldElement> Machine<'a, T> for WriteOnceMemory<'a, T> {
fn process_lookup_direct<'b, 'c, Q: QueryCallback<T>>(
&mut self,
_mutable_state: &'b MutableState<'a, T, Q>,
_identity_id: u64,
_values: &mut [LookupCell<'c, T>],
) -> Result<bool, EvalError<T>> {
unimplemented!("Direct lookup not supported by machine {}.", self.name())
}
fn identity_ids(&self) -> Vec<u64> {
self.connections.keys().copied().collect()
}

View File

@@ -23,7 +23,7 @@ use super::{
Constraints, EvalError, EvalValue, IncompleteCause, QueryCallback,
};
type Left<'a, T> = Vec<AffineExpression<AlgebraicVariable<'a>, T>>;
pub type Left<'a, T> = Vec<AffineExpression<AlgebraicVariable<'a>, T>>;
/// The data mutated by the processor
pub(crate) struct SolverState<'a, T: FieldElement> {