From 37f88cc17474ff6c002a8ad0a220bc178026e05d Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 9 Oct 2023 01:25:32 +0100 Subject: [PATCH 1/2] chore: remove clones in csat transformer code --- .../acvm/src/compiler/transformers/mod.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/transformers/mod.rs b/acvm-repo/acvm/src/compiler/transformers/mod.rs index b909bc54662..09fee6ab08a 100644 --- a/acvm-repo/acvm/src/compiler/transformers/mod.rs +++ b/acvm-repo/acvm/src/compiler/transformers/mod.rs @@ -73,13 +73,13 @@ pub(super) fn transform_internal( // maps a normalized expression to the intermediate variable which represents the expression, along with its 'norm' // the 'norm' is simply the value of the first non zero coefficient in the expression, taken from the linear terms, or quadratic terms if there is none. let mut intermediate_variables: IndexMap = IndexMap::new(); - for (index, opcode) in acir.opcodes.iter().enumerate() { + for (index, opcode) in acir.opcodes.into_iter().enumerate() { match opcode { Opcode::Arithmetic(arith_expr) => { let len = intermediate_variables.len(); let arith_expr = transformer.transform( - arith_expr.clone(), + arith_expr, &mut intermediate_variables, &mut next_witness_index, ); @@ -101,7 +101,7 @@ pub(super) fn transform_internal( transformed_opcodes.push(Opcode::Arithmetic(opcode)); } } - Opcode::BlackBoxFuncCall(func) => { + Opcode::BlackBoxFuncCall(ref func) => { match func { acir::circuit::opcodes::BlackBoxFuncCall::AND { output, .. } | acir::circuit::opcodes::BlackBoxFuncCall::XOR { output, .. } => { @@ -143,9 +143,9 @@ pub(super) fn transform_internal( } new_acir_opcode_positions.push(acir_opcode_positions[index]); - transformed_opcodes.push(opcode.clone()); + transformed_opcodes.push(opcode); } - Opcode::Directive(directive) => { + Opcode::Directive(ref directive) => { match directive { Directive::Quotient(quotient_directive) => { transformer.mark_solvable(quotient_directive.q); @@ -163,14 +163,14 @@ pub(super) fn transform_internal( } } new_acir_opcode_positions.push(acir_opcode_positions[index]); - transformed_opcodes.push(opcode.clone()); + transformed_opcodes.push(opcode); } Opcode::MemoryInit { .. } => { // `MemoryInit` does not write values to the `WitnessMap` new_acir_opcode_positions.push(acir_opcode_positions[index]); - transformed_opcodes.push(opcode.clone()); + transformed_opcodes.push(opcode); } - Opcode::MemoryOp { op, .. } => { + Opcode::MemoryOp { ref op, .. } => { for (_, witness1, witness2) in &op.value.mul_terms { transformer.mark_solvable(*witness1); transformer.mark_solvable(*witness2); @@ -179,9 +179,9 @@ pub(super) fn transform_internal( transformer.mark_solvable(*witness); } new_acir_opcode_positions.push(acir_opcode_positions[index]); - transformed_opcodes.push(opcode.clone()); + transformed_opcodes.push(opcode); } - Opcode::Brillig(brillig) => { + Opcode::Brillig(ref brillig) => { for output in &brillig.outputs { match output { BrilligOutputs::Simple(w) => transformer.mark_solvable(*w), @@ -193,7 +193,7 @@ pub(super) fn transform_internal( } } new_acir_opcode_positions.push(acir_opcode_positions[index]); - transformed_opcodes.push(opcode.clone()); + transformed_opcodes.push(opcode); } } } From 052102e199cedcbca46f18dd3aa5afb0dbb6208d Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 9 Oct 2023 14:02:13 +0100 Subject: [PATCH 2/2] chore: remove unnecessary clones --- .../acvm/src/compiler/optimizers/general.rs | 2 +- acvm-repo/acvm/src/compiler/optimizers/mod.rs | 20 ++++++++++--------- .../compiler/optimizers/redundant_range.rs | 17 +++++----------- .../src/compiler/optimizers/unused_memory.rs | 6 +++--- 4 files changed, 20 insertions(+), 25 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/optimizers/general.rs b/acvm-repo/acvm/src/compiler/optimizers/general.rs index 50271348c11..2bd781f7bb5 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/general.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/general.rs @@ -31,7 +31,7 @@ fn simplify_mul_terms(mut gate: Expression) -> Expression { let mut hash_map: IndexMap<(Witness, Witness), FieldElement> = IndexMap::new(); // Canonicalize the ordering of the multiplication, lets just order by variable name - for (scale, w_l, w_r) in gate.mul_terms.clone().into_iter() { + for (scale, w_l, w_r) in gate.mul_terms.into_iter() { let mut pair = [w_l, w_r]; // Sort using rust sort algorithm pair.sort(); diff --git a/acvm-repo/acvm/src/compiler/optimizers/mod.rs b/acvm-repo/acvm/src/compiler/optimizers/mod.rs index d04d21039f9..2fe1c31258e 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/mod.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/mod.rs @@ -14,20 +14,22 @@ use super::{transform_assert_messages, AcirTransformationMap}; /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] independent optimizations to a [`Circuit`]. pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) { // General optimizer pass - let mut opcodes: Vec = Vec::new(); - for opcode in acir.opcodes { - match opcode { - Opcode::Arithmetic(arith_expr) => { - opcodes.push(Opcode::Arithmetic(GeneralOptimizer::optimize(arith_expr))); + let opcodes: Vec = acir + .opcodes + .into_iter() + .map(|opcode| { + if let Opcode::Arithmetic(arith_expr) = opcode { + Opcode::Arithmetic(GeneralOptimizer::optimize(arith_expr)) + } else { + opcode } - other_opcode => opcodes.push(other_opcode), - }; - } + }) + .collect(); let acir = Circuit { opcodes, ..acir }; // Track original acir opcode positions throughout the transformation passes of the compilation // by applying the modifications done to the circuit opcodes and also to the opcode_positions (delete and insert) - let acir_opcode_positions = acir.opcodes.iter().enumerate().map(|(i, _)| i).collect(); + let acir_opcode_positions = (0..acir.opcodes.len()).collect(); // Unused memory optimization pass let memory_optimizer = UnusedMemoryOptimizer::new(acir); diff --git a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs index 9833a31a199..b1696704108 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs @@ -74,13 +74,13 @@ impl RangeOptimizer { let mut new_order_list = Vec::with_capacity(order_list.len()); let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len()); - for (idx, opcode) in self.circuit.opcodes.iter().enumerate() { - let (witness, num_bits) = match extract_range_opcode(opcode) { + for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() { + let (witness, num_bits) = match extract_range_opcode(&opcode) { Some(range_opcode) => range_opcode, None => { // If its not the range opcode, add it to the opcode // list and continue; - optimized_opcodes.push(opcode.clone()); + optimized_opcodes.push(opcode); new_order_list.push(order_list[idx]); continue; } @@ -101,18 +101,11 @@ impl RangeOptimizer { if is_lowest_bit_size { already_seen_witness.insert(witness); new_order_list.push(order_list[idx]); - optimized_opcodes.push(opcode.clone()); + optimized_opcodes.push(opcode); } } - ( - Circuit { - current_witness_index: self.circuit.current_witness_index, - opcodes: optimized_opcodes, - ..self.circuit - }, - new_order_list, - ) + (Circuit { opcodes: optimized_opcodes, ..self.circuit }, new_order_list) } } diff --git a/acvm-repo/acvm/src/compiler/optimizers/unused_memory.rs b/acvm-repo/acvm/src/compiler/optimizers/unused_memory.rs index eccea631723..18eefa79ac2 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/unused_memory.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/unused_memory.rs @@ -42,16 +42,16 @@ impl UnusedMemoryOptimizer { ) -> (Circuit, Vec) { let mut new_order_list = Vec::with_capacity(order_list.len()); let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len()); - for (idx, opcode) in self.circuit.opcodes.iter().enumerate() { + for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() { match opcode { Opcode::MemoryInit { block_id, .. } - if self.unused_memory_initializations.contains(block_id) => + if self.unused_memory_initializations.contains(&block_id) => { // Drop opcode } _ => { new_order_list.push(order_list[idx]); - optimized_opcodes.push(opcode.clone()); + optimized_opcodes.push(opcode); } } }