Skip to content

Commit

Permalink
chore: remove clones in optimizer/transformer code (#3037)
Browse files Browse the repository at this point in the history
Co-authored-by: kevaundray <kevtheappdev@gmail.com>
  • Loading branch information
TomAFrench and kevaundray authored Oct 9, 2023
1 parent 2e008e2 commit 9fb7183
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 36 deletions.
2 changes: 1 addition & 1 deletion acvm-repo/acvm/src/compiler/optimizers/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
20 changes: 11 additions & 9 deletions acvm-repo/acvm/src/compiler/optimizers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,22 @@ pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) {
/// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] independent optimizations to a [`Circuit`].
pub(super) fn optimize_internal(acir: Circuit) -> (Circuit, AcirTransformationMap) {
// General optimizer pass
let mut opcodes: Vec<Opcode> = Vec::new();
for opcode in acir.opcodes {
match opcode {
Opcode::Arithmetic(arith_expr) => {
opcodes.push(Opcode::Arithmetic(GeneralOptimizer::optimize(arith_expr)));
let opcodes: Vec<Opcode> = 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);
Expand Down
17 changes: 5 additions & 12 deletions acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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)
}
}

Expand Down
6 changes: 3 additions & 3 deletions acvm-repo/acvm/src/compiler/optimizers/unused_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,16 @@ impl UnusedMemoryOptimizer {
) -> (Circuit, Vec<usize>) {
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);
}
}
}
Expand Down
22 changes: 11 additions & 11 deletions acvm-repo/acvm/src/compiler/transformers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,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<Expression, (FieldElement, Witness)> = 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,
);
Expand All @@ -104,7 +104,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, .. } => {
Expand Down Expand Up @@ -146,9 +146,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);
Expand All @@ -166,14 +166,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);
Expand All @@ -182,9 +182,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),
Expand All @@ -196,7 +196,7 @@ pub(super) fn transform_internal(
}
}
new_acir_opcode_positions.push(acir_opcode_positions[index]);
transformed_opcodes.push(opcode.clone());
transformed_opcodes.push(opcode);
}
}
}
Expand Down

0 comments on commit 9fb7183

Please sign in to comment.