diff --git a/.noir-sync-commit b/.noir-sync-commit index 2863ab07e89..2a6270fb774 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -1969ce39378f633e88adedf43b747724b89ed7d7 +9704bd0abfe2dba1e7a4aef6cdb6cc83d70b929e diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/brillig.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/brillig.rs index e75d335d52b..7f87aabf9d5 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/brillig.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/brillig.rs @@ -33,7 +33,7 @@ pub struct Brillig { /// This is purely a wrapper struct around a list of Brillig opcode's which represents /// a full Brillig function to be executed by the Brillig VM. /// This is stored separately on a program and accessed through a [BrilligPointer]. -#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Default, Debug)] pub struct BrilligBytecode { pub bytecode: Vec, } 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 b94e02e5119..3f5e4129dd0 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 @@ -1,5 +1,5 @@ use super::big_int::BigIntContext; -use super::generated_acir::GeneratedAcir; +use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX}; use crate::brillig::brillig_gen::brillig_directive; use crate::brillig::brillig_ir::artifact::GeneratedBrillig; use crate::errors::{InternalError, RuntimeError, SsaReport}; @@ -326,13 +326,15 @@ impl AcirContext { // Compute the inverse with brillig code let inverse_code = brillig_directive::directive_invert(); - let results = self.brillig( + let results = self.brillig_call( predicate, - inverse_code, + &inverse_code, vec![AcirValue::Var(var, AcirType::field())], vec![AcirType::field()], true, false, + PLACEHOLDER_BRILLIG_INDEX, + Some(BrilligStdlibFunc::Inverse), )?; let inverted_var = Self::expect_one_var(results); @@ -711,9 +713,9 @@ impl AcirContext { } let [q_value, r_value]: [AcirValue; 2] = self - .brillig( + .brillig_call( predicate, - brillig_directive::directive_quotient(bit_size + 1), + &brillig_directive::directive_quotient(bit_size + 1), vec![ AcirValue::Var(lhs, AcirType::unsigned(bit_size)), AcirValue::Var(rhs, AcirType::unsigned(bit_size)), @@ -721,6 +723,8 @@ impl AcirContext { vec![AcirType::unsigned(max_q_bits), AcirType::unsigned(max_rhs_bits)], true, false, + PLACEHOLDER_BRILLIG_INDEX, + Some(BrilligStdlibFunc::Quotient(bit_size + 1)), )? .try_into() .expect("quotient only returns two values"); @@ -1464,97 +1468,6 @@ impl AcirContext { id } - // TODO: Delete this method once we remove the `Brillig` opcode - pub(crate) fn brillig( - &mut self, - predicate: AcirVar, - generated_brillig: GeneratedBrillig, - inputs: Vec, - outputs: Vec, - attempt_execution: bool, - unsafe_return_values: bool, - ) -> Result, RuntimeError> { - let b_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, &b_inputs, &outputs) - { - return Ok(brillig_outputs); - } - } - - // Otherwise we must generate ACIR for it and execute at runtime. - let mut b_outputs = Vec::new(); - let outputs_var = vecmap(outputs, |output| match output { - AcirType::NumericType(_) => { - let witness_index = self.acir_ir.next_witness_index(); - b_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); - b_outputs.push(BrilligOutputs::Array(witnesses)); - acir_value - } - }); - let predicate = self.var_to_expression(predicate)?; - self.acir_ir.brillig(Some(predicate), generated_brillig, b_inputs, b_outputs); - - 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) - } - #[allow(clippy::too_many_arguments)] pub(crate) fn brillig_call( &mut self, @@ -1565,6 +1478,7 @@ impl AcirContext { attempt_execution: bool, unsafe_return_values: bool, brillig_function_index: u32, + brillig_stdlib_func: Option, ) -> Result, RuntimeError> { let brillig_inputs = try_vecmap(inputs, |i| -> Result<_, InternalError> { match i { @@ -1618,6 +1532,7 @@ impl AcirContext { brillig_inputs, brillig_outputs, brillig_function_index, + brillig_stdlib_func, ); fn range_constraint_value( 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 999ff2ddb5d..0b04d1b63ab 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 @@ -10,7 +10,7 @@ use crate::{ use acvm::acir::{ circuit::{ - brillig::{Brillig as AcvmBrillig, BrilligInputs, BrilligOutputs}, + brillig::{BrilligInputs, BrilligOutputs}, opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode}, OpcodeLocation, }, @@ -24,6 +24,12 @@ use acvm::{ use iter_extended::vecmap; use num_bigint::BigUint; +/// Brillig calls such as for the Brillig std lib are resolved only after code generation is finished. +/// This index should be used when adding a Brillig call during code generation. +/// Code generation should then keep track of that unresolved call opcode which will be resolved with the +/// correct function index after code generation. +pub(crate) const PLACEHOLDER_BRILLIG_INDEX: u32 = 0; + #[derive(Debug, Default)] /// The output of the Acir-gen pass, which should only be produced for entry point Acir functions pub(crate) struct GeneratedAcir { @@ -62,6 +68,29 @@ pub(crate) struct GeneratedAcir { /// Name for the corresponding entry point represented by this Acir-gen output. /// Only used for debugging and benchmarking purposes pub(crate) name: String, + + /// Maps the opcode index to a Brillig std library function call. + /// As to avoid passing the ACIR gen shared context into each individual ACIR + /// we can instead keep this map and resolve the Brillig calls at the end of code generation. + pub(crate) brillig_stdlib_func_locations: BTreeMap, +} + +#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] +pub(crate) enum BrilligStdlibFunc { + Inverse, + // The Brillig quotient code is different depending upon the bit size. + Quotient(u32), +} + +impl BrilligStdlibFunc { + pub(crate) fn get_generated_brillig(&self) -> GeneratedBrillig { + match self { + BrilligStdlibFunc::Inverse => brillig_directive::directive_invert(), + BrilligStdlibFunc::Quotient(bit_size) => { + brillig_directive::directive_quotient(*bit_size) + } + } + } } impl GeneratedAcir { @@ -456,7 +485,14 @@ impl GeneratedAcir { let inverse_code = brillig_directive::directive_invert(); let inputs = vec![BrilligInputs::Single(expr)]; let outputs = vec![BrilligOutputs::Simple(inverted_witness)]; - self.brillig(Some(Expression::one()), inverse_code, inputs, outputs); + self.brillig_call( + Some(Expression::one()), + &inverse_code, + inputs, + outputs, + PLACEHOLDER_BRILLIG_INDEX, + Some(BrilligStdlibFunc::Inverse), + ); inverted_witness } @@ -589,35 +625,6 @@ impl GeneratedAcir { Ok(()) } - // TODO: Delete this method once we remove the `Brillig` opcode - pub(crate) fn brillig( - &mut self, - predicate: Option, - generated_brillig: GeneratedBrillig, - inputs: Vec, - outputs: Vec, - ) { - let opcode = AcirOpcode::Brillig(AcvmBrillig { - inputs, - outputs, - bytecode: generated_brillig.byte_code, - predicate, - }); - self.push_opcode(opcode); - for (brillig_index, call_stack) in generated_brillig.locations { - self.locations.insert( - OpcodeLocation::Brillig { acir_index: self.opcodes.len() - 1, brillig_index }, - call_stack, - ); - } - for (brillig_index, message) in generated_brillig.assert_messages { - self.assert_messages.insert( - OpcodeLocation::Brillig { acir_index: self.opcodes.len() - 1, brillig_index }, - message, - ); - } - } - pub(crate) fn brillig_call( &mut self, predicate: Option, @@ -625,10 +632,16 @@ impl GeneratedAcir { inputs: Vec, outputs: Vec, brillig_function_index: u32, + stdlib_func: Option, ) { let opcode = AcirOpcode::BrilligCall { id: brillig_function_index, inputs, outputs, predicate }; self.push_opcode(opcode); + if let Some(stdlib_func) = stdlib_func { + self.brillig_stdlib_func_locations + .insert(self.last_acir_opcode_location(), stdlib_func); + } + for (brillig_index, call_stack) in generated_brillig.locations.iter() { self.locations.insert( OpcodeLocation::Brillig { @@ -649,6 +662,22 @@ impl GeneratedAcir { } } + // We can only resolve the Brillig stdlib after having processed the entire ACIR + pub(crate) fn resolve_brillig_stdlib_call( + &mut self, + opcode_location: OpcodeLocation, + brillig_function_index: u32, + ) { + let acir_index = match opcode_location { + OpcodeLocation::Acir(index) => index, + _ => panic!("should not have brillig index"), + }; + match &mut self.opcodes[acir_index] { + AcirOpcode::BrilligCall { id, .. } => *id = brillig_function_index, + _ => panic!("expected brillig call opcode"), + } + } + 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 8b16ad51ca1..ee236df8eac 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 @@ -5,6 +5,7 @@ use std::collections::{BTreeMap, HashSet}; use std::fmt::Debug; use self::acir_ir::acir_variable::{AcirContext, AcirType, AcirVar}; +use self::acir_ir::generated_acir::BrilligStdlibFunc; use super::function_builder::data_bus::DataBus; use super::ir::dfg::CallStack; use super::ir::function::FunctionId; @@ -30,6 +31,7 @@ use crate::ssa::ir::function::InlineType; pub(crate) use acir_ir::generated_acir::GeneratedAcir; use acvm::acir::circuit::brillig::BrilligBytecode; +use acvm::acir::circuit::OpcodeLocation; use acvm::acir::native_types::Witness; use acvm::acir::BlackBoxFunc; use acvm::{ @@ -55,6 +57,14 @@ struct SharedContext { /// This uses the brillig parameters in the map since using slices with different lengths /// needs to create different brillig entrypoints brillig_generated_func_pointers: BTreeMap<(FunctionId, Vec), u32>, + + /// Maps a Brillig std lib function (a handwritten primitive such as for inversion) -> Final generated Brillig artifact index. + /// A separate mapping from normal Brillig calls is necessary as these methods do not have an associated function id from SSA. + brillig_stdlib_func_pointer: HashMap, + + /// Keeps track of Brillig std lib calls per function that need to still be resolved + /// with the correct function pointer from the `brillig_stdlib_func_pointer` map. + brillig_stdlib_calls_to_resolve: HashMap>, } impl SharedContext { @@ -84,6 +94,47 @@ impl SharedContext { fn new_generated_pointer(&self) -> u32 { self.generated_brillig.len() as u32 } + + fn generate_brillig_calls_to_resolve( + &mut self, + brillig_stdlib_func: &BrilligStdlibFunc, + func_id: FunctionId, + opcode_location: OpcodeLocation, + ) { + if let Some(generated_pointer) = + self.brillig_stdlib_func_pointer.get(brillig_stdlib_func).copied() + { + self.add_call_to_resolve(func_id, (opcode_location, generated_pointer)); + } else { + let code = brillig_stdlib_func.get_generated_brillig(); + let generated_pointer = self.new_generated_pointer(); + self.insert_generated_brillig_stdlib( + *brillig_stdlib_func, + generated_pointer, + func_id, + opcode_location, + code, + ); + } + } + + /// Insert a newly generated Brillig stdlib function + fn insert_generated_brillig_stdlib( + &mut self, + brillig_stdlib_func: BrilligStdlibFunc, + generated_pointer: u32, + func_id: FunctionId, + opcode_location: OpcodeLocation, + code: GeneratedBrillig, + ) { + self.brillig_stdlib_func_pointer.insert(brillig_stdlib_func, generated_pointer); + self.add_call_to_resolve(func_id, (opcode_location, generated_pointer)); + self.generated_brillig.push(code); + } + + fn add_call_to_resolve(&mut self, func_id: FunctionId, call_to_resolve: (OpcodeLocation, u32)) { + self.brillig_stdlib_calls_to_resolve.entry(func_id).or_default().push(call_to_resolve); + } } /// Context struct for the acir generation pass. @@ -240,6 +291,35 @@ impl Ssa { if let Some(mut generated_acir) = context.convert_ssa_function(&self, function, brillig)? { + // We want to be able to insert Brillig stdlib functions anywhere during the ACIR generation process (e.g. such as on the `GeneratedAcir`). + // As we don't want a reference to the `SharedContext` on the generated ACIR itself, + // we instead store the opcode location at which a Brillig call to a std lib function occurred. + // We then defer resolving the function IDs of those Brillig functions to when we have generated Brillig + // for all normal Brillig calls. + for (opcode_location, brillig_stdlib_func) in + &generated_acir.brillig_stdlib_func_locations + { + shared_context.generate_brillig_calls_to_resolve( + brillig_stdlib_func, + function.id(), + *opcode_location, + ); + } + + // Fetch the Brillig stdlib calls to resolve for this function + if let Some(calls_to_resolve) = + shared_context.brillig_stdlib_calls_to_resolve.get(&function.id()) + { + // Resolve the Brillig stdlib calls + // We have to do a separate loop as the generated ACIR cannot be borrowed as mutable after an immutable borrow + for (opcode_location, brillig_function_pointer) in calls_to_resolve { + generated_acir.resolve_brillig_stdlib_call( + *opcode_location, + *brillig_function_pointer, + ); + } + } + generated_acir.name = function.name().to_owned(); acirs.push(generated_acir); } @@ -376,6 +456,7 @@ impl<'a> Context<'a> { true, // We are guaranteed to have a Brillig function pointer of `0` as main itself is marked as unconstrained 0, + None, )?; self.shared_context.insert_generated_brillig(main_func.id(), arguments, 0, code); @@ -687,6 +768,7 @@ impl<'a> Context<'a> { true, false, *generated_pointer, + None, )? } else { let code = @@ -701,6 +783,7 @@ impl<'a> Context<'a> { true, false, generated_pointer, + None, )?; self.shared_context.insert_generated_brillig( *id, @@ -2518,14 +2601,20 @@ fn can_omit_element_sizes_array(array_typ: &Type) -> bool { #[cfg(test)] mod test { + use std::collections::BTreeMap; + use acvm::{ - acir::{circuit::Opcode, native_types::Witness}, + acir::{ + circuit::{Opcode, OpcodeLocation}, + native_types::Witness, + }, FieldElement, }; use crate::{ brillig::Brillig, ssa::{ + acir_gen::acir_ir::generated_acir::BrilligStdlibFunc, function_builder::FunctionBuilder, ir::{ function::{FunctionId, InlineType}, @@ -2848,10 +2937,12 @@ mod 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) + // v7 = call f2(v0, v1) + // v8 = call f1(v0, v1) + // v9 = call f2(v0, v1) // return // } // brillig fn foo f1 { @@ -2898,11 +2989,7 @@ mod test { .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" - ); + assert_eq!(brillig_functions.len(), 2, "Should only have generated two Brillig functions"); let main_acir = &acir_functions[0]; let main_opcodes = main_acir.opcodes(); @@ -2919,4 +3006,295 @@ mod test { } } } + + // Test that given multiple primitive operations that are represented by Brillig directives (e.g. invert/quotient), + // we will only generate one bytecode and the appropriate Brillig call opcodes are generated. + #[test] + fn multiple_brillig_stdlib_calls() { + // acir(inline) fn main f0 { + // b0(v0: u32, v1: u32, v2: u32): + // v3 = div v0, v1 + // constrain v3 == v2 + // v4 = div v1, v2 + // constrain v4 == u32 1 + // return + // } + let foo_id = Id::test_new(0); + let mut builder = FunctionBuilder::new("main".into(), foo_id); + let main_v0 = builder.add_parameter(Type::unsigned(32)); + let main_v1 = builder.add_parameter(Type::unsigned(32)); + let main_v2 = builder.add_parameter(Type::unsigned(32)); + + // Call a primitive operation that uses Brillig + let v0_div_v1 = builder.insert_binary(main_v0, BinaryOp::Div, main_v1); + builder.insert_constrain(v0_div_v1, main_v2, None); + + // Call the same primitive operation again + let v1_div_v2 = builder.insert_binary(main_v1, BinaryOp::Div, main_v2); + let one = builder.numeric_constant(1u128, Type::unsigned(32)); + builder.insert_constrain(v1_div_v2, one, None); + + builder.terminate_with_return(vec![]); + + let ssa = builder.finish(); + println!("{}", ssa); + + // The Brillig bytecode we insert for the stdlib is hardcoded so we do not need to provide any + // Brillig artifacts to the ACIR gen pass. + let (acir_functions, brillig_functions) = ssa + .into_acir(&Brillig::default(), noirc_frontend::ast::Distinctness::Distinct) + .expect("Should compile manually written SSA into ACIR"); + + assert_eq!(acir_functions.len(), 1, "Should only have a `main` ACIR function"); + // We expect two brillig functions: + // - Quotient (shared between both divisions) + // - Inversion, caused by division-by-zero check (shared between both divisions) + assert_eq!(brillig_functions.len(), 2, "Should only have generated two Brillig functions"); + + let main_acir = &acir_functions[0]; + let main_opcodes = main_acir.opcodes(); + check_brillig_calls( + &acir_functions[0].brillig_stdlib_func_locations, + main_opcodes, + 0, + 4, + 0, + ); + } + + // Test that given both hardcoded Brillig directives and calls to normal Brillig functions, + // we generate a single bytecode for the directives and a single bytecode for the normal Brillig calls. + #[test] + fn brillig_stdlib_calls_with_regular_brillig_call() { + // acir(inline) fn main f0 { + // b0(v0: u32, v1: u32, v2: u32): + // v4 = div v0, v1 + // constrain v4 == v2 + // v5 = call f1(v0, v1) + // v6 = call f1(v0, v1) + // v7 = div v1, v2 + // constrain v7 == u32 1 + // return + // } + // brillig fn foo f1 { + // 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::unsigned(32)); + let main_v1 = builder.add_parameter(Type::unsigned(32)); + let main_v2 = builder.add_parameter(Type::unsigned(32)); + + let foo_id = Id::test_new(1); + let foo = builder.import_function(foo_id); + + // Call a primitive operation that uses Brillig + let v0_div_v1 = builder.insert_binary(main_v0, BinaryOp::Div, main_v1); + builder.insert_constrain(v0_div_v1, main_v2, None); + + // 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(); + + // Call the same primitive operation again + let v1_div_v2 = builder.insert_binary(main_v1, BinaryOp::Div, main_v2); + let one = builder.numeric_constant(1u128, Type::unsigned(32)); + builder.insert_constrain(v1_div_v2, one, None); + + builder.terminate_with_return(vec![]); + + build_basic_foo_with_return(&mut builder, foo_id, true); + + let ssa = builder.finish(); + // We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass. + let brillig = ssa.to_brillig(false); + println!("{}", ssa); + + let (acir_functions, brillig_functions) = ssa + .into_acir(&brillig, noirc_frontend::ast::Distinctness::Distinct) + .expect("Should compile manually written SSA into ACIR"); + + assert_eq!(acir_functions.len(), 1, "Should only have a `main` ACIR function"); + // We expect 3 brillig functions: + // - Quotient (shared between both divisions) + // - Inversion, caused by division-by-zero check (shared between both divisions) + // - Custom brillig function `foo` + assert_eq!( + brillig_functions.len(), + 3, + "Should only have generated three Brillig functions" + ); + + let main_acir = &acir_functions[0]; + let main_opcodes = main_acir.opcodes(); + check_brillig_calls( + &acir_functions[0].brillig_stdlib_func_locations, + main_opcodes, + 1, + 4, + 2, + ); + } + + // Test that given both normal Brillig calls, Brillig stdlib calls, and non-inlined ACIR calls, that we accurately generate ACIR. + #[test] + fn brillig_stdlib_calls_with_multiple_acir_calls() { + // acir(inline) fn main f0 { + // b0(v0: u32, v1: u32, v2: u32): + // v4 = div v0, v1 + // constrain v4 == v2 + // v5 = call f1(v0, v1) + // v6 = call f2(v0, v1) + // v7 = div v1, v2 + // constrain v7 == u32 1 + // return + // } + // brillig fn foo f1 { + // b0(v0: Field, v1: Field): + // v2 = eq v0, v1 + // constrain v2 == u1 0 + // return v0 + // } + // acir(fold) 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::unsigned(32)); + let main_v1 = builder.add_parameter(Type::unsigned(32)); + let main_v2 = builder.add_parameter(Type::unsigned(32)); + + 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); + + // Call a primitive operation that uses Brillig + let v0_div_v1 = builder.insert_binary(main_v0, BinaryOp::Div, main_v1); + builder.insert_constrain(v0_div_v1, main_v2, None); + + // 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(bar, vec![main_v0, main_v1], vec![Type::field()]).to_vec(); + + // Call the same primitive operation again + let v1_div_v2 = builder.insert_binary(main_v1, BinaryOp::Div, main_v2); + let one = builder.numeric_constant(1u128, Type::unsigned(32)); + builder.insert_constrain(v1_div_v2, one, None); + + builder.terminate_with_return(vec![]); + + build_basic_foo_with_return(&mut builder, foo_id, true); + build_basic_foo_with_return(&mut builder, bar_id, false); + + let ssa = builder.finish(); + // We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass. + let brillig = ssa.to_brillig(false); + println!("{}", ssa); + + let (acir_functions, brillig_functions) = ssa + .into_acir(&brillig, noirc_frontend::ast::Distinctness::Distinct) + .expect("Should compile manually written SSA into ACIR"); + + assert_eq!(acir_functions.len(), 2, "Should only have two ACIR functions"); + // We expect 3 brillig functions: + // - Quotient (shared between both divisions) + // - Inversion, caused by division-by-zero check (shared between both divisions) + // - Custom brillig function `foo` + assert_eq!( + brillig_functions.len(), + 3, + "Should only have generated three Brillig functions" + ); + + let main_acir = &acir_functions[0]; + let main_opcodes = main_acir.opcodes(); + check_brillig_calls( + &acir_functions[0].brillig_stdlib_func_locations, + main_opcodes, + 1, + 4, + 2, + ); + + let foo_acir = &acir_functions[1]; + let foo_opcodes = foo_acir.opcodes(); + check_brillig_calls(&acir_functions[1].brillig_stdlib_func_locations, foo_opcodes, 1, 1, 0); + } + + fn check_brillig_calls( + brillig_stdlib_function_locations: &BTreeMap, + opcodes: &[Opcode], + num_normal_brillig_functions: u32, + expected_num_stdlib_calls: u32, + expected_num_normal_calls: u32, + ) { + // First we check calls to the Brillig stdlib + let mut num_brillig_stdlib_calls = 0; + for (i, (opcode_location, brillig_stdlib_func)) in + brillig_stdlib_function_locations.iter().enumerate() + { + // We can take just modulo 2 to determine the expected ID as we only code generated two Brillig stdlib function + let stdlib_func_index = (i % 2) as u32; + if stdlib_func_index == 0 { + assert!(matches!(brillig_stdlib_func, BrilligStdlibFunc::Inverse)); + } else { + assert!(matches!(brillig_stdlib_func, BrilligStdlibFunc::Quotient(_))); + } + + match opcode_location { + OpcodeLocation::Acir(acir_index) => { + match opcodes[*acir_index] { + Opcode::BrilligCall { id, .. } => { + // Brillig stdlib function calls are only resolved at the end of ACIR generation so their + // IDs are expected to always reference Brillig bytecode at the end of the Brillig functions list. + // We have one normal Brillig call so we add one here to the std lib function's index within the std lib. + let expected_id = stdlib_func_index + num_normal_brillig_functions; + assert_eq!(id, expected_id, "Expected {expected_id} but got {id}"); + num_brillig_stdlib_calls += 1; + } + _ => panic!("Expected BrilligCall opcode"), + } + } + _ => panic!("Expected OpcodeLocation::Acir"), + } + } + + assert_eq!( + num_brillig_stdlib_calls, expected_num_stdlib_calls, + "Should have {expected_num_stdlib_calls} BrilligCall opcodes to stdlib functions but got {num_brillig_stdlib_calls}" + ); + + // Check the normal Brillig calls + // This check right now expects to only call one Brillig function. + let mut num_normal_brillig_calls = 0; + for (i, opcode) in opcodes.iter().enumerate() { + match opcode { + Opcode::BrilligCall { id, .. } => { + if brillig_stdlib_function_locations.get(&OpcodeLocation::Acir(i)).is_some() { + // We should have already checked Brillig stdlib functions and only want to check normal Brillig calls here + continue; + } + // We only generate one normal Brillig call so we should expect a function ID of `0` + let expected_id = 0u32; + assert_eq!(*id, expected_id, "Expected an id of {expected_id} but got {id}"); + num_normal_brillig_calls += 1; + } + _ => {} + } + } + + assert_eq!( + num_normal_brillig_calls, expected_num_normal_calls, + "Should have {expected_num_normal_calls} BrilligCall opcodes to normal Brillig functions but got {num_normal_brillig_calls}" + ); + } } diff --git a/noir/noir-repo/tooling/noir_js_backend_barretenberg/package.json b/noir/noir-repo/tooling/noir_js_backend_barretenberg/package.json index af9e47a8e63..438e91ff302 100644 --- a/noir/noir-repo/tooling/noir_js_backend_barretenberg/package.json +++ b/noir/noir-repo/tooling/noir_js_backend_barretenberg/package.json @@ -42,7 +42,7 @@ "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" }, "dependencies": { - "@aztec/bb.js": "0.35.1", + "@aztec/bb.js": "portal:../../../../barretenberg/ts", "@noir-lang/types": "workspace:*", "fflate": "^0.8.0" }, diff --git a/noir/noir-repo/yarn.lock b/noir/noir-repo/yarn.lock index e9915882fac..b45678f5d8b 100644 --- a/noir/noir-repo/yarn.lock +++ b/noir/noir-repo/yarn.lock @@ -221,19 +221,18 @@ __metadata: languageName: node linkType: hard -"@aztec/bb.js@npm:0.35.1": - version: 0.35.1 - resolution: "@aztec/bb.js@npm:0.35.1" +"@aztec/bb.js@portal:../../../../barretenberg/ts::locator=%40noir-lang%2Fbackend_barretenberg%40workspace%3Atooling%2Fnoir_js_backend_barretenberg": + version: 0.0.0-use.local + resolution: "@aztec/bb.js@portal:../../../../barretenberg/ts::locator=%40noir-lang%2Fbackend_barretenberg%40workspace%3Atooling%2Fnoir_js_backend_barretenberg" dependencies: comlink: ^4.4.1 commander: ^10.0.1 debug: ^4.3.4 tslib: ^2.4.0 bin: - bb.js: dest/node/main.js - checksum: 8e3551f059523d9494af4721a9219e2c6e63c8ed1df447a2d0daa9f8526a794758ae708bd1d9c9b1fbfb89c56dc867d9f0b87250dbabfcde23ec02dabbb5a32a + bb.js: ./dest/node/main.js languageName: node - linkType: hard + linkType: soft "@babel/code-frame@npm:^7.0.0, @babel/code-frame@npm:^7.10.4, @babel/code-frame@npm:^7.12.11, @babel/code-frame@npm:^7.16.0, @babel/code-frame@npm:^7.22.13, @babel/code-frame@npm:^7.23.5, @babel/code-frame@npm:^7.8.3": version: 7.23.5 @@ -4396,7 +4395,7 @@ __metadata: version: 0.0.0-use.local resolution: "@noir-lang/backend_barretenberg@workspace:tooling/noir_js_backend_barretenberg" dependencies: - "@aztec/bb.js": 0.35.1 + "@aztec/bb.js": "portal:../../../../barretenberg/ts" "@noir-lang/types": "workspace:*" "@types/node": ^20.6.2 "@types/prettier": ^3