From 12aca0e136cb39db408e47f9e67f5e50251e265c Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 12 Dec 2024 18:56:54 +0100 Subject: [PATCH] Various preparatory changes. (#2228) --- .../witgen/data_structures/mutable_state.rs | 38 +++++++++++++------ executor/src/witgen/machines/mod.rs | 14 +++++++ jit-compiler/src/compiler.rs | 4 +- jit-compiler/src/lib.rs | 2 +- number/src/goldilocks.rs | 4 ++ number/src/macros.rs | 4 ++ number/src/plonky3_macros.rs | 5 +++ number/src/traits.rs | 6 +++ 8 files changed, 63 insertions(+), 14 deletions(-) diff --git a/executor/src/witgen/data_structures/mutable_state.rs b/executor/src/witgen/data_structures/mutable_state.rs index 384329eb32..904a73e1e3 100644 --- a/executor/src/witgen/data_structures/mutable_state.rs +++ b/executor/src/witgen/data_structures/mutable_state.rs @@ -1,12 +1,12 @@ use std::{ - cell::RefCell, + cell::{RefCell, RefMut}, collections::{BTreeMap, HashMap}, }; use powdr_number::FieldElement; use crate::witgen::{ - machines::{KnownMachine, Machine}, + machines::{KnownMachine, LookupCell, Machine}, rows::RowPair, EvalError, EvalResult, QueryCallback, }; @@ -52,19 +52,35 @@ impl<'a, T: FieldElement, Q: QueryCallback> MutableState<'a, T, Q> { /// Call the machine responsible for the right-hand-side of an identity given its ID /// and the row pair of the caller. pub fn call(&self, identity_id: u64, caller_rows: &RowPair<'_, 'a, T>) -> EvalResult<'a, T> { + self.responsible_machine(identity_id)? + .process_plookup_timed(self, identity_id, caller_rows) + } + + /// Call the machine responsible for the right-hand-side of an identity given its ID, + /// use the direct interface. + #[allow(unused)] + pub fn call_direct( + &self, + identity_id: u64, + values: &mut [LookupCell<'_, T>], + ) -> Result> { + self.responsible_machine(identity_id)? + .process_lookup_direct_timed(self, identity_id, values) + } + + fn responsible_machine( + &self, + identity_id: u64, + ) -> Result>, EvalError> { let machine_index = *self .identity_to_machine_index .get(&identity_id) .unwrap_or_else(|| panic!("No executor machine matched identity ID: {identity_id}")); - - self.machines[machine_index] - .try_borrow_mut() - .map_err(|_| { - EvalError::RecursiveMachineCalls(format!( - "Detected when processing identity with ID {identity_id}" - )) - })? - .process_plookup_timed(self, identity_id, caller_rows) + self.machines[machine_index].try_borrow_mut().map_err(|_| { + EvalError::RecursiveMachineCalls(format!( + "Detected when processing identity with ID {identity_id}" + )) + }) } /// Extracts the witness column values from the machines. diff --git a/executor/src/witgen/machines/mod.rs b/executor/src/witgen/machines/mod.rs index d632e7a6e1..9f1ed714f2 100644 --- a/executor/src/witgen/machines/mod.rs +++ b/executor/src/witgen/machines/mod.rs @@ -66,6 +66,19 @@ pub trait Machine<'a, T: FieldElement>: Send + Sync { result } + /// Like 'process_lookup_direct', but also records the time spent in this machine. + fn process_lookup_direct_timed<'b, 'c, Q: QueryCallback>( + &mut self, + mutable_state: &'b MutableState<'a, T, Q>, + identity_id: u64, + values: &mut [LookupCell<'c, T>], + ) -> Result> { + record_start(self.name()); + let result = self.process_lookup_direct(mutable_state, identity_id, values); + record_end(self.name()); + result + } + /// Returns a unique name for this machine. fn name(&self) -> &str; @@ -106,6 +119,7 @@ pub trait Machine<'a, T: FieldElement>: Send + Sync { fn identity_ids(&self) -> Vec; } +#[repr(C)] pub enum LookupCell<'a, T> { /// Value is known (i.e. an input) Input(&'a T), diff --git a/jit-compiler/src/compiler.rs b/jit-compiler/src/compiler.rs index a5fbb4348a..e6f73db9e8 100644 --- a/jit-compiler/src/compiler.rs +++ b/jit-compiler/src/compiler.rs @@ -218,10 +218,10 @@ pub fn call_cargo(code: &str) -> Result { if log::log_enabled!(log::Level::Debug) { let stderr = from_utf8(&out.stderr).unwrap_or("UTF-8 error in error message."); return Err(format!( - "Rust compiler error when JIT-compiling. Will use evaluator for all symbols. Error message:\n{stderr}." + "Rust compiler error when JIT-compiling. Will use interpreter instead. Error message:\n{stderr}." )); } else { - return Err("Rust compiler error when JIT-compiling. Will use evaluator for all symbols. Set log level to DEBUG for reason.".to_string()); + return Err("Rust compiler error when JIT-compiling. Will use interpreter instead. Set log level to DEBUG for reason.".to_string()); } } let extension = if cfg!(target_os = "windows") { diff --git a/jit-compiler/src/lib.rs b/jit-compiler/src/lib.rs index 1e331d487a..d0fa4c13d8 100644 --- a/jit-compiler/src/lib.rs +++ b/jit-compiler/src/lib.rs @@ -73,7 +73,7 @@ pub fn compile( let successful_hash = successful_symbol_names.iter().collect::>(); // TODO this should be changed back to Info after the introduction of the ToCol trait. log::debug!( - "Unable to generate code during JIT-compilation for the following symbols. Will use evaluator instead.\n{}", + "Unable to generate code during JIT-compilation for the following symbols. Will use interpreter instead.\n{}", requested_symbols .iter() .filter(|&sym| !successful_hash.contains(sym)) diff --git a/number/src/goldilocks.rs b/number/src/goldilocks.rs index 375dae1fc1..3503b71b83 100644 --- a/number/src/goldilocks.rs +++ b/number/src/goldilocks.rs @@ -398,6 +398,10 @@ impl FieldElement for GoldilocksField { // Undo the shift Some(v.wrapping_sub(SHIFT as u32) as i32) } + + fn has_direct_repr() -> bool { + true + } } impl LowerHex for GoldilocksField { diff --git a/number/src/macros.rs b/number/src/macros.rs index d50d49fbbd..b8c9877337 100644 --- a/number/src/macros.rs +++ b/number/src/macros.rs @@ -400,6 +400,10 @@ macro_rules! powdr_field { // Undo the shift Some(v.wrapping_sub(SHIFT as u32) as i32) } + + fn has_direct_repr() -> bool { + false + } } impl From<$ark_type> for $name { diff --git a/number/src/plonky3_macros.rs b/number/src/plonky3_macros.rs index 988fa09802..9bd09af63f 100644 --- a/number/src/plonky3_macros.rs +++ b/number/src/plonky3_macros.rs @@ -117,6 +117,11 @@ macro_rules! powdr_field_plonky3 { fn try_into_i32(&self) -> Option { Some(self.to_canonical_u32() as i32) } + + fn has_direct_repr() -> bool { + // No direct repr, because 'mod' is not always applied. + false + } } impl LowerHex for $name { diff --git a/number/src/traits.rs b/number/src/traits.rs index d15672480b..55809d1ad1 100644 --- a/number/src/traits.rs +++ b/number/src/traits.rs @@ -185,6 +185,12 @@ pub trait FieldElement: /// As conventional, negative values are in relation to 0 in the field. /// Returns None if out of the range [0 - 2^31, 2^31). fn try_into_i32(&self) -> Option; + + /// Returns `true` if values of this type are directly stored as their integer + /// value (i.e. not in montgomery representation and there are also no + /// additional fields), i.e. the `to_integer` function can be implemented as + /// a mem::transmute operation on pointers. + fn has_direct_repr() -> bool; } #[cfg(test)]