feat: Sync from noir

.noir-sync-commit

@@ -1 +1 @@
-13856a121125b1ccca15919942081a5d157d280e
+852c87ae9ecdd441ee4c2ab3e78e86b2da07d8a4

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs

@@ -1,3 +1,6 @@
+use noirc_errors::debug_info::ProcedureDebugId;
+use serde::{Deserialize, Serialize};
+
 mod array_copy;
 mod array_reverse;
 mod check_max_stack_depth;
@@ -14,11 +17,9 @@ use array_copy::compile_array_copy_procedure;
 use array_reverse::compile_array_reverse_procedure;
 use check_max_stack_depth::compile_check_max_stack_depth_procedure;
 use mem_copy::compile_mem_copy_procedure;
-use noirc_errors::debug_info::ProcedureDebugId;
 use prepare_vector_insert::compile_prepare_vector_insert_procedure;
 use prepare_vector_push::compile_prepare_vector_push_procedure;
 use revert_with_string::compile_revert_with_string_procedure;
-use serde::{Deserialize, Serialize};
 use vector_copy::compile_vector_copy_procedure;
 use vector_pop_back::compile_vector_pop_back_procedure;
 use vector_pop_front::compile_vector_pop_front_procedure;

noir/noir-repo/compiler/noirc_evaluator/src/lib.rs

@@ -12,7 +12,6 @@ pub mod ssa;
 pub mod brillig;
 
 pub use ssa::create_program;
-
 pub use ssa::ir::instruction::ErrorType;
 
 /// Trims leading whitespace from each line of the input string, according to

noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs

@@ -96,28 +96,28 @@ pub(crate) fn optimize_into_acir(
     .run_pass(Ssa::remove_paired_rc, "After Removing Paired rc_inc & rc_decs:")
     .run_pass(Ssa::separate_runtime, "After Runtime Separation:")
     .run_pass(Ssa::resolve_is_unconstrained, "After Resolving IsUnconstrained:")
-    .run_pass(|ssa| ssa.inline_functions(options.inliner_aggressiveness), "After Inlining:")
+    .run_pass(|ssa| ssa.inline_functions(options.inliner_aggressiveness), "After Inlining (1st):")
     // Run mem2reg with the CFG separated into blocks
-    .run_pass(Ssa::mem2reg, "After Mem2Reg:")
-    .run_pass(Ssa::simplify_cfg, "After Simplifying:")
+    .run_pass(Ssa::mem2reg, "After Mem2Reg (1st):")
+    .run_pass(Ssa::simplify_cfg, "After Simplifying (1st):")
     .run_pass(Ssa::as_slice_optimization, "After `as_slice` optimization")
     .try_run_pass(
         Ssa::evaluate_static_assert_and_assert_constant,
         "After `static_assert` and `assert_constant`:",
     )?
     .try_run_pass(Ssa::unroll_loops_iteratively, "After Unrolling:")?
-    .run_pass(Ssa::simplify_cfg, "After Simplifying:")
+    .run_pass(Ssa::simplify_cfg, "After Simplifying (2nd):")
     .run_pass(Ssa::flatten_cfg, "After Flattening:")
     .run_pass(Ssa::remove_bit_shifts, "After Removing Bit Shifts:")
     // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores
-    .run_pass(Ssa::mem2reg, "After Mem2Reg:")
+    .run_pass(Ssa::mem2reg, "After Mem2Reg (2nd):")
     // Run the inlining pass again to handle functions with `InlineType::NoPredicates`.
     // Before flattening is run, we treat functions marked with the `InlineType::NoPredicates` as an entry point.
     // This pass must come immediately following `mem2reg` as the succeeding passes
     // may create an SSA which inlining fails to handle.
     .run_pass(
         |ssa| ssa.inline_functions_with_no_predicates(options.inliner_aggressiveness),
-        "After Inlining:",
+        "After Inlining (2nd):",
     )
     .run_pass(Ssa::remove_if_else, "After Remove IfElse:")
     .run_pass(Ssa::fold_constants, "After Constant Folding:")

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -13,6 +13,7 @@ use acvm::acir::circuit::opcodes::{
 };
 use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode};
 use acvm::brillig_vm::{MemoryValue, VMStatus, VM};
+use acvm::BlackBoxFunctionSolver;
 use acvm::{
     acir::AcirField,
     acir::{
@@ -107,7 +108,9 @@ impl From<NumericType> for AcirType {
 /// Context object which holds the relationship between
 /// `Variables`(AcirVar) and types such as `Expression` and `Witness`
 /// which are placed into ACIR.
-pub(crate) struct AcirContext<F: AcirField> {
+pub(crate) struct AcirContext<F: AcirField, B: BlackBoxFunctionSolver<F>> {
+    blackbox_solver: B,
+
     /// Two-way map that links `AcirVar` to `AcirVarData`.
     ///
     /// The vars object is an instance of the `TwoWayMap`, which provides a bidirectional mapping between `AcirVar` and `AcirVarData`.
@@ -132,7 +135,7 @@ pub(crate) struct AcirContext<F: AcirField> {
     pub(crate) warnings: Vec<SsaReport>,
 }
 
-impl<F: AcirField> AcirContext<F> {
+impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
     pub(crate) fn set_expression_width(&mut self, expression_width: ExpressionWidth) {
         self.expression_width = expression_width;
     }
@@ -1758,8 +1761,8 @@ impl<F: AcirField> AcirContext<F> {
             brillig_stdlib_func,
         );
 
-        fn range_constraint_value<G: AcirField>(
-            context: &mut AcirContext<G>,
+        fn range_constraint_value<G: AcirField, C: BlackBoxFunctionSolver<G>>(
+            context: &mut AcirContext<G, C>,
             value: &AcirValue,
         ) -> Result<(), RuntimeError> {
             match value {
@@ -1878,7 +1881,7 @@ impl<F: AcirField> AcirContext<F> {
         inputs: &[BrilligInputs<F>],
         outputs_types: &[AcirType],
     ) -> Option<Vec<AcirValue>> {
-        let mut memory = (execute_brillig(code, inputs)?).into_iter();
+        let mut memory = (execute_brillig(code, &self.blackbox_solver, inputs)?).into_iter();
 
         let outputs_var = vecmap(outputs_types.iter(), |output| match output {
             AcirType::NumericType(_) => {
@@ -2171,8 +2174,9 @@ pub(crate) struct AcirVar(usize);
 /// Returns the finished state of the Brillig VM if execution can complete.
 ///
 /// Returns `None` if complete execution of the Brillig bytecode is not possible.
-fn execute_brillig<F: AcirField>(
+fn execute_brillig<F: AcirField, B: BlackBoxFunctionSolver<F>>(
     code: &[BrilligOpcode<F>],
+    blackbox_solver: &B,
     inputs: &[BrilligInputs<F>],
 ) -> Option<Vec<MemoryValue<F>>> {
     // Set input values
@@ -2198,12 +2202,8 @@ fn execute_brillig<F: AcirField>(
     }
 
     // Instantiate a Brillig VM given the solved input registers and memory, along with the Brillig bytecode.
-    //
-    // We pass a stubbed solver here as a concrete solver implies a field choice which conflicts with this function
-    // being generic.
-    let solver = acvm::blackbox_solver::StubbedBlackBoxSolver;
     let profiling_active = false;
-    let mut vm = VM::new(calldata, code, Vec::new(), &solver, profiling_active);
+    let mut vm = VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active);
 
     // Run the Brillig VM on these inputs, bytecode, etc!
     let vm_status = vm.process_opcodes();

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -30,6 +30,7 @@ use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunction
 use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
 pub(crate) use acir_ir::generated_acir::GeneratedAcir;
 use acvm::acir::circuit::opcodes::{AcirFunctionId, BlockType};
+use bn254_blackbox_solver::Bn254BlackBoxSolver;
 use noirc_frontend::monomorphization::ast::InlineType;
 
 use acvm::acir::circuit::brillig::{BrilligBytecode, BrilligFunctionId};
@@ -157,7 +158,7 @@ struct Context<'a> {
     current_side_effects_enabled_var: AcirVar,
 
     /// Manages and builds the `AcirVar`s to which the converted SSA values refer.
-    acir_context: AcirContext<FieldElement>,
+    acir_context: AcirContext<FieldElement, Bn254BlackBoxSolver>,
 
     /// Track initialized acir dynamic arrays
     ///

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/array_set.rs

@@ -28,14 +28,21 @@ impl Ssa {
 
 impl Function {
     pub(crate) fn array_set_optimization(&mut self) {
+        if matches!(self.runtime(), RuntimeType::Brillig(_)) {
+            // Brillig is supposed to use refcounting to decide whether to mutate an array;
+            // array mutation was only meant for ACIR. We could use it with Brillig as well,
+            // but then some of the optimizations that we can do in ACIR around shared
+            // references have to be skipped, which makes it more cumbersome.
+            return;
+        }
+
         let reachable_blocks = self.reachable_blocks();
 
         if !self.runtime().is_entry_point() {
             assert_eq!(reachable_blocks.len(), 1, "Expected there to be 1 block remaining in Acir function for array_set optimization");
         }
 
-        let mut context =
-            Context::new(&self.dfg, matches!(self.runtime(), RuntimeType::Brillig(_)));
+        let mut context = Context::new(&self.dfg);
 
         for block in reachable_blocks.iter() {
             context.analyze_last_uses(*block);
@@ -50,20 +57,18 @@ impl Function {
 
 struct Context<'f> {
     dfg: &'f DataFlowGraph,
-    is_brillig_runtime: bool,
     array_to_last_use: HashMap<ValueId, InstructionId>,
     instructions_that_can_be_made_mutable: HashSet<InstructionId>,
     // Mapping of an array that comes from a load and whether the address
-    // it was loaded from is a reference parameter.
+    // it was loaded from is a reference parameter passed to the block.
     arrays_from_load: HashMap<ValueId, bool>,
     inner_nested_arrays: HashMap<ValueId, InstructionId>,
 }
 
 impl<'f> Context<'f> {
-    fn new(dfg: &'f DataFlowGraph, is_brillig_runtime: bool) -> Self {
+    fn new(dfg: &'f DataFlowGraph) -> Self {
         Context {
             dfg,
-            is_brillig_runtime,
             array_to_last_use: HashMap::default(),
             instructions_that_can_be_made_mutable: HashSet::default(),
             arrays_from_load: HashMap::default(),
@@ -85,43 +90,41 @@ impl<'f> Context<'f> {
                         self.instructions_that_can_be_made_mutable.remove(&existing);
                     }
                 }
-                Instruction::ArraySet { array, value, .. } => {
+                Instruction::ArraySet { array, .. } => {
                     let array = self.dfg.resolve(*array);
 
                     if let Some(existing) = self.array_to_last_use.insert(array, *instruction_id) {
                         self.instructions_that_can_be_made_mutable.remove(&existing);
                     }
-                    if self.is_brillig_runtime {
-                        let value = self.dfg.resolve(*value);
-
-                        if let Some(existing) = self.inner_nested_arrays.get(&value) {
-                            self.instructions_that_can_be_made_mutable.remove(existing);
-                        }
-                        let result = self.dfg.instruction_results(*instruction_id)[0];
-                        self.inner_nested_arrays.insert(result, *instruction_id);
-                    }
 
                     // If the array we are setting does not come from a load we can safely mark it mutable.
                     // If the array comes from a load we may potentially being mutating an array at a reference
                     // that is loaded from by other values.
                     let terminator = self.dfg[block_id].unwrap_terminator();
+
                     // If we are in a return block we are not concerned about the array potentially being mutated again.
                     let is_return_block =
                         matches!(terminator, TerminatorInstruction::Return { .. });
+
                     // We also want to check that the array is not part of the terminator arguments, as this means it is used again.
-                    let mut array_in_terminator = false;
+                    let mut is_array_in_terminator = false;
                     terminator.for_each_value(|value| {
-                        if value == array {
-                            array_in_terminator = true;
+                        // The terminator can contain original IDs, while the SSA has replaced the array value IDs; we need to resolve to compare.
+                        if !is_array_in_terminator && self.dfg.resolve(value) == array {
+                            is_array_in_terminator = true;
                         }
                     });
-                    if let Some(is_from_param) = self.arrays_from_load.get(&array) {
+
+                    let can_mutate = if let Some(is_from_param) = self.arrays_from_load.get(&array)
+                    {
                         // If the array was loaded from a reference parameter, we cannot
                         // safely mark that array mutable as it may be shared by another value.
-                        if !is_from_param && is_return_block {
-                            self.instructions_that_can_be_made_mutable.insert(*instruction_id);
-                        }
-                    } else if !array_in_terminator {
+                        !is_from_param && is_return_block
+                    } else {
+                        !is_array_in_terminator
+                    };
+
+                    if can_mutate {
                         self.instructions_that_can_be_made_mutable.insert(*instruction_id);
                     }
                 }