chore: typos and some refactors, tests, etc in acvm/src/compiler

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

@@ -1,9 +1,9 @@
-//! The `compiler` module contains several pass to transform an ACIR program.
-//! Roughly, the passes are spearated into the `optimizers` which try to reduce the number of opcodes
+//! The `compiler` module contains several passes to transform an ACIR program.
+//! Roughly, the passes are separated into the `optimizers` which try to reduce the number of opcodes
 //! and the `transformers` which adapt the opcodes to the proving backend.
 //!
 //! # Optimizers
-//! - GeneralOptimizer: simple pass which simplify AssertZero opcodes when possible (e.g remove terms with null coeficient)
+//! - GeneralOptimizer: simple pass which simplifies AssertZero opcodes when possible (e.g remove terms with null coefficient)
 //! - UnusedMemoryOptimizer: simple pass which removes MemoryInit opcodes when they are not used (e.g no corresponding MemoryOp opcode)
 //! - RangeOptimizer: forward pass to collect range check information, and backward pass to remove the ones that are redundant.
 //!
@@ -25,11 +25,11 @@ use acir::{
 };
 
 // The various passes that we can use over ACIR
+pub use optimizers::optimize;
 mod optimizers;
 mod simulator;
 mod transformers;
 
-pub use optimizers::optimize;
 use optimizers::optimize_internal;
 pub use simulator::CircuitSimulator;
 use transformers::transform_internal;
@@ -50,9 +50,9 @@ pub struct AcirTransformationMap {
 impl AcirTransformationMap {
     /// Builds a map from a vector of pointers to the old acir opcodes.
     /// The index in the vector is the new opcode index.
-    /// The value of the vector is the old opcode index pointed.
+    /// The value of the vector is where the old opcode index was pointed.
     /// E.g: If acir_opcode_positions = 0,1,2,4,5,5,6
-    /// that means that old index  0,1,2,4,5,5,6 are mapped to the new indexes: 0,1,2,3,4,5,6
+    /// that means that old indices 0,1,2,4,5,5,6 are mapped to the new indexes: 0,1,2,3,4,5,6
     /// This gives the following map:
     /// 0 -> 0
     /// 1 -> 1
@@ -71,8 +71,8 @@ impl AcirTransformationMap {
     /// Returns the new opcode location(s) corresponding to the old opcode.
     /// An OpcodeLocation contains the index of the opcode in the vector of opcodes
     /// This function returns the new OpcodeLocation by 'updating' the index within the given OpcodeLocation
-    /// using the AcirTransformationMap. In fact it does not update the given OpcodeLocation 'in-memory' but rather
-    /// returns a new one, and even a vector of OpcodeLocation in case there are multiple new indexes corresponding
+    /// using the AcirTransformationMap. In fact, it does not update the given OpcodeLocation 'in-memory' but rather
+    /// returns a new one, and even a vector of OpcodeLocation's in case there are multiple new indexes corresponding
     /// to the old opcode index.
     pub fn new_locations(
         &self,
@@ -95,7 +95,7 @@ impl AcirTransformationMap {
         )
     }
 
-    /// This function is similar to `new_locations()`, but only deal with
+    /// This function is similar to `new_locations()`, but only deals with
     /// the AcirOpcodeLocation variant
     pub fn new_acir_locations(
         &self,
@@ -120,19 +120,19 @@ fn transform_assert_messages<F: Clone>(
         .into_iter()
         .flat_map(|(location, message)| {
             let new_locations = map.new_locations(location);
-            new_locations.into_iter().map(move |new_location| (new_location, message.clone()))
+            new_locations.map(move |new_location| (new_location, message.clone()))
         })
         .collect()
 }
 
 /// Applies backend specific optimizations to a [`Circuit`].
 ///
 /// optimize_internal:
-/// - General optimizer: canonalize AssertZero opcodes.
+/// - General optimizer: canonicalize AssertZero opcodes.
 /// - Unused Memory: remove unused MemoryInit opcodes.
 /// - Redundant Ranges: remove RANGE opcodes that are redundant.
 ///
-/// transform_internal: run multiple times (up to 3) until the output stabilizes.
+/// transform_internal: run multiple times (up to 4) until the output stabilizes.
 /// - CSAT: limit AssertZero opcodes to the Circuit's width.
 /// - Eliminate intermediate variables: Combine AssertZero opcodes used only once.
 /// - Redundant Ranges: some RANGEs may be redundant as a side effect of the previous pass.
@@ -146,8 +146,14 @@ pub fn compile<F: AcirField>(
     let (acir, acir_opcode_positions) =
         optimize_internal(acir, acir_opcode_positions, brillig_side_effects);
 
-    let (mut acir, acir_opcode_positions) =
-        transform_internal(acir, expression_width, acir_opcode_positions, brillig_side_effects);
+    let max_transformer_passes_or_default = None;
+    let (mut acir, acir_opcode_positions, _opcodes_hash_stabilized) = transform_internal(
+        acir,
+        expression_width,
+        acir_opcode_positions,
+        brillig_side_effects,
+        max_transformer_passes_or_default,
+    );
 
     let transformation_map = AcirTransformationMap::new(&acir_opcode_positions);
     acir.assert_messages = transform_assert_messages(acir.assert_messages, &transformation_map);

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

@@ -13,7 +13,7 @@ pub(crate) struct GeneralOptimizer;
 
 impl GeneralOptimizer {
     pub(crate) fn optimize<F: AcirField>(opcode: Expression<F>) -> Expression<F> {
-        // XXX: Perhaps this optimization can be done on the fly
+        // TODO(https://github.com/noir-lang/noir/issues/10109): Perhaps this optimization can be done on the fly
         let opcode = simplify_mul_terms(opcode);
         simplify_linear_terms(opcode)
     }
@@ -50,7 +50,7 @@ fn simplify_mul_terms<F: AcirField>(mut gate: Expression<F>) -> Expression<F> {
 fn simplify_linear_terms<F: AcirField>(mut gate: Expression<F>) -> Expression<F> {
     let mut hash_map: IndexMap<Witness, F> = IndexMap::new();
 
-    // Canonicalize the ordering of the terms, lets just order by variable name
+    // Canonicalize the ordering of the terms, let's just order by variable name
     for (scale, witness) in gate.linear_combinations.into_iter() {
         *hash_map.entry(witness).or_insert_with(F::zero) += scale;
     }
@@ -207,4 +207,22 @@ mod tests {
         ASSERT 0 = 0
         ");
     }
+
+    #[test]
+    fn simplify_mul_terms_example() {
+        let src = "
+        private parameters: [w0, w1]
+        public parameters: []
+        return values: []
+        ASSERT 0*w1*w1 + 2*w2*w1 - w2*w1 - w1*w2 = 0
+        ";
+        let circuit = Circuit::from_str(src).unwrap();
+        let optimized_circuit = optimize(circuit);
+        assert_circuit_snapshot!(optimized_circuit, @r"
+        private parameters: [w0, w1]
+        public parameters: []
+        return values: []
+        ASSERT 0 = 0
+        ");
+    }
 }