From 42ac852db0cdd963a2bb23359fffdced6d7b957d Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 7 Aug 2024 15:36:10 +0000 Subject: [PATCH 01/13] initial work to see reduction in debug metadata and thus artifact size --- acvm-repo/acvm/src/pwg/mod.rs | 5 +++- compiler/noirc_errors/src/debug_info.rs | 5 +++- compiler/noirc_evaluator/src/ssa.rs | 17 +++++++++-- .../ssa/acir_gen/acir_ir/generated_acir.rs | 29 +++++++++++++++++-- .../brillig_debug_metadata/Nargo.toml | 7 +++++ .../brillig_debug_metadata/src/main.nr | 20 +++++++++++++ tooling/nargo/src/errors.rs | 12 ++++++-- 7 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 test_programs/execution_success/brillig_debug_metadata/Nargo.toml create mode 100644 test_programs/execution_success/brillig_debug_metadata/src/main.nr diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 4292d72fad5..0c0f427108d 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -481,7 +481,10 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { )?, }; - let result = solver.solve().map_err(|err| self.map_brillig_error(err))?; + let result = solver.solve().map_err(|err| { + dbg!(id); + self.map_brillig_error(err) + })?; match result { BrilligSolverStatus::ForeignCallWait(foreign_call) => { diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index 54e2521e413..17fef30a8d5 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -97,6 +97,8 @@ pub struct DebugInfo { /// that they should be serialized to/from strings. #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, + #[serde_as(as = "BTreeMap<_, BTreeMap>")] + pub brillig_locations: BTreeMap>>, pub variables: DebugVariables, pub functions: DebugFunctions, pub types: DebugTypes, @@ -113,11 +115,12 @@ pub struct OpCodesCount { impl DebugInfo { pub fn new( locations: BTreeMap>, + brillig_locations: BTreeMap>>, variables: DebugVariables, functions: DebugFunctions, types: DebugTypes, ) -> Self { - Self { locations, variables, functions, types } + Self { locations, brillig_locations, variables, functions, types } } /// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified. diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 032e3d07d7b..66fc280750b 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -262,13 +262,14 @@ fn convert_generated_acir_into_circuit( let GeneratedAcir { return_witnesses, locations, + brillig_locations, input_witnesses, assertion_payloads: assert_messages, warnings, name, .. } = generated_acir; - + dbg!(locations.len()); let (public_parameter_witnesses, private_parameters) = split_public_and_private_inputs(&func_sig, &input_witnesses); @@ -292,7 +293,19 @@ fn convert_generated_acir_into_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let mut debug_info = DebugInfo::new(locations, debug_variables, debug_functions, debug_types); + let brillig_locations = brillig_locations + .into_iter() + .map(|(function_index, locations)| { + let locations = locations + .into_iter() + .map(|(index, locations)| (index, locations.into_iter().collect())) + .collect(); + (function_index, locations) + }) + .collect(); + + let mut debug_info = + DebugInfo::new(locations, brillig_locations, debug_variables, debug_functions, debug_types); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 1395d04f99e..cde0a3eb240 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -52,6 +52,8 @@ pub(crate) struct GeneratedAcir { /// Correspondence between an opcode index (in opcodes) and the source code call stack which generated it pub(crate) locations: BTreeMap, + pub(crate) brillig_locations: BTreeMap>, + /// Source code location of the current instruction being processed /// None if we do not know the location pub(crate) call_stack: CallStack, @@ -574,16 +576,39 @@ impl GeneratedAcir { self.brillig_stdlib_func_locations .insert(self.last_acir_opcode_location(), stdlib_func); } - + let seen_func_before = self.opcodes.iter().any(|opcode| { + if let AcirOpcode::BrilligCall { id, .. } = opcode { + // dbg!(id); + *id == brillig_function_index + } else { + false + } + }); + // dbg!(seen_func_before); for (brillig_index, call_stack) in generated_brillig.locations.iter() { - self.locations.insert( + // if self.brillig_locations.get(&brillig_function_index).is_some() { + // break; + // } + if seen_func_before { + // dbg!("got here"); + break; + } + self.brillig_locations.entry(brillig_function_index).or_default().insert( OpcodeLocation::Brillig { acir_index: self.opcodes.len() - 1, brillig_index: *brillig_index, }, call_stack.clone(), ); + // self.locations.insert( + // OpcodeLocation::Brillig { + // acir_index: self.opcodes.len() - 1, + // brillig_index: *brillig_index, + // }, + // call_stack.clone(), + // ); } + // dbg!(self.brillig_locations.len()); for (brillig_index, message) in generated_brillig.assert_messages.iter() { self.assertion_payloads.insert( OpcodeLocation::Brillig { diff --git a/test_programs/execution_success/brillig_debug_metadata/Nargo.toml b/test_programs/execution_success/brillig_debug_metadata/Nargo.toml new file mode 100644 index 00000000000..de207943530 --- /dev/null +++ b/test_programs/execution_success/brillig_debug_metadata/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "brillig_debug_metadata" +type = "bin" +authors = [""] +compiler_version = ">=0.32.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/brillig_debug_metadata/src/main.nr b/test_programs/execution_success/brillig_debug_metadata/src/main.nr new file mode 100644 index 00000000000..785662ac858 --- /dev/null +++ b/test_programs/execution_success/brillig_debug_metadata/src/main.nr @@ -0,0 +1,20 @@ + +global ITERATION_COUNT = 100000; + +unconstrained fn some_unconstrained_fn(x: Field, y: Field) -> Field { + let x_u128 = U128::from_integer(x); + let y_u128 = U128::from_integer(y); + (x_u128 / y_u128).to_integer() + // assert(x == y); + // x / y +} + +fn main(data: [Field; ITERATION_COUNT]) -> pub Field { + let mut acc = data[0]; + + for i in 1..ITERATION_COUNT { + acc = some_unconstrained_fn(acc, data[i]); + } + + acc +} diff --git a/tooling/nargo/src/errors.rs b/tooling/nargo/src/errors.rs index b2248605cb5..d887dc32289 100644 --- a/tooling/nargo/src/errors.rs +++ b/tooling/nargo/src/errors.rs @@ -128,7 +128,7 @@ fn extract_locations_from_error( }, _ => None, }?; - + // dbg!(opcode_locations.clone()); // Insert the top-level Acir location where the Brillig function failed for (i, resolved_location) in opcode_locations.iter().enumerate() { if let ResolvedOpcodeLocation { @@ -149,13 +149,21 @@ fn extract_locations_from_error( } } + // dbg!(debug.clone()); + Some( opcode_locations .iter() .flat_map(|resolved_location| { debug[resolved_location.acir_function_index] .opcode_location(&resolved_location.opcode_location) - .unwrap_or_default() + .unwrap_or_else(|| { + // dbg!(resolved_location.opcode_location.clone()); + // dbg!(debug[resolved_location.acir_function_index].brillig_locations.clone()); + // debug[resolved_location.acir_function_index].brillig_locations.get(&2).unwrap().get(&resolved_location.opcode_location).cloned().unwrap_or_default() + vec![] + }) + // .unwrap_or_default() }) .collect(), ) From 0bc1a56bda5bcfa03028a14f7a10aa5047ec3078 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 03:10:40 +0000 Subject: [PATCH 02/13] removed redundant brillig metadata and maintain call stack --- acvm-repo/acvm/src/pwg/brillig.rs | 13 ++++- acvm-repo/acvm/src/pwg/mod.rs | 10 ++-- .../ssa/acir_gen/acir_ir/generated_acir.rs | 56 ++++++++----------- .../brillig_debug_metadata/src/main.nr | 2 - tooling/debugger/src/source_code_printer.rs | 1 + tooling/nargo/src/errors.rs | 26 +++++++-- 6 files changed, 60 insertions(+), 48 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 635aa154c3e..aa4cff52cf4 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -29,6 +29,10 @@ pub enum BrilligSolverStatus { pub struct BrilligSolver<'b, F, B: BlackBoxFunctionSolver> { vm: VM<'b, F, B>, acir_index: usize, + /// This id references which Brillig function within the main ACIR program we are solving. + /// This is used for appropriately resolving errors as the ACIR program artifacts + /// set up their Brillig debug metadata by function id. + function_id: u32, } impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { @@ -61,10 +65,11 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { brillig_bytecode: &'b [BrilligOpcode], bb_solver: &'b B, acir_index: usize, + brillig_function_id: u32, ) -> Result> { let vm = Self::setup_brillig_vm(initial_witness, memory, inputs, brillig_bytecode, bb_solver)?; - Ok(Self { vm, acir_index }) + Ok(Self { vm, acir_index, function_id: brillig_function_id }) } fn setup_brillig_vm( @@ -182,7 +187,11 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { } }; - Err(OpcodeResolutionError::BrilligFunctionFailed { payload, call_stack }) + Err(OpcodeResolutionError::BrilligFunctionFailed { + function_id: self.function_id, + payload, + call_stack, + }) } VMStatus::ForeignCallWait { function, inputs } => { Ok(BrilligSolverStatus::ForeignCallWait(ForeignCallWaitInfo { function, inputs })) diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 0c0f427108d..86fd16da4f0 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -132,6 +132,7 @@ pub enum OpcodeResolutionError { BlackBoxFunctionFailed(BlackBoxFunc, String), #[error("Failed to solve brillig function")] BrilligFunctionFailed { + function_id: u32, call_stack: Vec, payload: Option>, }, @@ -478,13 +479,11 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { &self.unconstrained_functions[*id as usize].bytecode, self.backend, self.instruction_pointer, + *id, )?, }; - let result = solver.solve().map_err(|err| { - dbg!(id); - self.map_brillig_error(err) - })?; + let result = solver.solve().map_err(|err| self.map_brillig_error(err))?; match result { BrilligSolverStatus::ForeignCallWait(foreign_call) => { @@ -505,7 +504,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { fn map_brillig_error(&self, mut err: OpcodeResolutionError) -> OpcodeResolutionError { match &mut err { - OpcodeResolutionError::BrilligFunctionFailed { call_stack, payload } => { + OpcodeResolutionError::BrilligFunctionFailed { call_stack, payload, .. } => { // Some brillig errors have static strings as payloads, we can resolve them here let last_location = call_stack.last().expect("Call stacks should have at least one item"); @@ -552,6 +551,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { &self.unconstrained_functions[*id as usize].bytecode, self.backend, self.instruction_pointer, + *id, ); match solver { Ok(solver) => StepResult::IntoBrillig(solver), diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index cde0a3eb240..65b38910772 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -49,10 +49,11 @@ pub(crate) struct GeneratedAcir { /// All witness indices which are inputs to the main function pub(crate) input_witnesses: Vec, - /// Correspondence between an opcode index (in opcodes) and the source code call stack which generated it - pub(crate) locations: BTreeMap, + pub(crate) locations: OpcodeToLocationsMap, - pub(crate) brillig_locations: BTreeMap>, + /// Brillig function id -> Opcodes locations map + /// This map is used to prevent redundant locations being stored for the same Brillig entry point. + pub(crate) brillig_locations: BTreeMap, /// Source code location of the current instruction being processed /// None if we do not know the location @@ -73,6 +74,9 @@ pub(crate) struct GeneratedAcir { pub(crate) brillig_stdlib_func_locations: BTreeMap, } +/// Correspondence between an opcode index (in opcodes) and the source code call stack which generated it +pub(crate) type OpcodeToLocationsMap = BTreeMap; + #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] pub(crate) enum BrilligStdlibFunc { Inverse, @@ -569,6 +573,10 @@ impl GeneratedAcir { brillig_function_index: u32, stdlib_func: Option, ) { + // Check whether we have a call to this Brillig function already exists. + // This helps us optimize the Brillig metadata to only be stored once per Brillig entry point. + let inserted_func_before = self.brillig_locations.get(&brillig_function_index).is_some(); + let opcode = AcirOpcode::BrilligCall { id: brillig_function_index, inputs, outputs, predicate }; self.push_opcode(opcode); @@ -576,46 +584,28 @@ impl GeneratedAcir { self.brillig_stdlib_func_locations .insert(self.last_acir_opcode_location(), stdlib_func); } - let seen_func_before = self.opcodes.iter().any(|opcode| { - if let AcirOpcode::BrilligCall { id, .. } = opcode { - // dbg!(id); - *id == brillig_function_index - } else { - false - } - }); - // dbg!(seen_func_before); - for (brillig_index, call_stack) in generated_brillig.locations.iter() { - // if self.brillig_locations.get(&brillig_function_index).is_some() { - // break; - // } - if seen_func_before { - // dbg!("got here"); - break; - } - self.brillig_locations.entry(brillig_function_index).or_default().insert( + + for (brillig_index, message) in generated_brillig.assert_messages.iter() { + self.assertion_payloads.insert( OpcodeLocation::Brillig { acir_index: self.opcodes.len() - 1, brillig_index: *brillig_index, }, - call_stack.clone(), + AssertionPayload::StaticString(message.clone()), ); - // self.locations.insert( - // OpcodeLocation::Brillig { - // acir_index: self.opcodes.len() - 1, - // brillig_index: *brillig_index, - // }, - // call_stack.clone(), - // ); } - // dbg!(self.brillig_locations.len()); - for (brillig_index, message) in generated_brillig.assert_messages.iter() { - self.assertion_payloads.insert( + + if inserted_func_before { + return; + } + + for (brillig_index, call_stack) in generated_brillig.locations.iter() { + self.brillig_locations.entry(brillig_function_index).or_default().insert( OpcodeLocation::Brillig { acir_index: self.opcodes.len() - 1, brillig_index: *brillig_index, }, - AssertionPayload::StaticString(message.clone()), + call_stack.clone(), ); } } diff --git a/test_programs/execution_success/brillig_debug_metadata/src/main.nr b/test_programs/execution_success/brillig_debug_metadata/src/main.nr index 785662ac858..a5ed21f35c8 100644 --- a/test_programs/execution_success/brillig_debug_metadata/src/main.nr +++ b/test_programs/execution_success/brillig_debug_metadata/src/main.nr @@ -5,8 +5,6 @@ unconstrained fn some_unconstrained_fn(x: Field, y: Field) -> Field { let x_u128 = U128::from_integer(x); let y_u128 = U128::from_integer(y); (x_u128 / y_u128).to_integer() - // assert(x == y); - // x / y } fn main(data: [Field; ITERATION_COUNT]) -> pub Field { diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index e9586b786bd..d1c82ad96ba 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -274,6 +274,7 @@ mod tests { BTreeMap::default(), BTreeMap::default(), BTreeMap::default(), + BTreeMap::default(), )]; let debug_artifact = DebugArtifact::new(debug_symbols, &fm); diff --git a/tooling/nargo/src/errors.rs b/tooling/nargo/src/errors.rs index d887dc32289..c1ff925d703 100644 --- a/tooling/nargo/src/errors.rs +++ b/tooling/nargo/src/errors.rs @@ -128,6 +128,7 @@ fn extract_locations_from_error( }, _ => None, }?; + // dbg!(opcode_locations.clone()); // Insert the top-level Acir location where the Brillig function failed for (i, resolved_location) in opcode_locations.iter().enumerate() { @@ -149,7 +150,13 @@ fn extract_locations_from_error( } } - // dbg!(debug.clone()); + let brillig_function_id = match error { + ExecutionError::SolvingError( + OpcodeResolutionError::BrilligFunctionFailed { function_id, .. }, + _, + ) => Some(*function_id), + _ => None, + }; Some( opcode_locations @@ -158,12 +165,19 @@ fn extract_locations_from_error( debug[resolved_location.acir_function_index] .opcode_location(&resolved_location.opcode_location) .unwrap_or_else(|| { - // dbg!(resolved_location.opcode_location.clone()); - // dbg!(debug[resolved_location.acir_function_index].brillig_locations.clone()); - // debug[resolved_location.acir_function_index].brillig_locations.get(&2).unwrap().get(&resolved_location.opcode_location).cloned().unwrap_or_default() - vec![] + if let Some(brillig_function_id) = brillig_function_id { + let brillig_locations = debug[resolved_location.acir_function_index] + .brillig_locations + .get(&brillig_function_id); + brillig_locations + .unwrap() + .get(&resolved_location.opcode_location) + .cloned() + .unwrap_or_default() + } else { + vec![] + } }) - // .unwrap_or_default() }) .collect(), ) From b333d26fcd323e5d0ecaf71be2208f6f1c116368 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 13:53:57 +0000 Subject: [PATCH 03/13] cleanup --- compiler/noirc_evaluator/src/ssa.rs | 2 +- tooling/nargo/src/errors.rs | 1 - tooling/profiler/src/flamegraph.rs | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 66fc280750b..b632958f37f 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -269,7 +269,7 @@ fn convert_generated_acir_into_circuit( name, .. } = generated_acir; - dbg!(locations.len()); + let (public_parameter_witnesses, private_parameters) = split_public_and_private_inputs(&func_sig, &input_witnesses); diff --git a/tooling/nargo/src/errors.rs b/tooling/nargo/src/errors.rs index c1ff925d703..86bb767b9fc 100644 --- a/tooling/nargo/src/errors.rs +++ b/tooling/nargo/src/errors.rs @@ -129,7 +129,6 @@ fn extract_locations_from_error( _ => None, }?; - // dbg!(opcode_locations.clone()); // Insert the top-level Acir location where the Brillig function failed for (i, resolved_location) in opcode_locations.iter().enumerate() { if let ResolvedOpcodeLocation { diff --git a/tooling/profiler/src/flamegraph.rs b/tooling/profiler/src/flamegraph.rs index 6b5a06405b3..0fdc65d8920 100644 --- a/tooling/profiler/src/flamegraph.rs +++ b/tooling/profiler/src/flamegraph.rs @@ -269,6 +269,7 @@ mod tests { BTreeMap::default(), BTreeMap::default(), BTreeMap::default(), + BTreeMap::default(), ); let samples_per_opcode = vec![10, 20, 30]; From 59ddefcee9f05db15e64eb30a0fc6732ece4bb00 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 14:25:41 +0000 Subject: [PATCH 04/13] remove old test and don't insert PLACEHOLDER_BRILLIG_INDEX --- .../src/ssa/acir_gen/acir_ir/generated_acir.rs | 4 ++++ .../brillig_debug_metadata/Nargo.toml | 7 ------- .../brillig_debug_metadata/src/main.nr | 18 ------------------ 3 files changed, 4 insertions(+), 25 deletions(-) delete mode 100644 test_programs/execution_success/brillig_debug_metadata/Nargo.toml delete mode 100644 test_programs/execution_success/brillig_debug_metadata/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 65b38910772..b7472844796 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -583,6 +583,10 @@ impl GeneratedAcir { if let Some(stdlib_func) = stdlib_func { self.brillig_stdlib_func_locations .insert(self.last_acir_opcode_location(), stdlib_func); + // The Brillig stdlib functions are handwritten and do not have any locations or assert messages. + // To avoid inserting the `PLACEHOLDER_BRILLIG_INDEX` into `self.brillig_locations` before the first + // user-specified Brillig call we can simply return after the Brillig stdlib function call. + return; } for (brillig_index, message) in generated_brillig.assert_messages.iter() { diff --git a/test_programs/execution_success/brillig_debug_metadata/Nargo.toml b/test_programs/execution_success/brillig_debug_metadata/Nargo.toml deleted file mode 100644 index de207943530..00000000000 --- a/test_programs/execution_success/brillig_debug_metadata/Nargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "brillig_debug_metadata" -type = "bin" -authors = [""] -compiler_version = ">=0.32.0" - -[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/brillig_debug_metadata/src/main.nr b/test_programs/execution_success/brillig_debug_metadata/src/main.nr deleted file mode 100644 index a5ed21f35c8..00000000000 --- a/test_programs/execution_success/brillig_debug_metadata/src/main.nr +++ /dev/null @@ -1,18 +0,0 @@ - -global ITERATION_COUNT = 100000; - -unconstrained fn some_unconstrained_fn(x: Field, y: Field) -> Field { - let x_u128 = U128::from_integer(x); - let y_u128 = U128::from_integer(y); - (x_u128 / y_u128).to_integer() -} - -fn main(data: [Field; ITERATION_COUNT]) -> pub Field { - let mut acc = data[0]; - - for i in 1..ITERATION_COUNT { - acc = some_unconstrained_fn(acc, data[i]); - } - - acc -} From a2ff1a05cc3614925d18c65c849abfc4ff6468a2 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 15:12:11 +0000 Subject: [PATCH 05/13] add tests --- acvm-repo/acvm/tests/solver.rs | 1 + .../ssa/acir_gen/acir_ir/generated_acir.rs | 1 + .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 33 ++++++++++++++----- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 279b0444609..5aa78e32208 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -675,6 +675,7 @@ fn unsatisfied_opcode_resolved_brillig() { assert_eq!( solver_status, ACVMStatus::Failure(OpcodeResolutionError::BrilligFunctionFailed { + function_id: 0, payload: None, call_stack: vec![OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 }] }), diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index b7472844796..6160cd1e49d 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -580,6 +580,7 @@ impl GeneratedAcir { 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); diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 0e4bbbf759c..bf9e221ef40 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -2837,8 +2837,6 @@ fn can_omit_element_sizes_array(array_typ: &Type) -> bool { #[cfg(test)] mod test { - use std::collections::BTreeMap; - use acvm::{ acir::{ circuit::{ExpressionWidth, Opcode, OpcodeLocation}, @@ -2846,7 +2844,10 @@ mod test { }, FieldElement, }; + use im::vector; + use noirc_errors::Location; use noirc_frontend::monomorphization::ast::InlineType; + use std::collections::BTreeMap; use crate::{ brillig::Brillig, @@ -2874,6 +2875,8 @@ mod test { } else { builder.new_brillig_function("foo".into(), foo_id); } + builder.set_call_stack(vector![Location::dummy(), Location::dummy()]); + let foo_v0 = builder.add_parameter(Type::field()); let foo_v1 = builder.add_parameter(Type::field()); @@ -3257,6 +3260,12 @@ mod test { _ => panic!("Expected only Brillig call opcode"), } } + + // We have two normal Brillig functions that was called multiple times. + // We should have a single locations map for each function's debug metadata. + assert_eq!(main_acir.brillig_locations.len(), 2); + assert!(main_acir.brillig_locations.get(&0).is_some()); + assert!(main_acir.brillig_locations.get(&1).is_some()); } // Test that given multiple primitive operations that are represented by Brillig directives (e.g. invert/quotient), @@ -3312,6 +3321,8 @@ mod test { 4, 0, ); + + assert_eq!(main_acir.brillig_locations.len(), 0); } // Test that given both hardcoded Brillig directives and calls to normal Brillig functions, @@ -3382,13 +3393,12 @@ mod test { 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, - ); + check_brillig_calls(&main_acir.brillig_stdlib_func_locations, main_opcodes, 1, 4, 2); + + // We have one normal Brillig functions that was called twice. + // We should have a single locations map for each function's debug metadata. + assert_eq!(main_acir.brillig_locations.len(), 1); + assert!(main_acir.brillig_locations.get(&0).is_some()); } // Test that given both normal Brillig calls, Brillig stdlib calls, and non-inlined ACIR calls, that we accurately generate ACIR. @@ -3479,9 +3489,14 @@ mod test { 2, ); + assert_eq!(main_acir.brillig_locations.len(), 1); + assert!(main_acir.brillig_locations.get(&0).is_some()); + 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); + + assert_eq!(foo_acir.brillig_locations.len(), 0); } fn check_brillig_calls( From 4041f0deddd2b7279fc3f8ebfa239985c5bb32b6 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 15:14:02 +0000 Subject: [PATCH 06/13] add comment --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index bf9e221ef40..e44042dcf68 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -2875,6 +2875,7 @@ mod test { } else { builder.new_brillig_function("foo".into(), foo_id); } + // Set a call stack for testing whether `brillig_locations` in the `GeneratedAcir` was accurately set. builder.set_call_stack(vector![Location::dummy(), Location::dummy()]); let foo_v0 = builder.add_parameter(Type::field()); From 0c0f4889d4c39850c8489cdb45c47d24fe7c141c Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 15:14:40 +0000 Subject: [PATCH 07/13] missing test debuginfo structure --- tooling/noirc_artifacts/src/debug.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/noirc_artifacts/src/debug.rs b/tooling/noirc_artifacts/src/debug.rs index 11a3e1c4dd7..8ae4156a5f6 100644 --- a/tooling/noirc_artifacts/src/debug.rs +++ b/tooling/noirc_artifacts/src/debug.rs @@ -248,6 +248,7 @@ mod tests { BTreeMap::default(), BTreeMap::default(), BTreeMap::default(), + BTreeMap::default(), )]; let debug_artifact = DebugArtifact::new(debug_symbols, &fm); From cb5d1f83e5ae71480376d994846863bbfb116dce Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 13:10:28 -0400 Subject: [PATCH 08/13] Update acvm-repo/acvm/src/pwg/brillig.rs Co-authored-by: jfecher --- acvm-repo/acvm/src/pwg/brillig.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index aa4cff52cf4..51fbdb5cd33 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -32,7 +32,7 @@ pub struct BrilligSolver<'b, F, B: BlackBoxFunctionSolver> { /// This id references which Brillig function within the main ACIR program we are solving. /// This is used for appropriately resolving errors as the ACIR program artifacts /// set up their Brillig debug metadata by function id. - function_id: u32, + function_id: BrilligFunctionId, } impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { From 33e2a779328524a2fb3521d53d3fcf6268ab6fd9 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 17:16:14 +0000 Subject: [PATCH 09/13] u32 -> BrilligFunctionIds --- acvm-repo/acir/src/circuit/brillig.rs | 3 +++ acvm-repo/acir/src/circuit/opcodes.rs | 4 ++-- acvm-repo/acvm/src/pwg/brillig.rs | 2 +- .../ssa/acir_gen/acir_ir/generated_acir.rs | 8 +++---- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 23 +++++++++++-------- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/acvm-repo/acir/src/circuit/brillig.rs b/acvm-repo/acir/src/circuit/brillig.rs index ee25d05afb0..736c23d92e3 100644 --- a/acvm-repo/acir/src/circuit/brillig.rs +++ b/acvm-repo/acir/src/circuit/brillig.rs @@ -27,3 +27,6 @@ pub enum BrilligOutputs { pub struct BrilligBytecode { pub bytecode: Vec>, } + +/// Id for the function being called. +pub type BrilligFunctionId = u32; diff --git a/acvm-repo/acir/src/circuit/opcodes.rs b/acvm-repo/acir/src/circuit/opcodes.rs index d303f9fbbab..17b5388faa0 100644 --- a/acvm-repo/acir/src/circuit/opcodes.rs +++ b/acvm-repo/acir/src/circuit/opcodes.rs @@ -1,5 +1,5 @@ use super::{ - brillig::{BrilligInputs, BrilligOutputs}, + brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}, directives::Directive, }; use crate::native_types::{Expression, Witness}; @@ -111,7 +111,7 @@ pub enum Opcode { BrilligCall { /// Id for the function being called. It is the responsibility of the executor /// to fetch the appropriate Brillig bytecode from this id. - id: u32, + id: BrilligFunctionId, /// Inputs to the function call inputs: Vec>, /// Outputs to the function call diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 51fbdb5cd33..06b62dce14e 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use acir::{ brillig::{ForeignCallParam, ForeignCallResult, Opcode as BrilligOpcode}, circuit::{ - brillig::{BrilligInputs, BrilligOutputs}, + brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}, opcodes::BlockId, ErrorSelector, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 6160cd1e49d..94c544b09d6 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -9,7 +9,7 @@ use crate::{ }; use acvm::acir::{ circuit::{ - brillig::{BrilligInputs, BrilligOutputs}, + brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}, opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode}, AssertionPayload, OpcodeLocation, }, @@ -27,7 +27,7 @@ use num_bigint::BigUint; /// 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; +pub(crate) const PLACEHOLDER_BRILLIG_INDEX: BrilligFunctionId = 0; #[derive(Debug, Default)] /// The output of the Acir-gen pass, which should only be produced for entry point Acir functions @@ -53,7 +53,7 @@ pub(crate) struct GeneratedAcir { /// Brillig function id -> Opcodes locations map /// This map is used to prevent redundant locations being stored for the same Brillig entry point. - pub(crate) brillig_locations: BTreeMap, + pub(crate) brillig_locations: BTreeMap, /// Source code location of the current instruction being processed /// None if we do not know the location @@ -570,7 +570,7 @@ impl GeneratedAcir { generated_brillig: &GeneratedBrillig, inputs: Vec>, outputs: Vec, - brillig_function_index: u32, + brillig_function_index: BrilligFunctionId, stdlib_func: Option, ) { // Check whether we have a call to this Brillig function already exists. diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index e44042dcf68..dbea90c52a9 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -32,7 +32,7 @@ pub(crate) use acir_ir::generated_acir::GeneratedAcir; use acvm::acir::circuit::opcodes::BlockType; use noirc_frontend::monomorphization::ast::InlineType; -use acvm::acir::circuit::brillig::BrilligBytecode; +use acvm::acir::circuit::brillig::{BrilligBytecode, BrilligFunctionId}; use acvm::acir::circuit::{AssertionPayload, ErrorSelector, ExpressionWidth, OpcodeLocation}; use acvm::acir::native_types::Witness; use acvm::acir::BlackBoxFunc; @@ -54,15 +54,16 @@ struct SharedContext { /// This mapping is necessary to use the correct function pointer for a Brillig call. /// 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>, + brillig_generated_func_pointers: + BTreeMap<(FunctionId, Vec), BrilligFunctionId>, /// 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, + 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>, + brillig_stdlib_calls_to_resolve: HashMap>, } impl SharedContext { @@ -70,7 +71,7 @@ impl SharedContext { &self, func_id: FunctionId, arguments: Vec, - ) -> Option<&u32> { + ) -> Option<&BrilligFunctionId> { self.brillig_generated_func_pointers.get(&(func_id, arguments)) } @@ -82,14 +83,14 @@ impl SharedContext { &mut self, func_id: FunctionId, arguments: Vec, - generated_pointer: u32, + generated_pointer: BrilligFunctionId, code: GeneratedBrillig, ) { self.brillig_generated_func_pointers.insert((func_id, arguments), generated_pointer); self.generated_brillig.push(code); } - fn new_generated_pointer(&self) -> u32 { + fn new_generated_pointer(&self) -> BrilligFunctionId { self.generated_brillig.len() as u32 } @@ -120,7 +121,7 @@ impl SharedContext { fn insert_generated_brillig_stdlib( &mut self, brillig_stdlib_func: BrilligStdlibFunc, - generated_pointer: u32, + generated_pointer: BrilligFunctionId, func_id: FunctionId, opcode_location: OpcodeLocation, code: GeneratedBrillig, @@ -130,7 +131,11 @@ impl SharedContext { self.generated_brillig.push(code); } - fn add_call_to_resolve(&mut self, func_id: FunctionId, call_to_resolve: (OpcodeLocation, u32)) { + fn add_call_to_resolve( + &mut self, + func_id: FunctionId, + call_to_resolve: (OpcodeLocation, BrilligFunctionId), + ) { self.brillig_stdlib_calls_to_resolve.entry(func_id).or_default().push(call_to_resolve); } } From b12ee8fe977f1ad9f879c4f43f8bd0a3dec48b0a Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 17:19:32 +0000 Subject: [PATCH 10/13] a couple more u32 -> BrilligFunctionid --- acvm-repo/acvm/src/pwg/brillig.rs | 2 +- acvm-repo/acvm/src/pwg/mod.rs | 2 +- compiler/noirc_errors/src/debug_info.rs | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 06b62dce14e..c12629b0543 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -65,7 +65,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { brillig_bytecode: &'b [BrilligOpcode], bb_solver: &'b B, acir_index: usize, - brillig_function_id: u32, + brillig_function_id: BrilligFunctionId, ) -> Result> { let vm = Self::setup_brillig_vm(initial_witness, memory, inputs, brillig_bytecode, bb_solver)?; diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 86fd16da4f0..5052c85977e 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -132,7 +132,7 @@ pub enum OpcodeResolutionError { BlackBoxFunctionFailed(BlackBoxFunc, String), #[error("Failed to solve brillig function")] BrilligFunctionFailed { - function_id: u32, + function_id: BrilligFunctionId, call_stack: Vec, payload: Option>, }, diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index 17fef30a8d5..1a254175c0a 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -1,3 +1,4 @@ +use acvm::acir::circuit::brillig::BrilligFunctionId; use acvm::acir::circuit::OpcodeLocation; use acvm::compiler::AcirTransformationMap; @@ -98,7 +99,7 @@ pub struct DebugInfo { #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, #[serde_as(as = "BTreeMap<_, BTreeMap>")] - pub brillig_locations: BTreeMap>>, + pub brillig_locations: BTreeMap>>, pub variables: DebugVariables, pub functions: DebugFunctions, pub types: DebugTypes, @@ -115,7 +116,7 @@ pub struct OpCodesCount { impl DebugInfo { pub fn new( locations: BTreeMap>, - brillig_locations: BTreeMap>>, + brillig_locations: BTreeMap>>, variables: DebugVariables, functions: DebugFunctions, types: DebugTypes, From 89e8fb0eec69fcc9f0dd52ea72178fb7b074fb3a Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 17:20:00 +0000 Subject: [PATCH 11/13] one more --- acvm-repo/acvm/src/pwg/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 5052c85977e..d94c00f0e5f 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -5,7 +5,7 @@ use std::collections::HashMap; use acir::{ brillig::ForeignCallResult, circuit::{ - brillig::BrilligBytecode, + brillig::{BrilligBytecode, BrilligFunctionId}, opcodes::{BlockId, ConstantOrWitnessEnum, FunctionInput}, AssertionPayload, ErrorSelector, ExpressionOrMemory, Opcode, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, From 22e116a35177590ffba8fa591ba0750fd7233872 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 19:20:28 +0000 Subject: [PATCH 12/13] switch BrilligFunctionid from type alias to struct type --- acvm-repo/acir/src/circuit/brillig.rs | 18 +++++++++++- .../acir/tests/test_program_serialization.rs | 6 ++-- acvm-repo/acvm/src/pwg/mod.rs | 4 +-- acvm-repo/acvm/tests/solver.rs | 14 +++++----- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 4 +-- .../ssa/acir_gen/acir_ir/generated_acir.rs | 4 +-- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 27 +++++++++++------- tooling/debugger/src/context.rs | 28 +++++++++++++------ tooling/debugger/src/repl.rs | 6 ++-- 9 files changed, 72 insertions(+), 39 deletions(-) diff --git a/acvm-repo/acir/src/circuit/brillig.rs b/acvm-repo/acir/src/circuit/brillig.rs index 736c23d92e3..a9714ce29b2 100644 --- a/acvm-repo/acir/src/circuit/brillig.rs +++ b/acvm-repo/acir/src/circuit/brillig.rs @@ -29,4 +29,20 @@ pub struct BrilligBytecode { } /// Id for the function being called. -pub type BrilligFunctionId = u32; +#[derive( + Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Hash, Copy, Default, PartialOrd, Ord, +)] +#[serde(transparent)] +pub struct BrilligFunctionId(pub u32); + +impl BrilligFunctionId { + pub fn as_usize(&self) -> usize { + self.0 as usize + } +} + +impl std::fmt::Display for BrilligFunctionId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} diff --git a/acvm-repo/acir/tests/test_program_serialization.rs b/acvm-repo/acir/tests/test_program_serialization.rs index 3610ce6493e..1a634eeea9c 100644 --- a/acvm-repo/acir/tests/test_program_serialization.rs +++ b/acvm-repo/acir/tests/test_program_serialization.rs @@ -13,7 +13,7 @@ use std::collections::BTreeSet; use acir::{ circuit::{ - brillig::{BrilligBytecode, BrilligInputs, BrilligOutputs}, + brillig::{BrilligBytecode, BrilligFunctionId, BrilligInputs, BrilligOutputs}, opcodes::{BlackBoxFuncCall, BlockId, FunctionInput, MemOp}, Circuit, Opcode, Program, PublicInputs, }, @@ -181,7 +181,7 @@ fn simple_brillig_foreign_call() { }; let opcodes = vec![Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![ BrilligInputs::Single(w_input.into()), // Input Register 0, ], @@ -272,7 +272,7 @@ fn complex_brillig_foreign_call() { }; let opcodes = vec![Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![ // Input 0,1,2 BrilligInputs::Array(vec![ diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index d94c00f0e5f..83c5aeb6296 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -476,7 +476,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { &self.witness_map, &self.block_solvers, inputs, - &self.unconstrained_functions[*id as usize].bytecode, + &self.unconstrained_functions[id.as_usize()].bytecode, self.backend, self.instruction_pointer, *id, @@ -548,7 +548,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { witness, &self.block_solvers, inputs, - &self.unconstrained_functions[*id as usize].bytecode, + &self.unconstrained_functions[id.as_usize()].bytecode, self.backend, self.instruction_pointer, *id, diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 5aa78e32208..a1b8b62f8bf 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -4,7 +4,7 @@ use acir::{ acir_field::GenericFieldElement, brillig::{BinaryFieldOp, HeapArray, MemoryAddress, Opcode as BrilligOpcode, ValueOrArray}, circuit::{ - brillig::{BrilligBytecode, BrilligInputs, BrilligOutputs}, + brillig::{BrilligBytecode, BrilligFunctionId, BrilligInputs, BrilligOutputs}, opcodes::{BlackBoxFuncCall, BlockId, BlockType, FunctionInput, MemOp}, Opcode, OpcodeLocation, }, @@ -82,7 +82,7 @@ fn inversion_brillig_oracle_equivalence() { let opcodes = vec![ Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![ BrilligInputs::Single(Expression { // Input Register 0 @@ -211,7 +211,7 @@ fn double_inversion_brillig_oracle() { let opcodes = vec![ Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![ BrilligInputs::Single(Expression { // Input Register 0 @@ -406,7 +406,7 @@ fn oracle_dependent_execution() { let opcodes = vec![ Opcode::AssertZero(equality_check), Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![ BrilligInputs::Single(w_x.into()), // Input Register 0 BrilligInputs::Single(Expression::default()), // Input Register 1 @@ -514,7 +514,7 @@ fn brillig_oracle_predicate() { }; let opcodes = vec![Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![ BrilligInputs::Single(Expression { mul_terms: vec![], @@ -650,7 +650,7 @@ fn unsatisfied_opcode_resolved_brillig() { let opcodes = vec![ Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![ BrilligInputs::Single(Expression { mul_terms: vec![], @@ -675,7 +675,7 @@ fn unsatisfied_opcode_resolved_brillig() { assert_eq!( solver_status, ACVMStatus::Failure(OpcodeResolutionError::BrilligFunctionFailed { - function_id: 0, + function_id: BrilligFunctionId(0), payload: None, call_stack: vec![OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 }] }), diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index fdad06a520b..b2a73106468 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -7,7 +7,7 @@ use crate::ssa::acir_gen::{AcirDynamicArray, AcirValue}; use crate::ssa::ir::dfg::CallStack; use crate::ssa::ir::types::Type as SsaType; use crate::ssa::ir::{instruction::Endian, types::NumericType}; -use acvm::acir::circuit::brillig::{BrilligInputs, BrilligOutputs}; +use acvm::acir::circuit::brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}; use acvm::acir::circuit::opcodes::{BlockId, BlockType, MemOp}; use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode}; use acvm::blackbox_solver; @@ -1612,7 +1612,7 @@ impl AcirContext { outputs: Vec, attempt_execution: bool, unsafe_return_values: bool, - brillig_function_index: u32, + brillig_function_index: BrilligFunctionId, brillig_stdlib_func: Option, ) -> Result, RuntimeError> { let predicate = self.var_to_expression(predicate)?; diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 94c544b09d6..661371c5de6 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -27,7 +27,7 @@ use num_bigint::BigUint; /// 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: BrilligFunctionId = 0; +pub(crate) const PLACEHOLDER_BRILLIG_INDEX: BrilligFunctionId = BrilligFunctionId(0); #[derive(Debug, Default)] /// The output of the Acir-gen pass, which should only be produced for entry point Acir functions @@ -619,7 +619,7 @@ impl GeneratedAcir { pub(crate) fn resolve_brillig_stdlib_call( &mut self, opcode_location: OpcodeLocation, - brillig_function_index: u32, + brillig_function_index: BrilligFunctionId, ) { let acir_index = match opcode_location { OpcodeLocation::Acir(index) => index, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 23117a6bb28..3e855ecbf9b 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -91,7 +91,7 @@ impl SharedContext { } fn new_generated_pointer(&self) -> BrilligFunctionId { - self.generated_brillig.len() as u32 + BrilligFunctionId(self.generated_brillig.len() as u32) } fn generate_brillig_calls_to_resolve( @@ -485,10 +485,15 @@ impl<'a> Context<'a> { false, true, // We are guaranteed to have a Brillig function pointer of `0` as main itself is marked as unconstrained - 0, + BrilligFunctionId(0), None, )?; - self.shared_context.insert_generated_brillig(main_func.id(), arguments, 0, code); + self.shared_context.insert_generated_brillig( + main_func.id(), + arguments, + BrilligFunctionId(0), + code, + ); let return_witnesses: Vec = output_values .iter() @@ -805,7 +810,7 @@ impl<'a> Context<'a> { { let code = self .shared_context - .generated_brillig(*generated_pointer as usize); + .generated_brillig(generated_pointer.as_usize()); self.acir_context.brillig_call( self.current_side_effects_enabled_var, code, @@ -2859,7 +2864,7 @@ fn can_omit_element_sizes_array(array_typ: &Type) -> bool { mod test { use acvm::{ acir::{ - circuit::{ExpressionWidth, Opcode, OpcodeLocation}, + circuit::{brillig::BrilligFunctionId, ExpressionWidth, Opcode, OpcodeLocation}, native_types::Witness, }, FieldElement, @@ -3276,6 +3281,7 @@ mod test { match opcode { Opcode::BrilligCall { id, .. } => { let expected_id = if i == 3 || i == 5 { 1 } else { 0 }; + let expected_id = BrilligFunctionId(expected_id); assert_eq!(*id, expected_id, "Expected an id of {expected_id} but got {id}"); } _ => panic!("Expected only Brillig call opcode"), @@ -3285,8 +3291,8 @@ mod test { // We have two normal Brillig functions that was called multiple times. // We should have a single locations map for each function's debug metadata. assert_eq!(main_acir.brillig_locations.len(), 2); - assert!(main_acir.brillig_locations.get(&0).is_some()); - assert!(main_acir.brillig_locations.get(&1).is_some()); + assert!(main_acir.brillig_locations.get(&BrilligFunctionId(0)).is_some()); + assert!(main_acir.brillig_locations.get(&BrilligFunctionId(1)).is_some()); } // Test that given multiple primitive operations that are represented by Brillig directives (e.g. invert/quotient), @@ -3419,7 +3425,7 @@ mod test { // We have one normal Brillig functions that was called twice. // We should have a single locations map for each function's debug metadata. assert_eq!(main_acir.brillig_locations.len(), 1); - assert!(main_acir.brillig_locations.get(&0).is_some()); + assert!(main_acir.brillig_locations.get(&BrilligFunctionId(0)).is_some()); } // Test that given both normal Brillig calls, Brillig stdlib calls, and non-inlined ACIR calls, that we accurately generate ACIR. @@ -3511,7 +3517,7 @@ mod test { ); assert_eq!(main_acir.brillig_locations.len(), 1); - assert!(main_acir.brillig_locations.get(&0).is_some()); + assert!(main_acir.brillig_locations.get(&BrilligFunctionId(0)).is_some()); let foo_acir = &acir_functions[1]; let foo_opcodes = foo_acir.opcodes(); @@ -3548,6 +3554,7 @@ mod test { // 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; + let expected_id = BrilligFunctionId(expected_id); assert_eq!(id, expected_id, "Expected {expected_id} but got {id}"); num_brillig_stdlib_calls += 1; } @@ -3573,7 +3580,7 @@ mod test { continue; } // We only generate one normal Brillig call so we should expect a function ID of `0` - let expected_id = 0u32; + let expected_id = BrilligFunctionId(0); assert_eq!(*id, expected_id, "Expected an id of {expected_id} but got {id}"); num_normal_brillig_calls += 1; } diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 7cdbe515649..69fcce99846 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -68,7 +68,7 @@ impl AddressMap { ) -> Self { let opcode_address_size = |opcode: &Opcode| { if let Opcode::BrilligCall { id, .. } = opcode { - unconstrained_functions[*id as usize].bytecode.len() + unconstrained_functions[id.as_usize()].bytecode.len() } else { 1 } @@ -431,7 +431,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { let opcode = &opcodes[*acir_index]; match opcode { Opcode::BrilligCall { id, .. } => { - let first_opcode = &self.unconstrained_functions[*id as usize].bytecode[0]; + let first_opcode = &self.unconstrained_functions[id.as_usize()].bytecode[0]; format!("BRILLIG {first_opcode:?}") } _ => format!("{opcode:?}"), @@ -439,7 +439,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } OpcodeLocation::Brillig { acir_index, brillig_index } => match &opcodes[*acir_index] { Opcode::BrilligCall { id, .. } => { - let bytecode = &self.unconstrained_functions[*id as usize].bytecode; + let bytecode = &self.unconstrained_functions[id.as_usize()].bytecode; let opcode = &bytecode[*brillig_index]; format!(" | {opcode:?}") } @@ -751,7 +751,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { if acir_index < opcodes.len() { match &opcodes[acir_index] { Opcode::BrilligCall { id, .. } => { - let bytecode = &self.unconstrained_functions[*id as usize].bytecode; + let bytecode = &self.unconstrained_functions[id.as_usize()].bytecode; brillig_index < bytecode.len() } _ => false, @@ -855,7 +855,7 @@ mod tests { acir::{ brillig::IntegerBitSize, circuit::{ - brillig::{BrilligInputs, BrilligOutputs}, + brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}, opcodes::{BlockId, BlockType}, }, native_types::Expression, @@ -896,7 +896,7 @@ mod tests { ], }; let opcodes = vec![Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![BrilligInputs::Single(Expression { linear_combinations: vec![(fe_1, w_x)], ..Expression::default() @@ -1009,7 +1009,7 @@ mod tests { let opcodes = vec![ // z = x + y Opcode::BrilligCall { - id: 0, + id: BrilligFunctionId(0), inputs: vec![ BrilligInputs::Single(Expression { linear_combinations: vec![(fe_1, w_x)], @@ -1101,7 +1101,12 @@ mod tests { init: vec![], block_type: BlockType::Memory, }, - Opcode::BrilligCall { id: 0, inputs: vec![], outputs: vec![], predicate: None }, + Opcode::BrilligCall { + id: BrilligFunctionId(0), + inputs: vec![], + outputs: vec![], + predicate: None, + }, Opcode::Call { id: 1, inputs: vec![], outputs: vec![], predicate: None }, Opcode::AssertZero(Expression::default()), ], @@ -1109,7 +1114,12 @@ mod tests { }; let circuit_two = Circuit { opcodes: vec![ - Opcode::BrilligCall { id: 1, inputs: vec![], outputs: vec![], predicate: None }, + Opcode::BrilligCall { + id: BrilligFunctionId(0), + inputs: vec![], + outputs: vec![], + predicate: None, + }, Opcode::AssertZero(Expression::default()), ], ..Circuit::default() diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index bd9b316331d..b4b2bff53be 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -83,7 +83,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { OpcodeLocation::Brillig { acir_index, brillig_index } => { let brillig_bytecode = if let Opcode::BrilligCall { id, .. } = opcodes[*acir_index] { - &self.unconstrained_functions[id as usize].bytecode + &self.unconstrained_functions[id.as_usize()].bytecode } else { unreachable!("Brillig location does not contain Brillig opcodes"); }; @@ -111,7 +111,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { OpcodeLocation::Brillig { acir_index, brillig_index } => { let brillig_bytecode = if let Opcode::BrilligCall { id, .. } = opcodes[*acir_index] { - &self.unconstrained_functions[id as usize].bytecode + &self.unconstrained_functions[id.as_usize()].bytecode } else { unreachable!("Brillig location does not contain Brillig opcodes"); }; @@ -207,7 +207,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { circuit_id, acir_index, marker, id, inputs ); println!(" | outputs={:?}", outputs); - let bytecode = &self.unconstrained_functions[*id as usize].bytecode; + let bytecode = &self.unconstrained_functions[id.as_usize()].bytecode; print_brillig_bytecode(acir_index, bytecode); } _ => println!("{:>2}:{:>3} {:2} {:?}", circuit_id, acir_index, marker, opcode), From db2647f679873023be3fa4cea3d61567c83cb292 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 8 Aug 2024 19:40:20 +0000 Subject: [PATCH 13/13] fix accident break of test --- tooling/debugger/src/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 69fcce99846..2a550b84020 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -1115,7 +1115,7 @@ mod tests { let circuit_two = Circuit { opcodes: vec![ Opcode::BrilligCall { - id: BrilligFunctionId(0), + id: BrilligFunctionId(1), inputs: vec![], outputs: vec![], predicate: None,