feat(test): Check that nargo::ops::transform_program is idempotent (#…

…6694)

Co-authored-by: Tom French <[email protected]>

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

acvm-repo/acvm/src/compiler/mod.rs

@@ -16,6 +16,10 @@ pub use simulator::CircuitSimulator;
 use transformers::transform_internal;
 pub use transformers::{transform, MIN_EXPRESSION_WIDTH};
 
+/// We need multiple passes to stabilize the output.
+/// The value was determined by running tests.
+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)]
@@ -28,9 +32,9 @@ impl AcirTransformationMap {
     /// Builds a map from a vector of pointers to the old acir opcodes.
     /// The index of the vector is the new opcode index.
     /// The value of the vector is the old opcode index pointed.
-    fn new(acir_opcode_positions: Vec<usize>) -> Self {
+    fn new(acir_opcode_positions: &[usize]) -> Self {
         let mut old_indices_to_new_indices = HashMap::with_capacity(acir_opcode_positions.len());
-        for (new_index, old_index) in acir_opcode_positions.into_iter().enumerate() {
+        for (new_index, old_index) in acir_opcode_positions.iter().copied().enumerate() {
             old_indices_to_new_indices.entry(old_index).or_insert_with(Vec::new).push(new_index);
         }
         AcirTransformationMap { old_indices_to_new_indices }
@@ -72,17 +76,42 @@ fn transform_assert_messages<F: Clone>(
 }
 
 /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] specific optimizations to a [`Circuit`].
+///
+/// Runs multiple passes until the output stabilizes.
 pub fn compile<F: AcirField>(
     acir: Circuit<F>,
     expression_width: ExpressionWidth,
 ) -> (Circuit<F>, 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);
+
+        // Stop if we have already done at least one transform and an extra optimization changed nothing.
+        if pass > 0 && prev_opcodes_hash == fxhash::hash64(&acir.opcodes) {
+            break (acir, acir_opcode_positions);
+        }
 
-    let (mut acir, acir_opcode_positions) =
-        transform_internal(acir, expression_width, acir_opcode_positions);
+        let (acir, acir_opcode_positions) =
+            transform_internal(acir, expression_width, acir_opcode_positions);
+
+        let opcodes_hash = fxhash::hash64(&acir.opcodes);
+
+        // Stop if the output hasn't change in this loop or we went too long.
+        if pass == MAX_OPTIMIZER_PASSES - 1 || prev_opcodes_hash == opcodes_hash {
+            break (acir, acir_opcode_positions);
+        }
 
-    let transformation_map = AcirTransformationMap::new(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);
 
     (acir, transformation_map)

acvm-repo/acvm/src/compiler/optimizers/mod.rs

@@ -23,7 +23,7 @@ use super::{transform_assert_messages, AcirTransformationMap};
 pub fn optimize<F: AcirField>(acir: Circuit<F>) -> (Circuit<F>, AcirTransformationMap) {
     let (mut acir, new_opcode_positions) = optimize_internal(acir);
 
-    let transformation_map = AcirTransformationMap::new(new_opcode_positions);
+    let transformation_map = AcirTransformationMap::new(&new_opcode_positions);
 
     acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map);