Reorganiser compiler in terms of optimisers and transformers

Passes that are required for a circuit to usable, rather than just for improving performance, are now called transformers instead of optimisations.

Transforms:
fallback
r1cs
csat

Optimisations:
general
more to come

General optimising is now separated into its own pass instead of being executed by r1cs of csat. As a result the r1cs pass current has no work to perform.

acir/src/circuit/mod.rs

@@ -16,6 +16,8 @@ const VERSION_NUMBER: u32 = 0;
 
 #[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
 pub struct Circuit {
+    // current_witness_index is the highest witness index in the circuit. The next witness to be added to this circuit
+    // will take on this value. (The value is cached here as an optimisation.)
     pub current_witness_index: u32,
     pub opcodes: Vec<Opcode>,
     pub public_inputs: PublicInputs,
@@ -204,7 +206,7 @@ mod test {
                 Opcode::Arithmetic(crate::native_types::Expression {
                     mul_terms: vec![],
                     linear_combinations: vec![],
-                    q_c: FieldElement::from_hex("FFFF").unwrap(),
+                    q_c: FieldElement::from(8u128),
                 }),
                 range_opcode(),
                 and_opcode(),

acir/src/native_types/witness.rs

@@ -6,6 +6,7 @@ use flate2::{
 };
 use serde::{Deserialize, Serialize};
 
+// Witness might be a misnomer. This is an index that represents the position a witness will take
 #[derive(
     Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Default, Serialize, Deserialize,
 )]

acvm/src/compiler.rs

@@ -1,6 +1,6 @@
 // The various passes that we can use over ACIR
-pub mod fallback;
-pub mod optimiser;
+pub mod optimisers;
+pub mod transformers;
 
 use crate::Language;
 use acir::{
@@ -9,10 +9,9 @@ use acir::{
     BlackBoxFunc,
 };
 use indexmap::IndexMap;
-use optimiser::{CSatOptimiser, GeneralOptimiser};
+use optimisers::GeneralOptimiser;
 use thiserror::Error;
-
-use self::{fallback::IsBlackBoxSupported, optimiser::R1CSOptimiser};
+use transformers::{CSatTransformer, FallbackTransformer, IsBlackBoxSupported, R1CSTransformer};
 
 #[derive(PartialEq, Eq, Debug, Error)]
 pub enum CompileError {
@@ -29,33 +28,48 @@ pub fn compile(
     // Currently the optimiser and reducer are one in the same
     // for CSAT
 
-    // Fallback pass
-    let fallback = fallback::fallback(acir, is_blackbox_supported)?;
+    // Fallback transformer pass
+    let acir = FallbackTransformer::transform(acir, is_blackbox_supported)?;
+
+    // General optimiser pass
+    let mut opcodes: Vec<Opcode> = Vec::new();
+    for opcode in acir.opcodes {
+        match opcode {
+            Opcode::Arithmetic(arith_expr) => {
+                opcodes.push(Opcode::Arithmetic(GeneralOptimiser::optimise(arith_expr)))
+            }
+            other_gate => opcodes.push(other_gate),
+        };
+    }
+    let acir = Circuit { opcodes, ..acir };
 
-    let optimiser = match &np_language {
+    let transformer = match &np_language {
         crate::Language::R1CS => {
-            let optimiser = R1CSOptimiser::new(fallback);
-            return Ok(optimiser.optimise());
+            let transformer = R1CSTransformer::new(acir);
+            return Ok(transformer.transform());
         }
-        crate::Language::PLONKCSat { width } => CSatOptimiser::new(*width),
+        crate::Language::PLONKCSat { width } => CSatTransformer::new(*width),
     };
 
-    // TODO: the code below is only for CSAT optimiser
+    // TODO: the code below is only for CSAT transformer
     // TODO it may be possible to refactor it in a way that we do not need to return early from the r1cs
     // TODO or at the very least, we could put all of it inside of CSATOptimiser pass
 
     // Optimise the arithmetic gates by reducing them into the correct width and
     // creating intermediate variables when necessary
-    let mut optimised_gates = Vec::new();
+    let mut transformed_gates = Vec::new();
 
-    let mut next_witness_index = fallback.current_witness_index + 1;
-    for opcode in fallback.opcodes {
+    let mut next_witness_index = acir.current_witness_index + 1;
+    for opcode in acir.opcodes {
         match opcode {
             Opcode::Arithmetic(arith_expr) => {
                 let mut intermediate_variables: IndexMap<Witness, Expression> = IndexMap::new();
 
-                let arith_expr =
-                    optimiser.optimise(arith_expr, &mut intermediate_variables, next_witness_index);
+                let arith_expr = transformer.transform(
+                    arith_expr,
+                    &mut intermediate_variables,
+                    next_witness_index,
+                );
 
                 // Update next_witness counter
                 next_witness_index += intermediate_variables.len() as u32;
@@ -67,18 +81,18 @@ pub fn compile(
                 new_gates.push(arith_expr);
                 new_gates.sort();
                 for gate in new_gates {
-                    optimised_gates.push(Opcode::Arithmetic(gate));
+                    transformed_gates.push(Opcode::Arithmetic(gate));
                 }
             }
-            other_gate => optimised_gates.push(other_gate),
+            other_gate => transformed_gates.push(other_gate),
         }
     }
 
     let current_witness_index = next_witness_index - 1;
 
     Ok(Circuit {
         current_witness_index,
-        opcodes: optimised_gates,
-        public_inputs: fallback.public_inputs, // The optimiser does not add public inputs
+        opcodes: transformed_gates,
+        public_inputs: acir.public_inputs, // The optimiser does not add public inputs
     })
 }

acvm/src/compiler/optimisers/mod.rs

@@ -0,0 +1,3 @@
+mod general;
+
+pub use general::GeneralOpt as GeneralOptimiser;

acvm/src/compiler/optimiser/csat_optimiser.rs → acvm/src/compiler/transformers/csat.rs

@@ -6,35 +6,30 @@ use acir::{
 };
 use indexmap::IndexMap;
 
-use super::general_optimiser::GeneralOpt;
-// Optimiser struct with all of the related optimisations to the arithmetic gate
-
 // Is this more of a Reducer than an optimiser?
 // Should we give it all of the gates?
 // Have a single optimiser that you instantiate with a width, then pass many gates through
-pub struct Optimiser {
+pub struct CSatTransformer {
     width: usize,
 }
 
-impl Optimiser {
+impl CSatTransformer {
     // Configure the width for the optimiser
-    pub fn new(width: usize) -> Optimiser {
+    pub fn new(width: usize) -> CSatTransformer {
         assert!(width > 2);
 
-        Optimiser { width }
+        CSatTransformer { width }
     }
 
     // Still missing dead witness optimisation.
     // To do this, we will need the whole set of arithmetic gates
     // I think it can also be done before the local optimisation seen here, as dead variables will come from the user
-    pub fn optimise(
+    pub fn transform(
         &self,
         gate: Expression,
         intermediate_variables: &mut IndexMap<Witness, Expression>,
         num_witness: u32,
     ) -> Expression {
-        let gate = GeneralOpt::optimise(gate);
-
         // Here we create intermediate variables and constrain them to be equal to any subset of the polynomial that can be represented as a full gate
         let gate = self.full_gate_scan_optimisation(gate, intermediate_variables, num_witness);
         // The last optimisation to do is to create intermediate variables in order to flatten the fan-in and the amount of mul terms
@@ -348,8 +343,9 @@ fn simple_reduction_smoke_test() {
 
     let num_witness = 4;
 
-    let optimiser = Optimiser::new(3);
-    let got_optimised_gate_a = optimiser.optimise(gate_a, &mut intermediate_variables, num_witness);
+    let optimiser = CSatTransformer::new(3);
+    let got_optimised_gate_a =
+        optimiser.transform(gate_a, &mut intermediate_variables, num_witness);
 
     // a = b + c + d => a - b - c - d = 0
     // For width3, the result becomes: