From a7b9d20a962c33d8585502fd00739138c6d79aca Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 16 Apr 2024 20:51:50 +0100 Subject: [PATCH] feat: Brillig pointer codegen and execution (#5737) Resolves https://github.com/noir-lang/noir/issues/3907 This PR builds upon https://github.com/AztecProtocol/aztec-packages/pull/5709. These changes do not yet include a Brillig stdlib and removal of the `Brillig` opcode itself. The generated stdlib Brillig (such as for quotient) is not created in the same manner as other Brillig calls which are generated during SSA. I have decided to leave this for another follow-up where we can actually remove `Brillig`. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: jfecher --- .../acvm-repo/acir/src/circuit/mod.rs | 4 + .../acvm-repo/acvm/src/pwg/brillig.rs | 54 ++++- noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs | 72 +++++- noir/noir-repo/acvm-repo/acvm/tests/solver.rs | 32 ++- .../acvm-repo/acvm_js/src/execute.rs | 25 +- .../src/brillig/brillig_gen/brillig_block.rs | 2 +- .../noirc_evaluator/src/brillig/brillig_ir.rs | 2 +- .../src/brillig/brillig_ir/artifact.rs | 2 +- .../compiler/noirc_evaluator/src/ssa.rs | 26 +- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 100 ++++++++ .../ssa/acir_gen/acir_ir/generated_acir.rs | 32 +++ .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 227 +++++++++++++++--- .../src/ssa/ssa_gen/program.rs | 29 ++- .../fold_after_inlined_calls/Nargo.toml | 7 + .../fold_after_inlined_calls/Prover.toml | 1 + .../fold_after_inlined_calls/src/main.nr | 14 ++ .../noir-repo/tooling/debugger/src/context.rs | 17 +- noir/noir-repo/tooling/debugger/src/dap.rs | 4 + noir/noir-repo/tooling/debugger/src/lib.rs | 4 +- noir/noir-repo/tooling/debugger/src/repl.rs | 25 +- .../tooling/nargo/src/ops/execute.rs | 21 +- .../tooling/nargo_cli/src/cli/debug_cmd.rs | 1 + yarn-project/circuits.js/src/constants.gen.ts | 2 +- 23 files changed, 615 insertions(+), 88 deletions(-) create mode 100644 noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/Nargo.toml create mode 100644 noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/Prover.toml create mode 100644 noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/src/main.nr diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs index 0c79cfdb815..188205b124c 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs @@ -266,6 +266,10 @@ impl std::fmt::Display for Program { writeln!(f, "func {}", func_index)?; writeln!(f, "{}", function)?; } + for (func_index, function) in self.unconstrained_functions.iter().enumerate() { + writeln!(f, "unconstrained func {}", func_index)?; + writeln!(f, "{:?}", function.bytecode)?; + } Ok(()) } } diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs index 9982fa890c2..67faf7f5007 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use acir::{ - brillig::{ForeignCallParam, ForeignCallResult}, + brillig::{ForeignCallParam, ForeignCallResult, Opcode as BrilligOpcode}, circuit::{ brillig::{Brillig, BrilligInputs, BrilligOutputs}, opcodes::BlockId, @@ -46,9 +46,9 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { /// Assigns the zero value to all outputs of the given [`Brillig`] bytecode. pub(super) fn zero_out_brillig_outputs( initial_witness: &mut WitnessMap, - brillig: &Brillig, + outputs: &[BrilligOutputs], ) -> Result<(), OpcodeResolutionError> { - for output in &brillig.outputs { + for output in outputs { match output { BrilligOutputs::Simple(witness) => { insert_value(witness, FieldElement::zero(), initial_witness)?; @@ -63,6 +63,7 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { Ok(()) } + // TODO: Delete this old method once `Brillig` is deleted /// Constructs a solver for a Brillig block given the bytecode and initial /// witness. pub(crate) fn new( @@ -72,13 +73,45 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { bb_solver: &'b B, acir_index: usize, ) -> Result { + let vm = Self::setup_brillig_vm( + initial_witness, + memory, + &brillig.inputs, + &brillig.bytecode, + bb_solver, + )?; + Ok(Self { vm, acir_index }) + } + + /// Constructs a solver for a Brillig block given the bytecode and initial + /// witness. + pub(crate) fn new_call( + initial_witness: &WitnessMap, + memory: &HashMap, + inputs: &'b [BrilligInputs], + brillig_bytecode: &'b [BrilligOpcode], + bb_solver: &'b B, + acir_index: usize, + ) -> Result { + let vm = + Self::setup_brillig_vm(initial_witness, memory, inputs, brillig_bytecode, bb_solver)?; + Ok(Self { vm, acir_index }) + } + + fn setup_brillig_vm( + initial_witness: &WitnessMap, + memory: &HashMap, + inputs: &[BrilligInputs], + brillig_bytecode: &'b [BrilligOpcode], + bb_solver: &'b B, + ) -> Result, OpcodeResolutionError> { // Set input values let mut calldata: Vec = Vec::new(); // Each input represents an expression or array of expressions to evaluate. // Iterate over each input and evaluate the expression(s) associated with it. // Push the results into memory. // If a certain expression is not solvable, we stall the ACVM and do not proceed with Brillig VM execution. - for input in &brillig.inputs { + for input in inputs { match input { BrilligInputs::Single(expr) => match get_value(expr, initial_witness) { Ok(value) => calldata.push(value), @@ -118,8 +151,8 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { // Instantiate a Brillig VM given the solved calldata // along with the Brillig bytecode. - let vm = VM::new(calldata, &brillig.bytecode, vec![], bb_solver); - Ok(Self { vm, acir_index }) + let vm = VM::new(calldata, brillig_bytecode, vec![], bb_solver); + Ok(vm) } pub fn get_memory(&self) -> &[MemoryValue] { @@ -204,13 +237,13 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { pub(crate) fn finalize( self, witness: &mut WitnessMap, - brillig: &Brillig, + outputs: &[BrilligOutputs], ) -> Result<(), OpcodeResolutionError> { // Finish the Brillig execution by writing the outputs to the witness map let vm_status = self.vm.get_status(); match vm_status { VMStatus::Finished { return_data_offset, return_data_size } => { - self.write_brillig_outputs(witness, return_data_offset, return_data_size, brillig)?; + self.write_brillig_outputs(witness, return_data_offset, return_data_size, outputs)?; Ok(()) } _ => panic!("Brillig VM has not completed execution"), @@ -222,12 +255,12 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { witness_map: &mut WitnessMap, return_data_offset: usize, return_data_size: usize, - brillig: &Brillig, + outputs: &[BrilligOutputs], ) -> Result<(), OpcodeResolutionError> { // Write VM execution results into the witness map let memory = self.vm.get_memory(); let mut current_ret_data_idx = return_data_offset; - for output in brillig.outputs.iter() { + for output in outputs.iter() { match output { BrilligOutputs::Simple(witness) => { insert_value(witness, memory[current_ret_data_idx].to_field(), witness_map)?; @@ -242,6 +275,7 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { } } } + assert!( current_ret_data_idx == return_data_offset + return_data_size, "Brillig VM did not write the expected number of return values" diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs index e7c009f87fa..5362af961a7 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use acir::{ brillig::ForeignCallResult, - circuit::{opcodes::BlockId, Opcode, OpcodeLocation}, + circuit::{brillig::BrilligBytecode, opcodes::BlockId, Opcode, OpcodeLocation}, native_types::{Expression, Witness, WitnessMap}, BlackBoxFunc, FieldElement, }; @@ -165,10 +165,18 @@ pub struct ACVM<'a, B: BlackBoxFunctionSolver> { /// Represents the outputs of all ACIR calls during an ACVM process /// List is appended onto by the caller upon reaching a [ACVMStatus::RequiresAcirCall] acir_call_results: Vec>, + + // Each unconstrained function referenced in the program + unconstrained_functions: &'a [BrilligBytecode], } impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { - pub fn new(backend: &'a B, opcodes: &'a [Opcode], initial_witness: WitnessMap) -> Self { + pub fn new( + backend: &'a B, + opcodes: &'a [Opcode], + initial_witness: WitnessMap, + unconstrained_functions: &'a [BrilligBytecode], + ) -> Self { let status = if opcodes.is_empty() { ACVMStatus::Solved } else { ACVMStatus::InProgress }; ACVM { status, @@ -181,6 +189,7 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { brillig_solver: None, acir_call_counter: 0, acir_call_results: Vec::default(), + unconstrained_functions, } } @@ -324,9 +333,10 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { Ok(Some(foreign_call)) => return self.wait_for_foreign_call(foreign_call), res => res.map(|_| ()), }, - Opcode::BrilligCall { .. } => { - todo!("implement brillig pointer handling"); - } + Opcode::BrilligCall { .. } => match self.solve_brillig_call_opcode() { + Ok(Some(foreign_call)) => return self.wait_for_foreign_call(foreign_call), + res => res.map(|_| ()), + }, Opcode::Call { .. } => match self.solve_call_opcode() { Ok(Some(input_values)) => return self.wait_for_acir_call(input_values), res => res.map(|_| ()), @@ -381,7 +391,8 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { let witness = &mut self.witness_map; if is_predicate_false(witness, &brillig.predicate)? { - return BrilligSolver::::zero_out_brillig_outputs(witness, brillig).map(|_| None); + return BrilligSolver::::zero_out_brillig_outputs(witness, &brillig.outputs) + .map(|_| None); } // If we're resuming execution after resolving a foreign call then @@ -407,7 +418,51 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { } BrilligSolverStatus::Finished => { // Write execution outputs - solver.finalize(witness, brillig)?; + solver.finalize(witness, &brillig.outputs)?; + Ok(None) + } + } + } + + fn solve_brillig_call_opcode( + &mut self, + ) -> Result, OpcodeResolutionError> { + let Opcode::BrilligCall { id, inputs, outputs, predicate } = + &self.opcodes[self.instruction_pointer] + else { + unreachable!("Not executing a Brillig opcode"); + }; + + let witness = &mut self.witness_map; + if is_predicate_false(witness, predicate)? { + return BrilligSolver::::zero_out_brillig_outputs(witness, outputs).map(|_| None); + } + + // If we're resuming execution after resolving a foreign call then + // there will be a cached `BrilligSolver` to avoid recomputation. + let mut solver: BrilligSolver<'_, B> = match self.brillig_solver.take() { + Some(solver) => solver, + None => BrilligSolver::new_call( + witness, + &self.block_solvers, + inputs, + &self.unconstrained_functions[*id as usize].bytecode, + self.backend, + self.instruction_pointer, + )?, + }; + match solver.solve()? { + BrilligSolverStatus::ForeignCallWait(foreign_call) => { + // Cache the current state of the solver + self.brillig_solver = Some(solver); + Ok(Some(foreign_call)) + } + BrilligSolverStatus::InProgress => { + unreachable!("Brillig solver still in progress") + } + BrilligSolverStatus::Finished => { + // Write execution outputs + solver.finalize(witness, outputs)?; Ok(None) } } @@ -425,7 +480,8 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { }; if should_skip { - let resolution = BrilligSolver::::zero_out_brillig_outputs(witness, brillig); + let resolution = + BrilligSolver::::zero_out_brillig_outputs(witness, &brillig.outputs); return StepResult::Status(self.handle_opcode_resolution(resolution)); } diff --git a/noir/noir-repo/acvm-repo/acvm/tests/solver.rs b/noir/noir-repo/acvm-repo/acvm/tests/solver.rs index ec7a6467ff5..f009e2c05b8 100644 --- a/noir/noir-repo/acvm-repo/acvm/tests/solver.rs +++ b/noir/noir-repo/acvm-repo/acvm/tests/solver.rs @@ -104,8 +104,9 @@ fn inversion_brillig_oracle_equivalence() { (Witness(2), FieldElement::from(3u128)), ]) .into(); - - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments); + let unconstrained_functions = vec![]; + let mut acvm = + ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -241,8 +242,9 @@ fn double_inversion_brillig_oracle() { (Witness(9), FieldElement::from(10u128)), ]) .into(); - - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments); + let unconstrained_functions = vec![]; + let mut acvm = + ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -370,8 +372,9 @@ fn oracle_dependent_execution() { let witness_assignments = BTreeMap::from([(w_x, FieldElement::from(2u128)), (w_y, FieldElement::from(2u128))]).into(); - - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments); + let unconstrained_functions = vec![]; + let mut acvm = + ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -474,8 +477,9 @@ fn brillig_oracle_predicate() { (Witness(2), FieldElement::from(3u128)), ]) .into(); - - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments); + let unconstrained_functions = vec![]; + let mut acvm = + ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved"); @@ -509,7 +513,8 @@ fn unsatisfied_opcode_resolved() { values.insert(d, FieldElement::from(2_i128)); let opcodes = vec![Opcode::AssertZero(opcode_a)]; - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, values); + let unconstrained_functions = vec![]; + let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, values, &unconstrained_functions); let solver_status = acvm.solve(); assert_eq!( solver_status, @@ -591,8 +596,8 @@ fn unsatisfied_opcode_resolved_brillig() { values.insert(w_result, FieldElement::from(0_i128)); let opcodes = vec![brillig_opcode, Opcode::AssertZero(opcode_a)]; - - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, values); + let unconstrained_functions = vec![]; + let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, values, &unconstrained_functions); let solver_status = acvm.solve(); assert_eq!( solver_status, @@ -635,8 +640,9 @@ fn memory_operations() { }); let opcodes = vec![init, read_op, expression]; - - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, initial_witness); + let unconstrained_functions = vec![]; + let mut acvm = + ACVM::new(&StubbedBlackBoxSolver, &opcodes, initial_witness, &unconstrained_functions); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs index 8bc56295992..2fab684467e 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs +++ b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs @@ -1,5 +1,6 @@ use std::{future::Future, pin::Pin}; +use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::BlackBoxFunctionSolver; use acvm::{ acir::circuit::{Circuit, Program}, @@ -181,7 +182,12 @@ async fn execute_program_with_native_program_and_return( initial_witness: JsWitnessMap, foreign_call_executor: &ForeignCallHandler, ) -> Result { - let executor = ProgramExecutor::new(&program.functions, &solver.0, foreign_call_executor); + let executor = ProgramExecutor::new( + &program.functions, + &program.unconstrained_functions, + &solver.0, + foreign_call_executor, + ); let witness_stack = executor.execute(initial_witness.into()).await?; Ok(witness_stack) @@ -190,6 +196,8 @@ async fn execute_program_with_native_program_and_return( struct ProgramExecutor<'a, B: BlackBoxFunctionSolver> { functions: &'a [Circuit], + unconstrained_functions: &'a [BrilligBytecode], + blackbox_solver: &'a B, foreign_call_handler: &'a ForeignCallHandler, @@ -198,10 +206,16 @@ struct ProgramExecutor<'a, B: BlackBoxFunctionSolver> { impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { fn new( functions: &'a [Circuit], + unconstrained_functions: &'a [BrilligBytecode], blackbox_solver: &'a B, foreign_call_handler: &'a ForeignCallHandler, ) -> Self { - ProgramExecutor { functions, blackbox_solver, foreign_call_handler } + ProgramExecutor { + functions, + unconstrained_functions, + blackbox_solver, + foreign_call_handler, + } } async fn execute(&self, initial_witness: WitnessMap) -> Result { @@ -220,7 +234,12 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { witness_stack: &'a mut WitnessStack, ) -> Pin> + 'a>> { Box::pin(async { - let mut acvm = ACVM::new(self.blackbox_solver, &circuit.opcodes, initial_witness); + let mut acvm = ACVM::new( + self.blackbox_solver, + &circuit.opcodes, + initial_witness, + self.unconstrained_functions, + ); loop { let solver_status = acvm.solve(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 369d8e78cda..22407fc5695 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1785,7 +1785,7 @@ pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type (Type::Numeric(lhs_type), Type::Numeric(rhs_type)) => { assert_eq!( lhs_type, rhs_type, - "lhs and rhs types in a binary operation are always the same" + "lhs and rhs types in a binary operation are always the same but got {lhs_type} and {rhs_type}" ); Type::Numeric(*lhs_type) } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index fdb03abe59f..29210dc6f99 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -140,7 +140,7 @@ pub(crate) mod tests { &self, _public_key_x: &FieldElement, _public_key_y: &FieldElement, - _signature: &[u8], + _signature: &[u8; 64], _message: &[u8], ) -> Result { Ok(true) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 8ce15ba4e73..8bd1bfda78f 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -17,7 +17,7 @@ pub(crate) enum BrilligParameter { /// The result of compiling and linking brillig artifacts. /// This is ready to run bytecode with attached metadata. -#[derive(Debug)] +#[derive(Debug, Default)] pub(crate) struct GeneratedBrillig { pub(crate) byte_code: Vec, pub(crate) locations: BTreeMap, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs index ce4a9bbe9f1..760340f1a88 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs @@ -14,7 +14,9 @@ use crate::{ errors::{RuntimeError, SsaReport}, }; use acvm::acir::{ - circuit::{Circuit, ExpressionWidth, Program as AcirProgram, PublicInputs}, + circuit::{ + brillig::BrilligBytecode, Circuit, ExpressionWidth, Program as AcirProgram, PublicInputs, + }, native_types::Witness, }; @@ -35,14 +37,16 @@ pub mod ssa_gen; /// Optimize the given program by converting it into SSA /// form and performing optimizations there. When finished, -/// convert the final SSA into ACIR and return it. +/// convert the final SSA into an ACIR program and return it. +/// An ACIR program is made up of both ACIR functions +/// and Brillig functions for unconstrained execution. pub(crate) fn optimize_into_acir( program: Program, print_passes: bool, print_brillig_trace: bool, force_brillig_output: bool, print_timings: bool, -) -> Result, RuntimeError> { +) -> Result<(Vec, Vec), RuntimeError> { let abi_distinctness = program.return_distinctness; let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); @@ -99,6 +103,18 @@ pub struct SsaProgramArtifact { } impl SsaProgramArtifact { + fn new(unconstrained_functions: Vec) -> Self { + let program = AcirProgram { functions: Vec::default(), unconstrained_functions }; + Self { + program, + debug: Vec::default(), + warnings: Vec::default(), + main_input_witnesses: Vec::default(), + main_return_witnesses: Vec::default(), + names: Vec::default(), + } + } + fn add_circuit(&mut self, mut circuit_artifact: SsaCircuitArtifact, is_main: bool) { self.program.functions.push(circuit_artifact.circuit); self.debug.push(circuit_artifact.debug_info); @@ -130,7 +146,7 @@ pub fn create_program( let func_sigs = program.function_signatures.clone(); let recursive = program.recursive; - let generated_acirs = optimize_into_acir( + let (generated_acirs, generated_brillig) = optimize_into_acir( program, enable_ssa_logging, enable_brillig_logging, @@ -143,7 +159,7 @@ pub fn create_program( "The generated ACIRs should match the supplied function signatures" ); - let mut program_artifact = SsaProgramArtifact::default(); + let mut program_artifact = SsaProgramArtifact::new(generated_brillig); // For setting up the ABI we need separately specify main's input and return witnesses let mut is_main = true; for (acir, func_sig) in generated_acirs.into_iter().zip(func_sigs) { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 775571f4a41..3294eb16695 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1463,6 +1463,7 @@ impl AcirContext { id } + // TODO: Delete this method once we remove the `Brillig` opcode pub(crate) fn brillig( &mut self, predicate: AcirVar, @@ -1553,6 +1554,105 @@ impl AcirContext { Ok(outputs_var) } + #[allow(clippy::too_many_arguments)] + pub(crate) fn brillig_call( + &mut self, + predicate: AcirVar, + generated_brillig: &GeneratedBrillig, + inputs: Vec, + outputs: Vec, + attempt_execution: bool, + unsafe_return_values: bool, + brillig_function_index: u32, + ) -> Result, RuntimeError> { + let brillig_inputs = try_vecmap(inputs, |i| -> Result<_, InternalError> { + match i { + AcirValue::Var(var, _) => Ok(BrilligInputs::Single(self.var_to_expression(var)?)), + AcirValue::Array(vars) => { + let mut var_expressions: Vec = Vec::new(); + for var in vars { + self.brillig_array_input(&mut var_expressions, var)?; + } + Ok(BrilligInputs::Array(var_expressions)) + } + AcirValue::DynamicArray(AcirDynamicArray { block_id, .. }) => { + Ok(BrilligInputs::MemoryArray(block_id)) + } + } + })?; + + // Optimistically try executing the brillig now, if we can complete execution they just return the results. + // This is a temporary measure pending SSA optimizations being applied to Brillig which would remove constant-input opcodes (See #2066) + // + // We do _not_ want to do this in the situation where the `main` function is unconstrained, as if execution succeeds + // the entire program will be replaced with witness constraints to its outputs. + if attempt_execution { + if let Some(brillig_outputs) = + self.execute_brillig(&generated_brillig.byte_code, &brillig_inputs, &outputs) + { + return Ok(brillig_outputs); + } + } + + // Otherwise we must generate ACIR for it and execute at runtime. + let mut brillig_outputs = Vec::new(); + let outputs_var = vecmap(outputs, |output| match output { + AcirType::NumericType(_) => { + let witness_index = self.acir_ir.next_witness_index(); + brillig_outputs.push(BrilligOutputs::Simple(witness_index)); + let var = self.add_data(AcirVarData::Witness(witness_index)); + AcirValue::Var(var, output.clone()) + } + AcirType::Array(element_types, size) => { + let (acir_value, witnesses) = self.brillig_array_output(&element_types, size); + brillig_outputs.push(BrilligOutputs::Array(witnesses)); + acir_value + } + }); + let predicate = self.var_to_expression(predicate)?; + + self.acir_ir.brillig_call( + Some(predicate), + generated_brillig, + brillig_inputs, + brillig_outputs, + brillig_function_index, + ); + + fn range_constraint_value( + context: &mut AcirContext, + value: &AcirValue, + ) -> Result<(), RuntimeError> { + match value { + AcirValue::Var(var, typ) => { + let numeric_type = match typ { + AcirType::NumericType(numeric_type) => numeric_type, + _ => unreachable!("`AcirValue::Var` may only hold primitive values"), + }; + context.range_constrain_var(*var, numeric_type, None)?; + } + AcirValue::Array(values) => { + for value in values { + range_constraint_value(context, value)?; + } + } + AcirValue::DynamicArray(_) => { + unreachable!("Brillig opcodes cannot return dynamic arrays") + } + } + Ok(()) + } + + // This is a hack to ensure that if we're compiling a brillig entrypoint function then + // we don't also add a number of range constraints. + if !unsafe_return_values { + for output_var in &outputs_var { + range_constraint_value(self, output_var)?; + } + } + Ok(outputs_var) + } + fn brillig_array_input( &mut self, var_expressions: &mut Vec, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index e16eb2172be..999ff2ddb5d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -589,6 +589,7 @@ impl GeneratedAcir { Ok(()) } + // TODO: Delete this method once we remove the `Brillig` opcode pub(crate) fn brillig( &mut self, predicate: Option, @@ -617,6 +618,37 @@ impl GeneratedAcir { } } + pub(crate) fn brillig_call( + &mut self, + predicate: Option, + generated_brillig: &GeneratedBrillig, + inputs: Vec, + outputs: Vec, + brillig_function_index: u32, + ) { + let opcode = + AcirOpcode::BrilligCall { id: brillig_function_index, inputs, outputs, predicate }; + self.push_opcode(opcode); + for (brillig_index, call_stack) in generated_brillig.locations.iter() { + self.locations.insert( + OpcodeLocation::Brillig { + acir_index: self.opcodes.len() - 1, + brillig_index: *brillig_index, + }, + call_stack.clone(), + ); + } + for (brillig_index, message) in generated_brillig.assert_messages.iter() { + self.assert_messages.insert( + OpcodeLocation::Brillig { + acir_index: self.opcodes.len() - 1, + brillig_index: *brillig_index, + }, + message.clone(), + ); + } + } + pub(crate) fn last_acir_opcode_location(&self) -> OpcodeLocation { OpcodeLocation::Acir(self.opcodes.len() - 1) } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 76cc8acf878..afd749485d7 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1,12 +1,13 @@ //! This file holds the pass to convert from Noir's SSA IR to ACIR. mod acir_ir; -use std::collections::HashSet; +use std::collections::{BTreeMap, HashSet}; use std::fmt::Debug; use self::acir_ir::acir_variable::{AcirContext, AcirType, AcirVar}; use super::function_builder::data_bus::DataBus; use super::ir::dfg::CallStack; +use super::ir::function::FunctionId; use super::ir::instruction::{ConstrainError, UserDefinedConstrainError}; use super::{ ir::{ @@ -28,6 +29,7 @@ use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport}; use crate::ssa::ir::function::InlineType; pub(crate) use acir_ir::generated_acir::GeneratedAcir; +use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::acir::native_types::Witness; use acvm::acir::BlackBoxFunc; use acvm::{ @@ -39,9 +41,47 @@ use im::Vector; use iter_extended::{try_vecmap, vecmap}; use noirc_frontend::Distinctness; +#[derive(Default)] +struct SharedContext { + /// Final list of Brillig functions which will be part of the final program + /// This is shared across `Context` structs as we want one list of Brillig + /// functions across all ACIR artifacts + generated_brillig: Vec, + + /// Maps SSA function index -> Final generated Brillig artifact index. + /// There can be Brillig functions specified in SSA which do not act as + /// entry points in ACIR (e.g. only called by other Brillig functions) + /// This mapping is necessary to use the correct function pointer for a Brillig call. + brillig_generated_func_pointers: BTreeMap, +} + +impl SharedContext { + fn generated_brillig_pointer(&self, func_id: &FunctionId) -> Option<&u32> { + self.brillig_generated_func_pointers.get(func_id) + } + + fn generated_brillig(&self, func_pointer: usize) -> &GeneratedBrillig { + &self.generated_brillig[func_pointer] + } + + fn insert_generated_brillig( + &mut self, + func_id: FunctionId, + generated_pointer: u32, + code: GeneratedBrillig, + ) { + self.brillig_generated_func_pointers.insert(func_id, generated_pointer); + self.generated_brillig.push(code); + } + + fn new_generated_pointer(&self) -> u32 { + self.generated_brillig.len() as u32 + } +} + /// Context struct for the acir generation pass. /// May be similar to the Evaluator struct in the current SSA IR. -struct Context { +struct Context<'a> { /// Maps SSA values to `AcirVar`. /// /// This is needed so that we only create a single @@ -91,6 +131,9 @@ struct Context { max_block_id: u32, data_bus: DataBus, + + /// Contains state that is generated and also used across ACIR functions + shared_context: &'a mut SharedContext, } #[derive(Clone)] @@ -181,11 +224,12 @@ impl Ssa { self, brillig: &Brillig, abi_distinctness: Distinctness, - ) -> Result, RuntimeError> { + ) -> Result<(Vec, Vec), RuntimeError> { let mut acirs = Vec::new(); // TODO: can we parallelise this? + let mut shared_context = SharedContext::default(); for function in self.functions.values() { - let context = Context::new(); + let context = Context::new(&mut shared_context); if let Some(mut generated_acir) = context.convert_ssa_function(&self, function, brillig)? { @@ -194,6 +238,10 @@ impl Ssa { } } + let brillig = vecmap(shared_context.generated_brillig, |brillig| BrilligBytecode { + bytecode: brillig.byte_code, + }); + // TODO: check whether doing this for a single circuit's return witnesses is correct. // We probably need it for all foldable circuits, as any circuit being folded is essentially an entry point. However, I do not know how that // plays a part when we potentially want not inlined functions normally as part of the compiler. @@ -215,15 +263,15 @@ impl Ssa { .collect(); main_func_acir.return_witnesses = distinct_return_witness; - Ok(acirs) } - Distinctness::DuplicationAllowed => Ok(acirs), + Distinctness::DuplicationAllowed => {} } + Ok((acirs, brillig)) } } -impl Context { - fn new() -> Context { +impl<'a> Context<'a> { + fn new(shared_context: &'a mut SharedContext) -> Context<'a> { let mut acir_context = AcirContext::default(); let current_side_effects_enabled_var = acir_context.add_constant(FieldElement::one()); @@ -237,6 +285,7 @@ impl Context { internal_mem_block_lengths: HashMap::default(), max_block_id: 0, data_bus: DataBus::default(), + shared_context, } } @@ -576,12 +625,12 @@ impl Context { sum + dfg.try_get_array_length(*result_id).unwrap_or(1) }); - let acir_program_id = ssa - .id_to_index + let acir_function_id = ssa + .entry_point_to_generated_index .get(id) .expect("ICE: should have an associated final index"); let output_vars = self.acir_context.call_acir_function( - *acir_program_id, + *acir_function_id, inputs, output_count, self.current_side_effects_enabled_var, @@ -603,24 +652,50 @@ impl Context { ); } } - let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg)); - let arguments = self.gen_brillig_parameters(arguments, dfg); - - let code = self.gen_brillig_for(func, arguments, brillig)?; let outputs: Vec = vecmap(result_ids, |result_id| { dfg.type_of_value(*result_id).into() }); - let output_values = self.acir_context.brillig( - self.current_side_effects_enabled_var, - code, - inputs, - outputs, - true, - false, - )?; + // Check whether we have already generated Brillig for this function + // If we have, re-use the generated code to set-up the Brillig call. + let output_values = if let Some(generated_pointer) = + self.shared_context.generated_brillig_pointer(id) + { + let code = self + .shared_context + .generated_brillig(*generated_pointer as usize); + self.acir_context.brillig_call( + self.current_side_effects_enabled_var, + code, + inputs, + outputs, + true, + false, + *generated_pointer, + )? + } else { + let arguments = self.gen_brillig_parameters(arguments, dfg); + let code = self.gen_brillig_for(func, arguments, brillig)?; + let generated_pointer = + self.shared_context.new_generated_pointer(); + let output_values = self.acir_context.brillig_call( + self.current_side_effects_enabled_var, + &code, + inputs, + outputs, + true, + false, + generated_pointer, + )?; + self.shared_context.insert_generated_brillig( + *id, + generated_pointer, + code, + ); + output_values + }; // Compiler sanity check assert_eq!(result_ids.len(), output_values.len(), "ICE: The number of Brillig output values should match the result ids in SSA"); @@ -2447,19 +2522,27 @@ mod test { }, }; - fn build_basic_foo_with_return(builder: &mut FunctionBuilder, foo_id: FunctionId) { - // acir(fold) fn foo f1 { + fn build_basic_foo_with_return( + builder: &mut FunctionBuilder, + foo_id: FunctionId, + is_brillig_func: bool, + ) { + // fn foo f1 { // b0(v0: Field, v1: Field): // v2 = eq v0, v1 // constrain v2 == u1 0 // return v0 // } - builder.new_function("foo".into(), foo_id, InlineType::Fold); + if is_brillig_func { + builder.new_brillig_function("foo".into(), foo_id); + } else { + builder.new_function("foo".into(), foo_id, InlineType::Fold); + } let foo_v0 = builder.add_parameter(Type::field()); let foo_v1 = builder.add_parameter(Type::field()); let foo_equality_check = builder.insert_binary(foo_v0, BinaryOp::Eq, foo_v1); - let zero = builder.field_constant(0u128); + let zero = builder.numeric_constant(0u128, Type::unsigned(1)); builder.insert_constrain(foo_equality_check, zero, None); builder.terminate_with_return(vec![foo_v0]); } @@ -2493,11 +2576,11 @@ mod test { builder.insert_constrain(main_call1_results[0], main_call2_results[0], None); builder.terminate_with_return(vec![]); - build_basic_foo_with_return(&mut builder, foo_id); + build_basic_foo_with_return(&mut builder, foo_id, false); let ssa = builder.finish(); - let acir_functions = ssa + let (acir_functions, _) = ssa .into_acir(&Brillig::default(), noirc_frontend::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); // Expected result: @@ -2589,11 +2672,11 @@ mod test { builder.insert_constrain(main_call1_results[0], main_call2_results[0], None); builder.terminate_with_return(vec![]); - build_basic_foo_with_return(&mut builder, foo_id); + build_basic_foo_with_return(&mut builder, foo_id, false); let ssa = builder.finish(); - let acir_functions = ssa + let (acir_functions, _) = ssa .into_acir(&Brillig::default(), noirc_frontend::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); // The expected result should look very similar to the abvoe test expect that the input witnesses of the `Call` @@ -2680,11 +2763,11 @@ mod test { .to_vec(); builder.terminate_with_return(vec![foo_call[0]]); - build_basic_foo_with_return(&mut builder, foo_id); + build_basic_foo_with_return(&mut builder, foo_id, false); let ssa = builder.finish(); - let acir_functions = ssa + let (acir_functions, _) = ssa .into_acir(&Brillig::default(), noirc_frontend::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); @@ -2744,4 +2827,82 @@ mod test { _ => panic!("Expected only Call opcode"), } } + + // Test that given multiple calls to the same brillig function we generate only one bytecode + // and the appropriate Brillig call opcodes are generated + #[test] + fn multiple_brillig_calls_one_bytecode() { + // acir(inline) fn main f0 { + // b0(v0: Field, v1: Field): + // v3 = call f1(v0, v1) + // v4 = call f1(v0, v1) + // v5 = call f1(v0, v1) + // v6 = call f1(v0, v1) + // return + // } + // brillig fn foo f1 { + // b0(v0: Field, v1: Field): + // v2 = eq v0, v1 + // constrain v2 == u1 0 + // return v0 + // } + // brillig fn foo f2 { + // b0(v0: Field, v1: Field): + // v2 = eq v0, v1 + // constrain v2 == u1 0 + // return v0 + // } + let foo_id = Id::test_new(0); + let mut builder = FunctionBuilder::new("main".into(), foo_id); + let main_v0 = builder.add_parameter(Type::field()); + let main_v1 = builder.add_parameter(Type::field()); + + let foo_id = Id::test_new(1); + let foo = builder.import_function(foo_id); + let bar_id = Id::test_new(2); + let bar = builder.import_function(bar_id); + + // Insert multiple calls to the same Brillig function + builder.insert_call(foo, vec![main_v0, main_v1], vec![Type::field()]).to_vec(); + builder.insert_call(foo, vec![main_v0, main_v1], vec![Type::field()]).to_vec(); + builder.insert_call(foo, vec![main_v0, main_v1], vec![Type::field()]).to_vec(); + // Interleave a call to a separate Brillig function to make sure that we can call multiple separate Brillig functions + builder.insert_call(bar, vec![main_v0, main_v1], vec![Type::field()]).to_vec(); + builder.insert_call(foo, vec![main_v0, main_v1], vec![Type::field()]).to_vec(); + builder.insert_call(bar, vec![main_v0, main_v1], vec![Type::field()]).to_vec(); + builder.terminate_with_return(vec![]); + + build_basic_foo_with_return(&mut builder, foo_id, true); + build_basic_foo_with_return(&mut builder, bar_id, true); + + let ssa = builder.finish(); + let brillig = ssa.to_brillig(false); + println!("{}", ssa); + + let (acir_functions, brillig_functions) = ssa + .into_acir(&brillig, noirc_frontend::Distinctness::Distinct) + .expect("Should compile manually written SSA into ACIR"); + + assert_eq!(acir_functions.len(), 1, "Should only have a `main` ACIR function"); + assert_eq!( + brillig_functions.len(), + 2, + "Should only have generated a single Brillig function" + ); + + let main_acir = &acir_functions[0]; + let main_opcodes = main_acir.opcodes(); + assert_eq!(main_opcodes.len(), 6, "Should have four calls to f1 and two calls to f2"); + + // We should only have `BrilligCall` opcodes in `main` + for (i, opcode) in main_opcodes.iter().enumerate() { + match opcode { + Opcode::BrilligCall { id, .. } => { + let expected_id = if i == 3 || i == 5 { 1 } else { 0 }; + assert_eq!(*id, expected_id, "Expected an id of {expected_id} but got {id}"); + } + _ => panic!("Expected only Brillig call opcode"), + } + } + } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index 9995c031145..b05a2cbc741 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -3,7 +3,7 @@ use std::{collections::BTreeMap, fmt::Display}; use iter_extended::btree_map; use crate::ssa::ir::{ - function::{Function, FunctionId}, + function::{Function, FunctionId, RuntimeType}, map::AtomicCounter, }; @@ -12,7 +12,11 @@ pub(crate) struct Ssa { pub(crate) functions: BTreeMap, pub(crate) main_id: FunctionId, pub(crate) next_id: AtomicCounter, - pub(crate) id_to_index: BTreeMap, + /// Maps SSA entry point function ID -> Final generated ACIR artifact index. + /// There can be functions specified in SSA which do not act as ACIR entry points. + /// This mapping is necessary to use the correct function pointer for an ACIR call, + /// as the final program artifact will be a list of only entry point functions. + pub(crate) entry_point_to_generated_index: BTreeMap, } impl Ssa { @@ -27,9 +31,26 @@ impl Ssa { (f.id(), f) }); - let id_to_index = btree_map(functions.iter().enumerate(), |(i, (id, _))| (*id, i as u32)); + let entry_point_to_generated_index = btree_map( + functions + .iter() + .filter(|(_, func)| { + let runtime = func.runtime(); + match func.runtime() { + RuntimeType::Acir(_) => runtime.is_entry_point() || func.id() == main_id, + RuntimeType::Brillig => false, + } + }) + .enumerate(), + |(i, (id, _))| (*id, i as u32), + ); - Self { functions, main_id, next_id: AtomicCounter::starting_after(max_id), id_to_index } + Self { + functions, + main_id, + next_id: AtomicCounter::starting_after(max_id), + entry_point_to_generated_index, + } } /// Returns the entry-point function of the program diff --git a/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/Nargo.toml b/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/Nargo.toml new file mode 100644 index 00000000000..d23924af083 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "fold_after_inlined_calls" +type = "bin" +authors = [""] +compiler_version = ">=0.27.0" + +[dependencies] \ No newline at end of file diff --git a/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/Prover.toml b/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/Prover.toml new file mode 100644 index 00000000000..4dd6b405159 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/Prover.toml @@ -0,0 +1 @@ +x = "1" diff --git a/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/src/main.nr b/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/src/main.nr new file mode 100644 index 00000000000..84c81190b9b --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/fold_after_inlined_calls/src/main.nr @@ -0,0 +1,14 @@ +fn main(x: u32) { + // We want to call a foldable function after a call to a function that is set to be inlined + assert(increment(x) == x + 1); + foo(x); +} + +#[fold] +fn foo(x: u32) { + assert(x == 1); +} + +fn increment(x: u32) -> u32 { + x + 1 +} diff --git a/noir/noir-repo/tooling/debugger/src/context.rs b/noir/noir-repo/tooling/debugger/src/context.rs index b211832518d..5639407df54 100644 --- a/noir/noir-repo/tooling/debugger/src/context.rs +++ b/noir/noir-repo/tooling/debugger/src/context.rs @@ -1,4 +1,5 @@ use crate::foreign_calls::DebugForeignCallExecutor; +use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::acir::circuit::{Circuit, Opcode, OpcodeLocation}; use acvm::acir::native_types::{Witness, WitnessMap}; use acvm::brillig_vm::brillig::ForeignCallResult; @@ -42,10 +43,17 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, foreign_call_executor: Box, + unconstrained_functions: &'a [BrilligBytecode], ) -> Self { let source_to_opcodes = build_source_to_opcode_debug_mappings(debug_artifact); Self { - acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), + // TODO: need to handle brillig pointer in the debugger + acvm: ACVM::new( + blackbox_solver, + &circuit.opcodes, + initial_witness, + unconstrained_functions, + ), brillig_solver: None, foreign_call_executor, debug_artifact, @@ -630,6 +638,7 @@ fn build_source_to_opcode_debug_mappings( result } +// TODO: update all debugger tests to use unconstrained brillig pointers #[cfg(test)] mod tests { use super::*; @@ -696,12 +705,14 @@ mod tests { let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)); + let brillig_funcs = &vec![]; let mut context = DebugContext::new( &StubbedBlackBoxSolver, circuit, debug_artifact, initial_witness, foreign_call_executor, + brillig_funcs, ); assert_eq!(context.get_current_opcode_location(), Some(OpcodeLocation::Acir(0))); @@ -803,12 +814,14 @@ mod tests { let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)); + let brillig_funcs = &vec![]; let mut context = DebugContext::new( &StubbedBlackBoxSolver, circuit, debug_artifact, initial_witness, foreign_call_executor, + brillig_funcs, ); // set breakpoint @@ -860,12 +873,14 @@ mod tests { let circuit = Circuit { opcodes, ..Circuit::default() }; let debug_artifact = DebugArtifact { debug_symbols: vec![], file_map: BTreeMap::new(), warnings: vec![] }; + let brillig_funcs = &vec![]; let context = DebugContext::new( &StubbedBlackBoxSolver, &circuit, &debug_artifact, WitnessMap::new(), Box::new(DefaultDebugForeignCallExecutor::new(true)), + brillig_funcs, ); assert_eq!(context.offset_opcode_location(&None, 0), (None, 0)); diff --git a/noir/noir-repo/tooling/debugger/src/dap.rs b/noir/noir-repo/tooling/debugger/src/dap.rs index ea3204ebbbc..dc83337a973 100644 --- a/noir/noir-repo/tooling/debugger/src/dap.rs +++ b/noir/noir-repo/tooling/debugger/src/dap.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use std::io::{Read, Write}; use std::str::FromStr; +use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::acir::circuit::{Circuit, OpcodeLocation}; use acvm::acir::native_types::WitnessMap; use acvm::BlackBoxFunctionSolver; @@ -64,6 +65,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { circuit: &'a Circuit, debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, + unconstrained_functions: &'a [BrilligBytecode], ) -> Self { let context = DebugContext::new( solver, @@ -71,6 +73,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { debug_artifact, initial_witness, Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)), + unconstrained_functions, ); Self { server, @@ -613,6 +616,7 @@ pub fn run_session( &program.program.functions[0], &debug_artifact, initial_witness, + &program.program.unconstrained_functions, ); session.run_loop() diff --git a/noir/noir-repo/tooling/debugger/src/lib.rs b/noir/noir-repo/tooling/debugger/src/lib.rs index 4a25e3417a0..a8fc61c893f 100644 --- a/noir/noir-repo/tooling/debugger/src/lib.rs +++ b/noir/noir-repo/tooling/debugger/src/lib.rs @@ -9,6 +9,7 @@ use std::io::{Read, Write}; use ::dap::errors::ServerError; use ::dap::server::Server; +use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::BlackBoxFunctionSolver; use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap}; @@ -22,8 +23,9 @@ pub fn debug_circuit( circuit: &Circuit, debug_artifact: DebugArtifact, initial_witness: WitnessMap, + unconstrained_functions: &[BrilligBytecode], ) -> Result, NargoError> { - repl::run(blackbox_solver, circuit, &debug_artifact, initial_witness) + repl::run(blackbox_solver, circuit, &debug_artifact, initial_witness, unconstrained_functions) } pub fn run_dap_loop( diff --git a/noir/noir-repo/tooling/debugger/src/repl.rs b/noir/noir-repo/tooling/debugger/src/repl.rs index e30d519b62e..2a92698e5ce 100644 --- a/noir/noir-repo/tooling/debugger/src/repl.rs +++ b/noir/noir-repo/tooling/debugger/src/repl.rs @@ -1,5 +1,6 @@ use crate::context::{DebugCommandResult, DebugContext}; +use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::acir::circuit::{Circuit, Opcode, OpcodeLocation}; use acvm::acir::native_types::{Witness, WitnessMap}; use acvm::{BlackBoxFunctionSolver, FieldElement}; @@ -20,6 +21,7 @@ pub struct ReplDebugger<'a, B: BlackBoxFunctionSolver> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, last_result: DebugCommandResult, + unconstrained_functions: &'a [BrilligBytecode], } impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { @@ -28,6 +30,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { circuit: &'a Circuit, debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, + unconstrained_functions: &'a [BrilligBytecode], ) -> Self { let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)); @@ -37,6 +40,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { debug_artifact, initial_witness.clone(), foreign_call_executor, + unconstrained_functions, ); let last_result = if context.get_current_opcode_location().is_none() { // handle circuit with no opcodes @@ -44,7 +48,15 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } else { DebugCommandResult::Ok }; - Self { context, blackbox_solver, circuit, debug_artifact, initial_witness, last_result } + Self { + context, + blackbox_solver, + circuit, + debug_artifact, + initial_witness, + last_result, + unconstrained_functions, + } } pub fn show_current_vm_status(&self) { @@ -271,6 +283,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { self.debug_artifact, self.initial_witness.clone(), foreign_call_executor, + self.unconstrained_functions, ); for opcode_location in breakpoints { self.context.add_breakpoint(opcode_location); @@ -361,9 +374,15 @@ pub fn run( circuit: &Circuit, debug_artifact: &DebugArtifact, initial_witness: WitnessMap, + unconstrained_functions: &[BrilligBytecode], ) -> Result, NargoError> { - let context = - RefCell::new(ReplDebugger::new(blackbox_solver, circuit, debug_artifact, initial_witness)); + let context = RefCell::new(ReplDebugger::new( + blackbox_solver, + circuit, + debug_artifact, + initial_witness, + unconstrained_functions, + )); let ref_context = &context; ref_context.borrow().show_current_vm_status(); diff --git a/noir/noir-repo/tooling/nargo/src/ops/execute.rs b/noir/noir-repo/tooling/nargo/src/ops/execute.rs index 5ee0c6a3891..c2c0208b395 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/execute.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/execute.rs @@ -1,3 +1,4 @@ +use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::acir::circuit::Program; use acvm::acir::native_types::WitnessStack; use acvm::brillig_vm::brillig::ForeignCallResult; @@ -12,6 +13,9 @@ use super::foreign_calls::{ForeignCallExecutor, NargoForeignCallResult}; struct ProgramExecutor<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> { functions: &'a [Circuit], + + unconstrained_functions: &'a [BrilligBytecode], + // This gets built as we run through the program looking at each function call witness_stack: WitnessStack, @@ -23,11 +27,13 @@ struct ProgramExecutor<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> { impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, B, F> { fn new( functions: &'a [Circuit], + unconstrained_functions: &'a [BrilligBytecode], blackbox_solver: &'a B, foreign_call_executor: &'a mut F, ) -> Self { ProgramExecutor { functions, + unconstrained_functions, witness_stack: WitnessStack::default(), blackbox_solver, foreign_call_executor, @@ -44,7 +50,12 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, circuit: &Circuit, initial_witness: WitnessMap, ) -> Result { - let mut acvm = ACVM::new(self.blackbox_solver, &circuit.opcodes, initial_witness); + let mut acvm = ACVM::new( + self.blackbox_solver, + &circuit.opcodes, + initial_witness, + self.unconstrained_functions, + ); // This message should be resolved by a nargo foreign call only when we have an unsatisfied assertion. let mut assert_message: Option = None; @@ -151,8 +162,12 @@ pub fn execute_program( ) -> Result { let main = &program.functions[0]; - let mut executor = - ProgramExecutor::new(&program.functions, blackbox_solver, foreign_call_executor); + let mut executor = ProgramExecutor::new( + &program.functions, + &program.unconstrained_functions, + blackbox_solver, + foreign_call_executor, + ); let main_witness = executor.execute_circuit(main, initial_witness)?; executor.witness_stack.push(0, main_witness); diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs index 4f3e2886b2e..5a9a33c0acd 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -242,6 +242,7 @@ pub(crate) fn debug_program( &compiled_program.program.functions[0], debug_artifact, initial_witness, + &compiled_program.program.unconstrained_functions, ) .map_err(CliError::from) } diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index abea48aef02..f3833975def 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -68,7 +68,7 @@ export const REGISTERER_UNCONSTRAINED_FUNCTION_BROADCASTED_MAGIC_VALUE = 0xe7af816635466f128568edb04c9fa024f6c87fb9010fdbffa68b3d99n; export const DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631n; -export const DEPLOYER_CONTRACT_ADDRESS = 0x1d2c645a18ab4d81672dd779bf1ea4fa2c0b4a2f09bf59d3fb4ee408299d00b2n; +export const DEPLOYER_CONTRACT_ADDRESS = 0x2b9c3612fc04623512ab47f68537c7b63a90b3efcc39f622f42d1820bfeb93acn; export const L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH = 17; export const MAX_NOTE_FIELDS_LENGTH = 20; export const GET_NOTE_ORACLE_RETURN_LENGTH = 23;