From 9a44d2a4c038ecb6a7746de7b6e4ecaf3887d707 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Tue, 10 Dec 2024 15:51:42 +0000 Subject: [PATCH 1/2] Do not simplify circuit outputs --- acvm-repo/acvm/src/compiler/mod.rs | 5 +++++ .../acvm/src/compiler/optimizers/merge_expressions.rs | 10 ++++++---- acvm-repo/acvm/src/pwg/mod.rs | 1 - 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/mod.rs b/acvm-repo/acvm/src/compiler/mod.rs index e32c0665c0f..4ad4952c5cc 100644 --- a/acvm-repo/acvm/src/compiler/mod.rs +++ b/acvm-repo/acvm/src/compiler/mod.rs @@ -82,6 +82,11 @@ pub fn compile( acir: Circuit, expression_width: ExpressionWidth, ) -> (Circuit, AcirTransformationMap) { + if MAX_OPTIMIZER_PASSES == 0 { + let acir_opcode_positions = (0..acir.opcodes.len()).collect::>(); + let transformation_map = AcirTransformationMap::new(&acir_opcode_positions); + return (acir, transformation_map); + } let mut pass = 0; let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes); let mut prev_acir = acir; diff --git a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs index f49cd61e813..fbc294b35a6 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs @@ -41,7 +41,9 @@ impl MergeExpressionsOptimizer { self.resolved_blocks.clear(); // Keep track, for each witness, of the gates that use it - let circuit_inputs = circuit.circuit_arguments(); + let circuit_io: BTreeSet = + circuit.circuit_arguments().union(&circuit.public_inputs().0).cloned().collect(); + let mut used_witness: BTreeMap> = BTreeMap::new(); for (i, opcode) in circuit.opcodes.iter().enumerate() { let witnesses = self.witness_inputs(opcode); @@ -49,8 +51,8 @@ impl MergeExpressionsOptimizer { self.resolved_blocks.insert(*block_id, witnesses.clone()); } for w in witnesses { - // We do not simplify circuit inputs - if !circuit_inputs.contains(&w) { + // We do not simplify circuit inputs and outputs + if !circuit_io.contains(&w) { used_witness.entry(w).or_default().insert(i); } } @@ -102,7 +104,7 @@ impl MergeExpressionsOptimizer { let mut witness_list = CircuitSimulator::expr_wit(&expr_use); witness_list.extend(CircuitSimulator::expr_wit(&expr_define)); for w2 in witness_list { - if !circuit_inputs.contains(&w2) { + if !circuit_io.contains(&w2) { used_witness.entry(w2).and_modify(|v| { v.insert(target); v.remove(&source); diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 20c12a72fc0..f9188cca700 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -359,7 +359,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { pub fn solve_opcode(&mut self) -> ACVMStatus { let opcode = &self.opcodes[self.instruction_pointer]; - let resolution = match opcode { Opcode::AssertZero(expr) => ExpressionSolver::solve(&mut self.witness_map, expr), Opcode::BlackBoxFuncCall(bb_func) => blackbox::solve( From a1d4eee6d2733e0a09f452bafecb727d58be1ccb Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Tue, 10 Dec 2024 19:31:59 +0000 Subject: [PATCH 2/2] Add unit test --- .../compiler/optimizers/merge_expressions.rs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs index fbc294b35a6..43e32101cc5 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs @@ -328,6 +328,50 @@ mod tests { check_circuit(circuit); } + #[test] + fn does_not_eliminate_witnesses_returned_from_circuit() { + let opcodes = vec![ + Opcode::AssertZero(Expression { + mul_terms: vec![(FieldElement::from(-1i128), Witness(0), Witness(0))], + linear_combinations: vec![(FieldElement::from(1i128), Witness(1))], + q_c: FieldElement::zero(), + }), + Opcode::AssertZero(Expression { + mul_terms: Vec::new(), + linear_combinations: vec![ + (FieldElement::from(-1i128), Witness(1)), + (FieldElement::from(1i128), Witness(2)), + ], + q_c: FieldElement::zero(), + }), + ]; + // Witness(1) could be eliminated because it's only used by 2 opcodes. + + let mut private_parameters = BTreeSet::new(); + private_parameters.insert(Witness(0)); + + let mut return_values = BTreeSet::new(); + return_values.insert(Witness(1)); + return_values.insert(Witness(2)); + + let circuit = Circuit { + current_witness_index: 2, + expression_width: ExpressionWidth::Bounded { width: 4 }, + opcodes, + private_parameters, + public_parameters: PublicInputs::default(), + return_values: PublicInputs(return_values), + assert_messages: Default::default(), + }; + + let mut merge_optimizer = MergeExpressionsOptimizer::new(); + let acir_opcode_positions = vec![0; 20]; + let (opcodes, _) = + merge_optimizer.eliminate_intermediate_variable(&circuit, acir_opcode_positions); + + assert_eq!(opcodes.len(), 2); + } + #[test] fn does_not_attempt_to_merge_into_previous_opcodes() { let opcodes = vec![