diff --git a/acvm-repo/acvm/src/compiler/mod.rs b/acvm-repo/acvm/src/compiler/mod.rs index 89e0d1dbb4f..012afd86ab9 100644 --- a/acvm-repo/acvm/src/compiler/mod.rs +++ b/acvm-repo/acvm/src/compiler/mod.rs @@ -16,6 +16,9 @@ pub use simulator::CircuitSimulator; use transformers::transform_internal; pub use transformers::MIN_EXPRESSION_WIDTH; +/// We need multiple passes to stabilize the output. +const MAX_OPTIMIZER_PASSES: usize = 3; + /// This module moves and decomposes acir opcodes. The transformation map allows consumers of this module to map /// metadata they had about the opcodes to the new opcode structure generated after the transformation. #[derive(Debug)] @@ -72,14 +75,32 @@ fn transform_assert_messages( } /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] specific optimizations to a [`Circuit`]. +/// +/// Runs multiple passes until the output stabilizes. pub fn compile( acir: Circuit, expression_width: ExpressionWidth, ) -> (Circuit, AcirTransformationMap) { - let (acir, acir_opcode_positions) = optimize_internal(acir); + let mut pass = 0; + let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes); + let mut prev_acir = acir; + + // For most test programs it would be enough to only loop `transform_internal`, + // but some of them don't stabilize unless we also repeat the backend agnostic optimizations. + let (mut acir, acir_opcode_positions) = loop { + let (acir, acir_opcode_positions) = optimize_internal(prev_acir); + let (acir, acir_opcode_positions) = + transform_internal(acir, expression_width, acir_opcode_positions); - let (mut acir, acir_opcode_positions) = - transform_internal(acir, expression_width, acir_opcode_positions); + let opcodes_hash = fxhash::hash64(&acir.opcodes); + + if pass == MAX_OPTIMIZER_PASSES || prev_opcodes_hash == opcodes_hash { + break (acir, acir_opcode_positions); + } + pass += 1; + prev_acir = acir; + prev_opcodes_hash = opcodes_hash + }; let transformation_map = AcirTransformationMap::new(&acir_opcode_positions); acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map); diff --git a/acvm-repo/acvm/src/compiler/transformers/mod.rs b/acvm-repo/acvm/src/compiler/transformers/mod.rs index a9bcad9e09e..9f140c671d1 100644 --- a/acvm-repo/acvm/src/compiler/transformers/mod.rs +++ b/acvm-repo/acvm/src/compiler/transformers/mod.rs @@ -17,12 +17,12 @@ pub use csat::MIN_EXPRESSION_WIDTH; use super::{ optimizers::MergeExpressionsOptimizer, transform_assert_messages, AcirTransformationMap, + MAX_OPTIMIZER_PASSES, }; -/// The `MergeExpressionOptimizer` needs multiple passes to stabilize the output. -const MAX_OPTIMIZER_PASSES: usize = 3; - /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] specific optimizations to a [`Circuit`]. +/// +/// TODO: Can this be removed? fn _transform( mut acir: Circuit, expression_width: ExpressionWidth, @@ -58,6 +58,8 @@ pub(super) fn transform_internal( // Allow multiple passes until we have stable output. let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes); + // For most test programs it would be enough to loop here, but some of them + // don't stabilize unless we also repeat the backend agnostic optimizations. for _ in 0..MAX_OPTIMIZER_PASSES { let (new_acir, new_acir_opcode_positions) = transform_internal_once(acir, expression_width, acir_opcode_positions); diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 9a5a27defca..d019e87eee6 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -414,7 +414,7 @@ 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 i in 0..0 { + // for i in 0..2 { // program_0 = nargo::ops::transform_program(program_0, width); // }