From 7ed65a2d32f936624c2b96644734925ba0c4e63f Mon Sep 17 00:00:00 2001 From: Kevin Jue Date: Fri, 13 Sep 2024 18:07:32 -0700 Subject: [PATCH 1/4] some clean up --- .../executor/src/events/precompiles/mod.rs | 94 ++++++++++++++++++- crates/core/executor/src/record.rs | 71 ++------------ crates/core/executor/src/utils.rs | 7 +- 3 files changed, 106 insertions(+), 66 deletions(-) diff --git a/crates/core/executor/src/events/precompiles/mod.rs b/crates/core/executor/src/events/precompiles/mod.rs index 9c44570dec..98ffed0154 100644 --- a/crates/core/executor/src/events/precompiles/mod.rs +++ b/crates/core/executor/src/events/precompiles/mod.rs @@ -9,42 +9,68 @@ mod uint256; pub use ec::*; pub use edwards::*; pub use fptower::*; +use hashbrown::HashMap; pub use keccak256_permute::*; use serde::{Deserialize, Serialize}; pub use sha256_compress::*; pub use sha256_extend::*; -use strum::EnumIter; +use strum::{EnumIter, IntoEnumIterator}; pub use uint256::*; +use crate::syscalls::SyscallCode; + use super::MemoryLocalEvent; #[derive(Clone, Debug, Serialize, Deserialize, EnumIter)] /// Precompile event. There should be one variant for every precompile syscall. pub enum PrecompileEvent { + /// Sha256 extend precompile event. ShaExtend(ShaExtendEvent), + /// Sha256 compress precompile event. ShaCompress(ShaCompressEvent), + /// Keccak256 permute precompile event. KeccakPermute(KeccakPermuteEvent), + /// Edwards curve add precompile event. EdAdd(EllipticCurveAddEvent), + /// Edwards curve decompress precompile event. EdDecompress(EdDecompressEvent), + /// Secp256k1 curve add precompile event. Secp256k1Add(EllipticCurveAddEvent), + /// Secp256k1 curve double precompile event. Secp256k1Double(EllipticCurveDoubleEvent), + /// Secp256k1 curve decompress precompile event. Secp256k1Decompress(EllipticCurveDecompressEvent), + /// K256 curve decompress precompile event. K256Decompress(EllipticCurveDecompressEvent), + /// Bn254 curve add precompile event. Bn254Add(EllipticCurveAddEvent), + /// Bn254 curve double precompile event. Bn254Double(EllipticCurveDoubleEvent), + /// Bn254 base field operation precompile event. Bn254Fp(FpOpEvent), + /// Bn254 quadratic field add/sub precompile event. Bn254Fp2AddSub(Fp2AddSubEvent), + /// Bn254 quadratic field mul precompile event. Bn254Fp2Mul(Fp2MulEvent), + /// Bls12-381 curve add precompile event. Bls12381Add(EllipticCurveAddEvent), + /// Bls12-381 curve double precompile event. Bls12381Double(EllipticCurveDoubleEvent), + /// Bls12-381 curve decompress precompile event. Bls12381Decompress(EllipticCurveDecompressEvent), + /// Bls12-381 base field operation precompile event. Bls12381Fp(FpOpEvent), + /// Bls12-381 quadratic field add/sub precompile event. Bls12381Fp2AddSub(Fp2AddSubEvent), + /// Bls12-381 quadratic field mul precompile event. Bls12381Fp2Mul(Fp2MulEvent), + /// Uint256 mul precompile event. Uint256Mul(Uint256MulEvent), } +/// Trait to retrieve all the local memory events from a vec of precompile events. pub trait PrecompileLocalMemory { + /// Get an iterator of all the local memory events. fn get_local_mem_events(&self) -> impl IntoIterator; } @@ -100,3 +126,69 @@ impl PrecompileLocalMemory for Vec { iterators.into_iter().flatten() } } + +/// A record of all the precompile events. +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct PrecompileEvents { + events: HashMap>, +} + +impl Default for PrecompileEvents { + fn default() -> Self { + let mut events = HashMap::new(); + for syscall_code in SyscallCode::iter() { + if syscall_code.should_send() == 1 { + events.insert(syscall_code, Vec::new()); + } + } + + Self { events } + } +} + +impl PrecompileEvents { + pub(crate) fn append(&mut self, other: &mut PrecompileEvents) { + for (syscall, events) in other.events.iter_mut() { + if !events.is_empty() { + self.events.entry(*syscall).or_default().append(events); + } + } + } + + #[inline] + /// Add a precompile event for a given syscall code. + pub(crate) fn add_event(&mut self, syscall_code: SyscallCode, event: PrecompileEvent) { + assert!(syscall_code.should_send() == 1); + self.events.entry(syscall_code).or_default().push(event); + } + + #[inline] + /// Insert a vector of precompile events for a given syscall code. + pub(crate) fn insert(&mut self, syscall_code: SyscallCode, events: Vec) { + assert!(syscall_code.should_send() == 1); + self.events.insert(syscall_code, events); + } + + #[inline] + pub(crate) fn into_iter(self) -> impl Iterator)> { + self.events.into_iter() + } + + #[inline] + /// Get all the precompile events for a given syscall code. + pub(crate) fn get_events(&self, syscall_code: SyscallCode) -> &Vec { + assert!(syscall_code.should_send() == 1); + &self.events[&syscall_code] + } + + /// Get all the local events from all the precompile events. + pub(crate) fn get_local_mem_events(&self) -> impl Iterator { + let mut iterators = Vec::new(); + + for (_, events) in self.events.iter() { + iterators.push(events.get_local_mem_events()); + } + + iterators.into_iter().flatten() + } +} diff --git a/crates/core/executor/src/record.rs b/crates/core/executor/src/record.rs index d7b3880401..f18a284d85 100644 --- a/crates/core/executor/src/record.rs +++ b/crates/core/executor/src/record.rs @@ -6,7 +6,6 @@ use sp1_stark::{ MachineRecord, SP1CoreOpts, SplitOpts, }; use std::{mem::take, sync::Arc}; -use strum::IntoEnumIterator; use serde::{Deserialize, Serialize}; @@ -15,7 +14,7 @@ use crate::{ events::{ add_sharded_byte_lookup_events, AluEvent, ByteLookupEvent, ByteRecord, CpuEvent, LookupId, MemoryInitializeFinalizeEvent, MemoryLocalEvent, MemoryRecordEnum, PrecompileEvent, - PrecompileLocalMemory, SyscallEvent, + PrecompileEvents, SyscallEvent, }, syscalls::SyscallCode, CoreShape, @@ -227,15 +226,20 @@ impl ExecutionRecord { } #[inline] + /// Add a precompile event to the execution record. pub fn add_precompile_event(&mut self, syscall_code: SyscallCode, event: PrecompileEvent) { - self.precompile_events.add_precompile_event(syscall_code, event) + self.precompile_events.add_event(syscall_code, event); } + /// Get all the precompile events for a syscall code. #[inline] + #[must_use] pub fn get_precompile_events(&self, syscall_code: SyscallCode) -> &Vec { self.precompile_events.get_events(syscall_code) } + /// Get all the local memory events. + #[inline] pub fn get_local_mem_events(&self) -> impl Iterator { let precompile_local_mem_events = self.precompile_events.get_local_mem_events(); precompile_local_mem_events.chain(self.cpu_local_memory_access.iter()) @@ -394,64 +398,3 @@ impl ByteRecord for ExecutionRecord { add_sharded_byte_lookup_events(&mut self.byte_lookups, new_events); } } - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct PrecompileEvents { - events: HashMap>, -} - -impl Default for PrecompileEvents { - fn default() -> Self { - let mut events = HashMap::new(); - for syscall_code in SyscallCode::iter() { - if syscall_code.should_send() == 1 { - events.insert(syscall_code, Vec::new()); - } - } - - Self { events } - } -} - -impl PrecompileEvents { - fn append(&mut self, other: &mut PrecompileEvents) { - for (syscall, events) in other.events.iter_mut() { - if !events.is_empty() { - self.events.entry(*syscall).or_default().append(events); - } - } - } - - #[inline] - fn add_precompile_event(&mut self, syscall_code: SyscallCode, event: PrecompileEvent) { - assert!(syscall_code.should_send() == 1); - self.events.entry(syscall_code).or_default().push(event); - } - - #[inline] - fn insert(&mut self, syscall_code: SyscallCode, events: Vec) { - assert!(syscall_code.should_send() == 1); - self.events.insert(syscall_code, events); - } - - #[inline] - fn into_iter(self) -> impl Iterator)> { - self.events.into_iter() - } - - #[inline] - fn get_events(&self, syscall_code: SyscallCode) -> &Vec { - assert!(syscall_code.should_send() == 1); - &self.events[&syscall_code] - } - - fn get_local_mem_events(&self) -> impl Iterator { - let mut iterators = Vec::new(); - - for (_, events) in self.events.iter() { - iterators.push(events.get_local_mem_events()); - } - - iterators.into_iter().flatten() - } -} diff --git a/crates/core/executor/src/utils.rs b/crates/core/executor/src/utils.rs index aece705076..ec4203aa50 100644 --- a/crates/core/executor/src/utils.rs +++ b/crates/core/executor/src/utils.rs @@ -5,6 +5,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use crate::Opcode; +/// Serialize a `HashMap` as a `Vec<(u32, V)>`. pub fn serialize_hashmap_as_vec( map: &HashMap>, serializer: S, @@ -12,6 +13,7 @@ pub fn serialize_hashmap_as_vec( Serialize::serialize(&map.iter().collect::>(), serializer) } +/// Deserialize a `Vec<(u32, V)>` as a `HashMap`. pub fn deserialize_hashmap_as_vec<'de, V: Deserialize<'de>, D: Deserializer<'de>>( deserializer: D, ) -> Result>, D::Error> { @@ -20,11 +22,13 @@ pub fn deserialize_hashmap_as_vec<'de, V: Deserialize<'de>, D: Deserializer<'de> } /// Returns `true` if the given `opcode` is a signed operation. +#[must_use] pub fn is_signed_operation(opcode: Opcode) -> bool { opcode == Opcode::DIV || opcode == Opcode::REM } /// Calculate the correct `quotient` and `remainder` for the given `b` and `c` per RISC-V spec. +#[must_use] pub fn get_quotient_and_remainder(b: u32, c: u32, opcode: Opcode) -> (u32, u32) { if c == 0 { // When c is 0, the quotient is 2^32 - 1 and the remainder is b regardless of whether we @@ -33,11 +37,12 @@ pub fn get_quotient_and_remainder(b: u32, c: u32, opcode: Opcode) -> (u32, u32) } else if is_signed_operation(opcode) { ((b as i32).wrapping_div(c as i32) as u32, (b as i32).wrapping_rem(c as i32) as u32) } else { - ((b as u32).wrapping_div(c as u32) as u32, (b as u32).wrapping_rem(c as u32) as u32) + (b.wrapping_div(c), b.wrapping_rem(c)) } } /// Calculate the most significant bit of the given 32-bit integer `a`, and returns it as a u8. +#[must_use] pub const fn get_msb(a: u32) -> u8 { ((a >> 31) & 1) as u8 } From 175afa0b4d6653bace42ba4012b550a1ad07207f Mon Sep 17 00:00:00 2001 From: Kevin Jue Date: Fri, 13 Sep 2024 18:16:29 -0700 Subject: [PATCH 2/4] more cleanup --- .../executor/src/events/precompiles/mod.rs | 5 +++ crates/core/executor/src/record.rs | 31 +++---------------- 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/crates/core/executor/src/events/precompiles/mod.rs b/crates/core/executor/src/events/precompiles/mod.rs index 98ffed0154..8acfed9e60 100644 --- a/crates/core/executor/src/events/precompiles/mod.rs +++ b/crates/core/executor/src/events/precompiles/mod.rs @@ -174,6 +174,11 @@ impl PrecompileEvents { self.events.into_iter() } + #[inline] + pub(crate) fn iter(&self) -> impl Iterator)> { + self.events.iter() + } + #[inline] /// Get all the precompile events for a given syscall code. pub(crate) fn get_events(&self, syscall_code: SyscallCode) -> &Vec { diff --git a/crates/core/executor/src/record.rs b/crates/core/executor/src/record.rs index f18a284d85..2d302db4b7 100644 --- a/crates/core/executor/src/record.rs +++ b/crates/core/executor/src/record.rs @@ -273,32 +273,11 @@ impl MachineRecord for ExecutionRecord { stats.insert("shift_right_events".to_string(), self.shift_right_events.len()); stats.insert("divrem_events".to_string(), self.divrem_events.len()); stats.insert("lt_events".to_string(), self.lt_events.len()); - // stats.insert("sha_extend_events".to_string(), self.sha_extend_events.len()); - // stats.insert("sha_compress_events".to_string(), self.sha_compress_events.len()); - // stats.insert("keccak_permute_events".to_string(), self.keccak_permute_events.len()); - // stats.insert("ed_add_events".to_string(), self.ed_add_events.len()); - // stats.insert("ed_decompress_events".to_string(), self.ed_decompress_events.len()); - // stats.insert("secp256k1_add_events".to_string(), self.secp256k1_add_events.len()); - // stats.insert("secp256k1_double_events".to_string(), self.secp256k1_double_events.len()); - // stats.insert("bn254_add_events".to_string(), self.bn254_add_events.len()); - // stats.insert("bn254_double_events".to_string(), self.bn254_double_events.len()); - // stats.insert("k256_decompress_events".to_string(), self.k256_decompress_events.len()); - // stats.insert("bls12381_add_events".to_string(), self.bls12381_add_events.len()); - // stats.insert("bls12381_double_events".to_string(), self.bls12381_double_events.len()); - // stats.insert("uint256_mul_events".to_string(), self.uint256_mul_events.len()); - // stats.insert("bls12381_fp_event".to_string(), self.bls12381_fp_events.len()); - // stats.insert( - // "bls12381_fp2_addsub_events".to_string(), - // self.bls12381_fp2_addsub_events.len(), - // ); - // stats.insert("bls12381_fp2_mul_events".to_string(), self.bls12381_fp2_mul_events.len()); - // stats.insert("bn254_fp_events".to_string(), self.bn254_fp_events.len()); - // stats.insert("bn254_fp2_addsub_events".to_string(), self.bn254_fp2_addsub_events.len()); - // stats.insert("bn254_fp2_mul_events".to_string(), self.bn254_fp2_mul_events.len()); - // stats.insert( - // "bls12381_decompress_events".to_string(), - // self.bls12381_decompress_events.len(), - // ); + + for (syscall_code, events) in self.precompile_events.iter() { + stats.insert(format!("syscall {syscall_code:?}"), events.len()); + } + stats.insert( "global_memory_initialize_events".to_string(), self.global_memory_initialize_events.len(), From 4ece415320bcab8f800cbeed7c4adeb9e1d21240 Mon Sep 17 00:00:00 2001 From: Kevin Jue Date: Fri, 13 Sep 2024 18:19:21 -0700 Subject: [PATCH 3/4] last clippy issue --- crates/stark/src/prover.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/stark/src/prover.rs b/crates/stark/src/prover.rs index ff27f6a1d4..ee8159fe13 100644 --- a/crates/stark/src/prover.rs +++ b/crates/stark/src/prover.rs @@ -28,6 +28,7 @@ use crate::{ pub struct MergedProverDataItem<'a, M> { /// The trace. pub trace: &'a M, + /// The main data index. pub main_data_idx: usize, } From bf69f54dfc831245a08b66a286f5a9d11c1544b9 Mon Sep 17 00:00:00 2001 From: Kevin Jue Date: Fri, 13 Sep 2024 18:51:23 -0700 Subject: [PATCH 4/4] some cleanup --- .../precompiles/edwards/decompress.rs | 1 - crates/core/machine/src/riscv/cost.rs | 1 - .../machine/src/syscall/precompiles/README.md | 80 +++++-------------- crates/sdk/src/provers/cpu.rs | 1 - crates/stark/src/prover.rs | 1 - 5 files changed, 22 insertions(+), 62 deletions(-) diff --git a/crates/core/executor/src/syscalls/precompiles/edwards/decompress.rs b/crates/core/executor/src/syscalls/precompiles/edwards/decompress.rs index 1f2f0429b8..90884a91bb 100644 --- a/crates/core/executor/src/syscalls/precompiles/edwards/decompress.rs +++ b/crates/core/executor/src/syscalls/precompiles/edwards/decompress.rs @@ -38,7 +38,6 @@ impl Syscall for EdwardsDecompressSyscall { let (y_memory_records_vec, y_vec) = rt.mr_slice(slice_ptr + (COMPRESSED_POINT_BYTES as u32), WORDS_FIELD_ELEMENT); - let y_memory_records: [MemoryReadRecord; 8] = y_memory_records_vec.try_into().unwrap(); let sign_bool = sign != 0; diff --git a/crates/core/machine/src/riscv/cost.rs b/crates/core/machine/src/riscv/cost.rs index b93c5aa521..6f81ce9255 100644 --- a/crates/core/machine/src/riscv/cost.rs +++ b/crates/core/machine/src/riscv/cost.rs @@ -135,7 +135,6 @@ impl CostEstimator for ExecutionReport { + self.opcode_counts[Opcode::REM] + self.opcode_counts[Opcode::DIVU] + self.opcode_counts[Opcode::REMU]; - total_area += (divrem_events as u64) * costs[&RiscvAirDiscriminants::DivRem]; total_chips += 1; diff --git a/crates/core/machine/src/syscall/precompiles/README.md b/crates/core/machine/src/syscall/precompiles/README.md index 68f732e007..1b7c645dd5 100644 --- a/crates/core/machine/src/syscall/precompiles/README.md +++ b/crates/core/machine/src/syscall/precompiles/README.md @@ -46,7 +46,7 @@ impl Syscall for Uint256MulChip { 1 } - fn execute(&self, rt: &mut SyscallContext, arg1: u32, arg2: u32) -> Option { + fn execute(&self, rt: &mut SyscallContext, syscall: SyscallCode, arg1: u32, arg2: u32) -> Option { // Your execution logic here // Parse input pointers, perform the multiplication, and write the result } @@ -79,76 +79,40 @@ impl MachineAir for CustomOpChip { } } ``` -You will also have to update `core/src/runtime/record.rs` accordingly to handle these new events. +You will also have to update `core/executor/src/events/precompiles/mod.rs` accordingly to register the new precompile op. #### Add a new field for your chip's events -In the `ExecutionRecord` struct, add a new field to track events specific to your chip. This field will store all events generated by your chip during execution. +In the `PrecompileEvent` enum, add a new variant for you precompile op. ```rust -#[derive(Default, Clone, Debug, Serialize, Deserialize)] -pub struct ExecutionRecord { - // Other existing fields... +#[derive(Clone, Debug, Serialize, Deserialize, EnumIter)] +/// Precompile event. There should be one variant for every precompile syscall. +pub enum PrecompileEvent { + // Other existing variants... - /// A trace of the events for your custom operation. - pub custom_op_events: Vec, + /// A variant for your custom operation. + pub CustomOp(CustomOpEvent), } ``` -#### Update the `stats` method -In the `stats` method, add an entry to track the number of events associated with your chip. +#### Update the `get_local_mem_events` method +In the `get_local_mem_events` method, add your variant to the match statement to add an iterator of the op's local +memory events (if it has local memory events). ```rust -fn stats(&self) -> HashMap { - let mut stats = HashMap::new(); - // Other existing entries... +fn get_local_mem_events(&self) -> impl IntoIterator { + let mut iterators = Vec::new(); - stats.insert("custom_op_events".to_string(), self.custom_op_events.len()); - // Add other stats as necessary - stats -} -``` -#### Update the `append` method -In the append method, ensure that events from your chip are correctly appended when merging two `ExecutionRecord` instances. - -```rust -fn append(&mut self, other: &mut ExecutionRecord) { - // Other existing append operations... - - self.custom_op_events.append(&mut other.custom_op_events); -} -``` - -#### Update the `defer` method -Modify the `defer` method to handle the deferring of events specific to your chip. - -```rust -pub fn defer(&mut self) -> ExecutionRecord { - ExecutionRecord { - // Other deferred events... + for event in self.iter() { + match event { + // Other existing variants... - custom_op_events: std::mem::take(&mut self.custom_op_events), - ..Default::default() + PrecompileEvent::CustomOp(e) => { + iterators.push(e.local_mem_access.iter()); + } + } } -} -``` - -#### Update the `split` method -In the `split` method, ensure that events associated with your chip are properly split when distributing deferred events across shards. - -```rust -pub fn split(&mut self, last: bool, opts: SplitOpts) -> Vec { - let mut shards = Vec::new(); - - split_events!( - self, - custom_op_events, - shards, - opts.deferred_shift_threshold, - last - ); - - // Other event splits... - shards + iterators.into_iter().flatten() } ``` diff --git a/crates/sdk/src/provers/cpu.rs b/crates/sdk/src/provers/cpu.rs index d512d47041..891c34459c 100644 --- a/crates/sdk/src/provers/cpu.rs +++ b/crates/sdk/src/provers/cpu.rs @@ -63,7 +63,6 @@ impl Prover for CpuProver { let deferred_proofs = stdin.proofs.iter().map(|(reduce_proof, _)| reduce_proof.clone()).collect(); - let public_values = proof.public_values.clone(); // Generate the compressed proof. diff --git a/crates/stark/src/prover.rs b/crates/stark/src/prover.rs index ee8159fe13..30b36453ca 100644 --- a/crates/stark/src/prover.rs +++ b/crates/stark/src/prover.rs @@ -85,7 +85,6 @@ pub trait MachineProver>: chip_name, begin.elapsed() ); - (chip_name, trace) }) .collect::>()