diff --git a/Cargo.lock b/Cargo.lock index 2f19ed704b2..f414a126495 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -48,6 +48,7 @@ dependencies = [ "ark-bn254", "bn254_blackbox_solver", "brillig_vm", + "fxhash", "indexmap 1.9.3", "num-bigint", "proptest", diff --git a/acvm-repo/acvm/Cargo.toml b/acvm-repo/acvm/Cargo.toml index e513ae4e727..ba01ac8ec16 100644 --- a/acvm-repo/acvm/Cargo.toml +++ b/acvm-repo/acvm/Cargo.toml @@ -17,7 +17,7 @@ workspace = true thiserror.workspace = true tracing.workspace = true serde.workspace = true - +fxhash.workspace = true acir.workspace = true brillig_vm.workspace = true acvm_blackbox_solver.workspace = true diff --git a/acvm-repo/acvm/src/compiler/transformers/mod.rs b/acvm-repo/acvm/src/compiler/transformers/mod.rs index 8f9ef63be1f..85a5ac8bb96 100644 --- a/acvm-repo/acvm/src/compiler/transformers/mod.rs +++ b/acvm-repo/acvm/src/compiler/transformers/mod.rs @@ -19,6 +19,10 @@ use super::{ optimizers::MergeExpressionsOptimizer, transform_assert_messages, AcirTransformationMap, }; +/// The `MergeExpressionOptimizer` needs multiple passes to stabilize the output. +/// Testing showed two passes to be enough. +const MAX_OPTIMIZER_PASSES: usize = 10; + /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] specific optimizations to a [`Circuit`]. pub fn transform( acir: Circuit, @@ -153,24 +157,37 @@ pub(super) fn transform_internal( let current_witness_index = next_witness_index - 1; - let acir = Circuit { + let mut acir = Circuit { current_witness_index, expression_width, opcodes: transformed_opcodes, // The transformer does not add new public inputs ..acir }; - let mut merge_optimizer = MergeExpressionsOptimizer::new(); - let (opcodes, new_acir_opcode_positions) = - merge_optimizer.eliminate_intermediate_variable(&acir, new_acir_opcode_positions); + // Allow multiple passes until we have stable output. + let mut prev_circuit_hash = fxhash::hash64(&acir); + for _ in 0..MAX_OPTIMIZER_PASSES { + let mut merge_optimizer = MergeExpressionsOptimizer::new(); - // n.b. if we do not update current_witness_index after the eliminate_intermediate_variable pass, the real index could be less. - let mut acir = Circuit { - opcodes, - // The optimizer does not add new public inputs - ..acir - }; + let (next_opcodes, next_acir_opcode_positions) = + merge_optimizer.eliminate_intermediate_variable(&acir, new_acir_opcode_positions); + + // n.b. if we do not update current_witness_index after the eliminate_intermediate_variable pass, the real index could be less. + acir = Circuit { + opcodes: next_opcodes, + // The optimizer does not add new public inputs + ..acir + }; + + let next_circuit_hash = fxhash::hash64(&acir); + new_acir_opcode_positions = next_acir_opcode_positions; + + if next_circuit_hash == prev_circuit_hash { + break; + } + prev_circuit_hash = next_circuit_hash; + } // After the elimination of intermediate variables the `current_witness_index` is potentially higher than it needs to be, // which would cause gaps if we ran the optimization a second time, making it look like new variables were added. diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index f134374f89e..4c7f55fa0cc 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -413,6 +413,11 @@ mod tests { let width = get_target_width(package.expression_width, None); + // Figure out if there is an upper bound to repeating the entire optimization step after which the results are stable: + // for _ in 0..2 { + // program_0 = nargo::ops::transform_program(program_0, width); + // } + let program_1 = nargo::ops::transform_program(program_0, width); let program_2 = nargo::ops::transform_program(program_1.clone(), width);