Merge branch 'master' into 6515-interpreter-blackbox

.aztec-sync-commit

@@ -1 +1 @@
-58761fcf181d0a26c4e387105b1bc8a3a75b0368
+b8bace9a00c3a8eb93f42682e8cbfa351fc5238c

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

@@ -50,31 +50,36 @@ impl MergeExpressionsOptimizer {
         let mut new_circuit = Vec::new();
         let mut new_acir_opcode_positions = Vec::new();
         // For each opcode, try to get a target opcode to merge with
-        for (i, opcode) in circuit.opcodes.iter().enumerate() {
+        for (i, (opcode, opcode_position)) in
+            circuit.opcodes.iter().zip(acir_opcode_positions).enumerate()
+        {
             if !matches!(opcode, Opcode::AssertZero(_)) {
                 new_circuit.push(opcode.clone());
-                new_acir_opcode_positions.push(acir_opcode_positions[i]);
+                new_acir_opcode_positions.push(opcode_position);
                 continue;
             }
             let opcode = modified_gates.get(&i).unwrap_or(opcode).clone();
             let mut to_keep = true;
             let input_witnesses = self.witness_inputs(&opcode);
-            for w in input_witnesses.clone() {
-                let empty_gates = BTreeSet::new();
-                let gates_using_w = used_witness.get(&w).unwrap_or(&empty_gates);
+            for w in input_witnesses {
+                let Some(gates_using_w) = used_witness.get(&w) else {
+                    continue;
+                };
                 // We only consider witness which are used in exactly two arithmetic gates
                 if gates_using_w.len() == 2 {
-                    let gates_using_w: Vec<_> = gates_using_w.iter().collect();
-                    let mut b = *gates_using_w[1];
-                    if b == i {
-                        b = *gates_using_w[0];
+                    let first = *gates_using_w.first().expect("gates_using_w.len == 2");
+                    let second = *gates_using_w.last().expect("gates_using_w.len == 2");
+                    let b = if second == i {
+                        first
                     } else {
                         // sanity check
-                        assert!(i == *gates_using_w[0]);
-                    }
-                    let second_gate = modified_gates.get(&b).unwrap_or(&circuit.opcodes[b]).clone();
+                        assert!(i == first);
+                        second
+                    };
+
+                    let second_gate = modified_gates.get(&b).unwrap_or(&circuit.opcodes[b]);
                     if let (Opcode::AssertZero(expr_define), Opcode::AssertZero(expr_use)) =
-                        (opcode.clone(), second_gate)
+                        (&opcode, second_gate)
                     {
                         // We cannot merge an expression into an earlier opcode, because this
                         // would break the 'execution ordering' of the opcodes
@@ -85,16 +90,15 @@ impl MergeExpressionsOptimizer {
                         // - doing this pass again until there is no change, or
                         // - merging 'b' into 'i' instead
                         if i < b {
-                            if let Some(expr) = Self::merge(&expr_use, &expr_define, w) {
+                            if let Some(expr) = Self::merge(expr_use, expr_define, w) {
                                 modified_gates.insert(b, Opcode::AssertZero(expr));
                                 to_keep = false;
                                 // Update the 'used_witness' map to account for the merge.
-                                for w2 in CircuitSimulator::expr_wit(&expr_define) {
+                                for w2 in CircuitSimulator::expr_wit(expr_define) {
                                     if !circuit_inputs.contains(&w2) {
-                                        let mut v = used_witness[&w2].clone();
+                                        let v = used_witness.entry(w2).or_default();
                                         v.insert(b);
                                         v.remove(&i);
-                                        used_witness.insert(w2, v);
                                     }
                                 }
                                 // We need to stop here and continue with the next opcode
@@ -107,12 +111,9 @@ impl MergeExpressionsOptimizer {
             }
 
             if to_keep {
-                if modified_gates.contains_key(&i) {
-                    new_circuit.push(modified_gates[&i].clone());
-                } else {
-                    new_circuit.push(opcode.clone());
-                }
-                new_acir_opcode_positions.push(acir_opcode_positions[i]);
+                let opcode = modified_gates.get(&i).cloned().unwrap_or(opcode);
+                new_circuit.push(opcode);
+                new_acir_opcode_positions.push(opcode_position);
             }
         }
         (new_circuit, new_acir_opcode_positions)

compiler/noirc_evaluator/src/acir/mod.rs

@@ -24,12 +24,10 @@ mod big_int;
 mod brillig_directive;
 mod generated_acir;
 
+use crate::brillig::brillig_gen::gen_brillig_for;
 use crate::brillig::{
     brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext,
-    brillig_ir::{
-        artifact::{BrilligParameter, GeneratedBrillig},
-        BrilligContext,
-    },
+    brillig_ir::artifact::{BrilligParameter, GeneratedBrillig},
     Brillig,
 };
 use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
@@ -518,7 +516,7 @@ impl<'a> Context<'a> {
         let outputs: Vec<AcirType> =
             vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into());
 
-        let code = self.gen_brillig_for(main_func, arguments.clone(), brillig)?;
+        let code = gen_brillig_for(main_func, arguments.clone(), brillig)?;
 
         // We specifically do not attempt execution of the brillig code being generated as this can result in it being
         // replaced with constraints on witnesses to the program outputs.
@@ -878,8 +876,7 @@ impl<'a> Context<'a> {
                                         None,
                                     )?
                                 } else {
-                                    let code =
-                                        self.gen_brillig_for(func, arguments.clone(), brillig)?;
+                                    let code = gen_brillig_for(func, arguments.clone(), brillig)?;
                                     let generated_pointer =
                                         self.shared_context.new_generated_pointer();
                                     let output_values = self.acir_context.brillig_call(
@@ -999,47 +996,6 @@ impl<'a> Context<'a> {
             .collect()
     }
 
-    fn gen_brillig_for(
-        &self,
-        func: &Function,
-        arguments: Vec<BrilligParameter>,
-        brillig: &Brillig,
-    ) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
-        // Create the entry point artifact
-        let mut entry_point = BrilligContext::new_entry_point_artifact(
-            arguments,
-            BrilligFunctionContext::return_values(func),
-            func.id(),
-        );
-        entry_point.name = func.name().to_string();
-
-        // Link the entry point with all dependencies
-        while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() {
-            let artifact = &brillig.find_by_label(unresolved_fn_label.clone());
-            let artifact = match artifact {
-                Some(artifact) => artifact,
-                None => {
-                    return Err(InternalError::General {
-                        message: format!("Cannot find linked fn {unresolved_fn_label}"),
-                        call_stack: CallStack::new(),
-                    })
-                }
-            };
-            entry_point.link_with(artifact);
-            // Insert the range of opcode locations occupied by a procedure
-            if let Some(procedure_id) = &artifact.procedure {
-                let num_opcodes = entry_point.byte_code.len();
-                let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
-                // We subtract one as to keep the range inclusive on both ends
-                entry_point
-                    .procedure_locations
-                    .insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1));
-            }
-        }
-        // Generate the final bytecode
-        Ok(entry_point.finish())
-    }
-
     /// Handles an ArrayGet or ArraySet instruction.
     /// To set an index of the array (and create a new array in doing so), pass Some(value) for
     /// store_value. To just retrieve an index of the array, pass None for store_value.

compiler/noirc_evaluator/src/brillig/brillig_gen.rs

@@ -9,11 +9,17 @@ mod variable_liveness;
 use acvm::FieldElement;
 
 use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext};
-use super::brillig_ir::{
-    artifact::{BrilligArtifact, Label},
-    BrilligContext,
+use super::{
+    brillig_ir::{
+        artifact::{BrilligArtifact, BrilligParameter, GeneratedBrillig, Label},
+        BrilligContext,
+    },
+    Brillig,
+};
+use crate::{
+    errors::InternalError,
+    ssa::ir::{dfg::CallStack, function::Function},
 };
-use crate::ssa::ir::function::Function;
 
 /// Converting an SSA function into Brillig bytecode.
 pub(crate) fn convert_ssa_function(
@@ -36,3 +42,43 @@ pub(crate) fn convert_ssa_function(
     artifact.name = func.name().to_string();
     artifact
 }
+
+pub(crate) fn gen_brillig_for(
+    func: &Function,
+    arguments: Vec<BrilligParameter>,
+    brillig: &Brillig,
+) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
+    // Create the entry point artifact
+    let mut entry_point = BrilligContext::new_entry_point_artifact(
+        arguments,
+        FunctionContext::return_values(func),
+        func.id(),
+    );
+    entry_point.name = func.name().to_string();
+
+    // Link the entry point with all dependencies
+    while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() {
+        let artifact = &brillig.find_by_label(unresolved_fn_label.clone());
+        let artifact = match artifact {
+            Some(artifact) => artifact,
+            None => {
+                return Err(InternalError::General {
+                    message: format!("Cannot find linked fn {unresolved_fn_label}"),
+                    call_stack: CallStack::new(),
+                })
+            }
+        };
+        entry_point.link_with(artifact);
+        // Insert the range of opcode locations occupied by a procedure
+        if let Some(procedure_id) = &artifact.procedure {
+            let num_opcodes = entry_point.byte_code.len();
+            let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
+            // We subtract one as to keep the range inclusive on both ends
+            entry_point
+                .procedure_locations
+                .insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1));
+        }
+    }
+    // Generate the final bytecode
+    Ok(entry_point.finish())
+}

compiler/noirc_evaluator/src/ssa.rs

@@ -140,6 +140,23 @@ pub(crate) fn optimize_into_acir(
         ssa.to_brillig(options.enable_brillig_logging)
     });
 
+    let ssa_gen_span = span!(Level::TRACE, "ssa_generation");
+    let ssa_gen_span_guard = ssa_gen_span.enter();
+
+    let ssa = SsaBuilder {
+        ssa,
+        print_ssa_passes: options.enable_ssa_logging,
+        print_codegen_timings: options.print_codegen_timings,
+    }
+    .run_pass(
+        |ssa| ssa.fold_constants_with_brillig(&brillig),
+        "After Constant Folding with Brillig:",
+    )
+    .run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:")
+    .finish();
+
+    drop(ssa_gen_span_guard);
+
     let artifacts = time("SSA to ACIR", options.print_codegen_timings, || {
         ssa.into_acir(&brillig, options.expression_width)
     })?;

compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs

@@ -806,7 +806,8 @@ fn simplify_derive_generators(
                 results.push(is_infinite);
             }
             let len = results.len();
-            let typ = Type::Array(vec![Type::field()].into(), len);
+            let typ =
+                Type::Array(vec![Type::field(), Type::field(), Type::unsigned(1)].into(), len / 3);
             let result = make_array(dfg, results.into(), typ, block, call_stack);
             SimplifyResult::SimplifiedTo(result)
         } else {
@@ -816,3 +817,34 @@ fn simplify_derive_generators(
         unreachable!("Unexpected number of arguments to derive_generators");
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::ssa::{opt::assert_normalized_ssa_equals, Ssa};
+
+    #[test]
+    fn simplify_derive_generators_has_correct_type() {
+        let src = "
+            brillig(inline) fn main f0 {
+              b0():
+                v0 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
+
+                // This call was previously incorrectly simplified to something that returned `[Field; 3]`
+                v2 = call derive_pedersen_generators(v0, u32 0) -> [(Field, Field, u1); 1]
+
+                return v2
+            }
+            ";
+        let ssa = Ssa::from_str(src).unwrap();
+
+        let expected = "
+            brillig(inline) fn main f0 {
+              b0():
+                v15 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
+                v19 = make_array [Field 3728882899078719075161482178784387565366481897740339799480980287259621149274, Field -9903063709032878667290627648209915537972247634463802596148419711785767431332, u1 0] : [(Field, Field, u1); 1]
+                return v19
+            }
+            ";
+        assert_normalized_ssa_equals(ssa, expected);
+    }
+}