From 68f9932b6b9311fbd825bd9c497671b466b770ee Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Mon, 13 Jan 2025 18:18:25 -0300 Subject: [PATCH 1/9] add a new ExecMode::Witness to the riscv executor ExecMode::Trace should only produce the memory and registers trace --- cli-rs/src/main.rs | 1 + riscv-executor/src/lib.rs | 84 +++++++++++++++++++++++++------------- riscv/src/continuations.rs | 2 + riscv/tests/common/mod.rs | 1 + 4 files changed, 59 insertions(+), 29 deletions(-) diff --git a/cli-rs/src/main.rs b/cli-rs/src/main.rs index 79f873603b..4ae18fb154 100644 --- a/cli-rs/src/main.rs +++ b/cli-rs/src/main.rs @@ -444,6 +444,7 @@ fn execute( pipeline.data_callback().unwrap(), &[], None, + true, profiling, ); diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index 6d750ad152..9b6d23da66 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -724,7 +724,7 @@ mod builder { regs[pc_idx as usize] = PC_INITIAL_VAL.into(); let submachines: HashMap<_, RefCell>>> = - if let ExecMode::Trace = mode { + if let ExecMode::Witness = mode { [ ( MachineInstance::memory, @@ -797,7 +797,7 @@ mod builder { lookup_args: &[F], extra: &[F], ) { - if let ExecMode::Trace = self.mode { + if let ExecMode::Witness = self.mode { self.trace .submachine_ops .get_mut(m as usize) @@ -884,31 +884,30 @@ mod builder { /// raw set next value of register by register index instead of name fn set_reg_idx(&mut self, idx: u16, value: Elem) { // Record register write in trace. Only for non-pc, non-assignment registers. - if let ExecMode::Trace = self.mode { + if let ExecMode::Trace | ExecMode::Witness = self.mode { if idx != self.pc_idx { self.trace.reg_writes[idx as usize] = Some(value.into_fe()); } } - self.regs[idx as usize] = value; } pub fn set_col_idx(&mut self, col: KnownWitnessCol, idx: usize, value: Elem) { - if let ExecMode::Trace = self.mode { + if let ExecMode::Witness = self.mode { let idx = (KnownWitnessCol::count() * idx) + col as usize; *self.trace.known_cols.get_mut(idx).unwrap() = value.into_fe(); } } pub fn set_col(&mut self, col: KnownWitnessCol, value: Elem) { - if let ExecMode::Trace = self.mode { + if let ExecMode::Witness = self.mode { let idx = (self.trace.known_cols.len() - KnownWitnessCol::count()) + col as usize; *self.trace.known_cols.get_mut(idx).unwrap() = value.into_fe(); } } pub fn push_row(&mut self, pc: u32) { - if let ExecMode::Trace = self.mode { + if let ExecMode::Witness = self.mode { let new_len = self.trace.known_cols.len() + KnownWitnessCol::count(); self.trace.known_cols.resize(new_len, F::zero()); self.trace.pc_trace.push(pc); @@ -960,6 +959,8 @@ mod builder { &[1.into(), addr.into(), step.into(), val.into()], &[], ); + } + if let ExecMode::Trace | ExecMode::Witness = self.mode { self.trace.mem_ops.push(MemOperation { row: self.trace.len, kind: MemOperationKind::Write, @@ -972,13 +973,15 @@ mod builder { pub(crate) fn get_mem(&mut self, addr: u32, step: u32, identity_id: u64) -> u32 { let val = *self.mem.get(&addr).unwrap_or(&0); - if let ExecMode::Trace = self.mode { + if let ExecMode::Witness = self.mode { self.submachine_op( MachineInstance::memory, identity_id, &[0.into(), addr.into(), step.into(), val.into()], &[], ); + } + if let ExecMode::Trace | ExecMode::Witness = self.mode { self.trace.mem_ops.push(MemOperation { row: self.trace.len, kind: MemOperationKind::Read, @@ -1041,6 +1044,17 @@ mod builder { start.elapsed().as_secs_f64(), ); + // trace mode doesn't generate a full witness + if let ExecMode::Trace = self.mode { + return Execution { + trace_len: self.trace.len, + memory: self.mem, + memory_accesses: std::mem::take(&mut self.trace.mem_ops), + trace: main_regs.into_iter().collect(), + register_memory: self.reg_mem.for_bootloader(), + }; + } + // This hashmap will be added to until we get the full witness. let mut cols = self.generate_main_columns(); @@ -1307,7 +1321,7 @@ impl Executor<'_, '_, F> { fn init(&mut self) { self.step = 4; - if let ExecMode::Trace = self.mode { + if let ExecMode::Witness = self.mode { for c in KnownFixedCol::all() { self.cached_fixed_cols .push(self.get_fixed(c.name()).unwrap().clone()); @@ -1384,30 +1398,33 @@ impl Executor<'_, '_, F> { /// Gets the identity id for a link associated with a given instruction. /// idx is based on the order link appear in the assembly (assumed to be the same in the optimized pil). fn instr_link_id(&mut self, instr: Instruction, target: MachineInstance, idx: usize) -> u64 { - if let ExecMode::Fast = self.mode { - return 0; // we don't care about identity ids in fast mode + if let ExecMode::Witness = self.mode { + let entries = self + .pil_instruction_links + .get_mut(instr as usize * MachineInstance::count() + target as usize) + .unwrap() + .get_or_insert_with(|| { + pil::find_instruction_links(&self.pil_links, instr.flag(), target.namespace()) + }); + entries.get(idx).unwrap().id() + } else { + // we don't care about identity ids in non witness mode + 0 } - - let entries = self - .pil_instruction_links - .get_mut(instr as usize * MachineInstance::count() + target as usize) - .unwrap() - .get_or_insert_with(|| { - pil::find_instruction_links(&self.pil_links, instr.flag(), target.namespace()) - }); - entries.get(idx).unwrap().id() } /// Find the identity id of a link. fn link_id(&mut self, from: &'static str, target: &'static str, idx: usize) -> u64 { - if let ExecMode::Fast = self.mode { - return 0; // we don't care about identity ids in fast mode + if let ExecMode::Witness = self.mode { + let entries = self + .pil_other_links + .entry((from, target)) + .or_insert_with(|| pil::find_links(&self.pil_links, from, target)); + entries.get(idx).unwrap().id() + } else { + // we don't care about identity ids in fast mode + 0 } - let entries = self - .pil_other_links - .entry((from, target)) - .or_insert_with(|| pil::find_links(&self.pil_links, from, target)); - entries.get(idx).unwrap().id() } fn exec_instruction(&mut self, name: &str, args: &[Expression]) -> Option> { @@ -1420,7 +1437,7 @@ impl Executor<'_, '_, F> { macro_rules! get_fixed { ($name:ident) => { - if let ExecMode::Trace = self.mode { + if let ExecMode::Witness = self.mode { Elem::Field( self.get_known_fixed(KnownFixedCol::$name, self.proc.get_pc().u() as usize), ) @@ -2816,8 +2833,12 @@ pub struct Execution { #[derive(Clone, Copy)] enum ExecMode { + /// Doesn't generate any execution information besides the length of the trace Fast, + /// Generate traces for memory and powdr asm registers Trace, + /// Generate the full witness + Witness, } /// Execute a Powdr/RISCV assembly program, without generating a witness. @@ -2854,6 +2875,7 @@ pub fn execute( prover_ctx: &Callback, bootloader_inputs: &[F], max_steps_to_execute: Option, + full_witness: bool, profiling: Option, ) -> Execution { log::info!("Executing (trace generation)..."); @@ -2866,7 +2888,11 @@ pub fn execute( prover_ctx, bootloader_inputs, max_steps_to_execute.unwrap_or(usize::MAX), - ExecMode::Trace, + if full_witness { + ExecMode::Witness + } else { + ExecMode::Trace + }, profiling, ) } diff --git a/riscv/src/continuations.rs b/riscv/src/continuations.rs index d6b8a68072..a89f4ddd10 100644 --- a/riscv/src/continuations.rs +++ b/riscv/src/continuations.rs @@ -332,6 +332,7 @@ pub fn rust_continuations_dry_run( // we only know them after the full trace has been generated. &default_input(&[]), None, + false, profiler_opt, ); @@ -436,6 +437,7 @@ pub fn rust_continuations_dry_run( pipeline.data_callback().unwrap(), &bootloader_inputs, Some(num_rows), + false, // profiling was done when full trace was generated None, ); diff --git a/riscv/tests/common/mod.rs b/riscv/tests/common/mod.rs index a5ed2269a6..af6ed8e61b 100644 --- a/riscv/tests/common/mod.rs +++ b/riscv/tests/common/mod.rs @@ -71,6 +71,7 @@ pub fn verify_riscv_asm_string Date: Tue, 14 Jan 2025 09:40:55 -0300 Subject: [PATCH 2/9] fix --- riscv-executor/src/lib.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index 9b6d23da66..359a7a8cfb 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -952,7 +952,7 @@ mod builder { } pub(crate) fn set_mem(&mut self, addr: u32, val: u32, step: u32, identity_id: u64) { - if let ExecMode::Trace = self.mode { + if let ExecMode::Witness = self.mode { self.submachine_op( MachineInstance::memory, identity_id, @@ -3137,7 +3137,7 @@ fn execute_inner( log::debug!("Program execution took {}s", start.elapsed().as_secs_f64()); - if let ExecMode::Trace = mode { + if let ExecMode::Trace | ExecMode::Witness = mode { let sink_id = e.sink_id(); // reset @@ -3164,12 +3164,14 @@ fn execute_inner( e.proc .set_col(KnownWitnessCol::_operation_id, sink_id.into()); - let start = Instant::now(); - program_columns = e.generate_program_columns(); - log::debug!( - "Generating program columns took {}s", - start.elapsed().as_secs_f64() - ); + if let ExecMode::Witness = mode { + let start = Instant::now(); + program_columns = e.generate_program_columns(); + log::debug!( + "Generating program columns took {}s", + start.elapsed().as_secs_f64() + ); + } } e.proc.finish(opt_pil, program_columns) From eb33770b5735b7eacbf7ea8b2d9186d5519aef15 Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Tue, 14 Jan 2025 10:10:55 -0300 Subject: [PATCH 3/9] fix --- riscv-executor/src/lib.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index 359a7a8cfb..b7b73f13ca 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -814,7 +814,9 @@ mod builder { let cols_len = self.trace.known_cols.len() / KnownWitnessCol::count(); // sanity check - assert!(self.trace.len <= cols_len); + if let ExecMode::Witness = self.mode { + assert!(self.trace.len <= cols_len); + } cols_len } @@ -1027,14 +1029,6 @@ mod builder { let pil = opt_pil.unwrap(); - let main_degree = { - let range = namespace_degree_range(pil, "main"); - std::cmp::max( - self.main_columns_len().next_power_of_two() as u32, - range.min as u32, - ) - }; - let start = Instant::now(); // turn register write operations into witness columns @@ -1055,6 +1049,14 @@ mod builder { }; } + let main_degree = { + let range = namespace_degree_range(pil, "main"); + std::cmp::max( + self.main_columns_len().next_power_of_two() as u32, + range.min as u32, + ) + }; + // This hashmap will be added to until we get the full witness. let mut cols = self.generate_main_columns(); From 4998074e31857eca3f5db09bd3f88a38fc27642e Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Tue, 14 Jan 2025 10:23:24 -0300 Subject: [PATCH 4/9] fix --- riscv-executor/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index b7b73f13ca..acc346ba5b 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -909,9 +909,7 @@ mod builder { } pub fn push_row(&mut self, pc: u32) { - if let ExecMode::Witness = self.mode { - let new_len = self.trace.known_cols.len() + KnownWitnessCol::count(); - self.trace.known_cols.resize(new_len, F::zero()); + if let ExecMode::Trace | ExecMode::Witness = self.mode { self.trace.pc_trace.push(pc); self.trace .reg_trace @@ -926,6 +924,10 @@ mod builder { } }); } + if let ExecMode::Witness = self.mode { + let new_len = self.trace.known_cols.len() + KnownWitnessCol::count(); + self.trace.known_cols.resize(new_len, F::zero()); + } } /// advance to next row, returns the index to the statement that must be From c895fa8de944aee1202d906bc1df3f5bd87f7cbf Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Tue, 14 Jan 2025 10:43:28 -0300 Subject: [PATCH 5/9] extend reg trace to next power of 2 --- riscv-executor/src/lib.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index acc346ba5b..1fdc57c4bc 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -1034,36 +1034,40 @@ mod builder { let start = Instant::now(); // turn register write operations into witness columns - let main_regs = self.trace.generate_registers_trace(); + let mut cols = self + .trace + .generate_registers_trace() + .into_iter() + .collect::>(); log::debug!( "Generating register traces took {}s", start.elapsed().as_secs_f64(), ); + let main_degree = { + let range = namespace_degree_range(pil, "main"); + std::cmp::max(self.trace.len.next_power_of_two() as u32, range.min as u32) + }; + + // fill up main trace to degree + cols.values_mut().for_each(|v| { + let last = *v.last().unwrap(); + v.resize(main_degree as usize, last); + }); + // trace mode doesn't generate a full witness if let ExecMode::Trace = self.mode { return Execution { trace_len: self.trace.len, memory: self.mem, memory_accesses: std::mem::take(&mut self.trace.mem_ops), - trace: main_regs.into_iter().collect(), + trace: cols, register_memory: self.reg_mem.for_bootloader(), }; } - let main_degree = { - let range = namespace_degree_range(pil, "main"); - std::cmp::max( - self.main_columns_len().next_power_of_two() as u32, - range.min as u32, - ) - }; - - // This hashmap will be added to until we get the full witness. - let mut cols = self.generate_main_columns(); - // add reg columns to trace - cols.extend(main_regs); + cols.extend(self.generate_main_columns()); // sanity check that program columns and main trace have the same length assert_eq!( From 677d5e9f05d9c5e482210942c095b6e5b459a411 Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Tue, 14 Jan 2025 10:44:26 -0300 Subject: [PATCH 6/9] comment fix --- riscv-executor/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index 1fdc57c4bc..5750d5a777 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -1049,7 +1049,7 @@ mod builder { std::cmp::max(self.trace.len.next_power_of_two() as u32, range.min as u32) }; - // fill up main trace to degree + // fill up reg trace to degree cols.values_mut().for_each(|v| { let last = *v.last().unwrap(); v.resize(main_degree as usize, last); From 8e3600f85c333456a5a9c9bba2989e7ec5e4ce2c Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Tue, 14 Jan 2025 11:32:32 -0300 Subject: [PATCH 7/9] remove invalid check --- riscv-executor/src/lib.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index 5750d5a777..973133f4ce 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -1069,12 +1069,6 @@ mod builder { // add reg columns to trace cols.extend(self.generate_main_columns()); - // sanity check that program columns and main trace have the same length - assert_eq!( - cols.values().next().unwrap().len(), - program_columns[0].1.len(), - ); - // add program columns to main trace cols.extend(program_columns); From e4d6977c49b2a877d24b43797b31b60581b3bafa Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Thu, 16 Jan 2025 08:55:22 -0300 Subject: [PATCH 8/9] remove bool parameter --- cli-rs/src/main.rs | 5 ++--- powdr/src/lib.rs | 2 +- riscv-executor/src/lib.rs | 40 +++++++++++++++++++++++++++++--------- riscv/src/continuations.rs | 6 ++---- riscv/tests/common/mod.rs | 5 ++--- riscv/tests/riscv.rs | 2 +- 6 files changed, 39 insertions(+), 21 deletions(-) diff --git a/cli-rs/src/main.rs b/cli-rs/src/main.rs index 4ae18fb154..269633c175 100644 --- a/cli-rs/src/main.rs +++ b/cli-rs/src/main.rs @@ -392,7 +392,7 @@ fn execute_fast( let start = Instant::now(); - let trace_len = powdr::riscv_executor::execute_fast::( + let trace_len = powdr::riscv_executor::execute::( &asm, powdr::riscv_executor::MemoryState::new(), pipeline.data_callback().unwrap(), @@ -436,7 +436,7 @@ fn execute( let start = Instant::now(); - let execution = powdr::riscv_executor::execute::( + let execution = powdr::riscv_executor::execute_with_witness::( &asm, &pil, fixed, @@ -444,7 +444,6 @@ fn execute( pipeline.data_callback().unwrap(), &[], None, - true, profiling, ); diff --git a/powdr/src/lib.rs b/powdr/src/lib.rs index b6997c6309..67ce734496 100644 --- a/powdr/src/lib.rs +++ b/powdr/src/lib.rs @@ -312,7 +312,7 @@ fn run_internal( let asm = pipeline.compute_analyzed_asm().unwrap().clone(); let initial_memory = riscv::continuations::load_initial_memory(&asm, pipeline.initial_memory()); - let trace_len = riscv_executor::execute_fast( + let trace_len = riscv_executor::execute( &asm, initial_memory, pipeline.data_callback().unwrap(), diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index 973133f4ce..b1a852b4ab 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -2845,7 +2845,7 @@ enum ExecMode { /// Execute a Powdr/RISCV assembly program, without generating a witness. /// Returns the execution trace length. -pub fn execute_fast( +pub fn execute( asm: &AnalysisASMFile, initial_memory: MemoryState, prover_ctx: &Callback, @@ -2867,9 +2867,9 @@ pub fn execute_fast( .trace_len } -/// Execute and generate a valid witness for a Powdr/RISCV assembly program. +/// Execute generating a witness for the PC and powdr asm registers. #[allow(clippy::too_many_arguments)] -pub fn execute( +pub fn execute_with_trace( asm: &AnalysisASMFile, opt_pil: &Analyzed, fixed: FixedColumns, @@ -2877,7 +2877,6 @@ pub fn execute( prover_ctx: &Callback, bootloader_inputs: &[F], max_steps_to_execute: Option, - full_witness: bool, profiling: Option, ) -> Execution { log::info!("Executing (trace generation)..."); @@ -2890,11 +2889,34 @@ pub fn execute( prover_ctx, bootloader_inputs, max_steps_to_execute.unwrap_or(usize::MAX), - if full_witness { - ExecMode::Witness - } else { - ExecMode::Trace - }, + ExecMode::Trace, + profiling, + ) +} + +/// Execute generating a full witness for the program +#[allow(clippy::too_many_arguments)] +pub fn execute_with_witness( + asm: &AnalysisASMFile, + opt_pil: &Analyzed, + fixed: FixedColumns, + initial_memory: MemoryState, + prover_ctx: &Callback, + bootloader_inputs: &[F], + max_steps_to_execute: Option, + profiling: Option, +) -> Execution { + log::info!("Executing (trace generation)..."); + + execute_inner( + asm, + Some(opt_pil), + Some(fixed), + initial_memory, + prover_ctx, + bootloader_inputs, + max_steps_to_execute.unwrap_or(usize::MAX), + ExecMode::Witness, profiling, ) } diff --git a/riscv/src/continuations.rs b/riscv/src/continuations.rs index a89f4ddd10..dc629875dd 100644 --- a/riscv/src/continuations.rs +++ b/riscv/src/continuations.rs @@ -320,7 +320,7 @@ pub fn rust_continuations_dry_run( // TODO: commit to the merkle_tree root in the verifier. log::info!("Initial execution..."); - let full_exec = powdr_riscv_executor::execute::( + let full_exec = powdr_riscv_executor::execute_with_trace::( &asm, &pil, fixed.clone(), @@ -332,7 +332,6 @@ pub fn rust_continuations_dry_run( // we only know them after the full trace has been generated. &default_input(&[]), None, - false, profiler_opt, ); @@ -429,7 +428,7 @@ pub fn rust_continuations_dry_run( ); log::info!("Simulating chunk execution..."); - let chunk_exec = powdr_riscv_executor::execute::( + let chunk_exec = powdr_riscv_executor::execute_with_trace::( &asm, &pil, fixed.clone(), @@ -437,7 +436,6 @@ pub fn rust_continuations_dry_run( pipeline.data_callback().unwrap(), &bootloader_inputs, Some(num_rows), - false, // profiling was done when full trace was generated None, ); diff --git a/riscv/tests/common/mod.rs b/riscv/tests/common/mod.rs index af6ed8e61b..d9e2b0b4d4 100644 --- a/riscv/tests/common/mod.rs +++ b/riscv/tests/common/mod.rs @@ -35,7 +35,7 @@ pub fn verify_riscv_asm_string Date: Thu, 16 Jan 2025 15:16:03 -0300 Subject: [PATCH 9/9] fix --- riscv-executor/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/riscv-executor/src/lib.rs b/riscv-executor/src/lib.rs index fac71b934a..85456fa295 100644 --- a/riscv-executor/src/lib.rs +++ b/riscv-executor/src/lib.rs @@ -909,7 +909,7 @@ mod builder { } pub fn col_is_defined(&self, name: &str) -> bool { - if let ExecMode::Trace = self.mode { + if let ExecMode::Trace | ExecMode::Witness = self.mode { self.trace.all_cols.contains(&name.to_string()) } else { false