From 9c690ef14819fc2122a4ed8b8dbd15d8bd5ca372 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 4 Jul 2023 08:41:36 +0000 Subject: [PATCH 1/5] feat: recursion working in brillig --- .../brillig_recursion/Nargo.toml | 5 ++++ .../brillig_recursion/Prover.toml | 1 + .../brillig_recursion/src/main.nr | 14 +++++++++++ .../src/brillig/brillig_ir/artifact.rs | 24 ++++++++----------- .../src/ssa_refactor/acir_gen/mod.rs | 10 ++++++-- 5 files changed, 38 insertions(+), 16 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/Prover.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/src/main.nr diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/Prover.toml new file mode 100644 index 00000000000..3a627b9188b --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/Prover.toml @@ -0,0 +1 @@ +x = "10" diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/src/main.nr new file mode 100644 index 00000000000..974d26165b6 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_recursion/src/main.nr @@ -0,0 +1,14 @@ +// Tests a very simple program. +// +// The feature being tested is brillig recursion +fn main(x: u32) { + assert(fibonacci(x) == 55); +} + +unconstrained fn fibonacci(x : u32) -> u32 { + if x <= 1 { + x + } else { + fibonacci(x - 1) + fibonacci(x - 2) + } +} diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 6ccb7cc0b16..b794898a608 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -74,13 +74,17 @@ impl BrilligArtifact { } /// Creates an entry point artifact wrapping the bytecode of the function provided. - pub(crate) fn to_entry_point_artifact(artifact: &BrilligArtifact) -> BrilligArtifact { + pub(crate) fn new_entry_point_artifact( + arguments: Vec, + return_parameters: Vec, + target_function: Label, + ) -> BrilligArtifact { let mut entry_point_artifact = - BrilligArtifact::new(artifact.arguments.clone(), artifact.return_parameters.clone()); + BrilligArtifact::new(arguments.clone(), return_parameters.clone()); entry_point_artifact.entry_point_instruction(); - entry_point_artifact.add_unresolved_jumps_and_calls(artifact); - entry_point_artifact.byte_code.extend_from_slice(&artifact.byte_code); + entry_point_artifact + .add_unresolved_external_call(BrilligOpcode::Call { location: 0 }, target_function); entry_point_artifact.exit_point_instruction(); entry_point_artifact @@ -126,16 +130,6 @@ impl BrilligArtifact { // We want all functions to follow the calling convention of returning // their results in the first `n` registers. So we modify the bytecode of the // function to move the return values to the first `n` registers once completed. - // - // Swap the stop opcode with a jump to the exit point section - - let stop_position = - self.byte_code.iter().position(|opcode| matches!(opcode, BrilligOpcode::Stop)); - - let stop_position = stop_position.expect("expected a stop opcode"); - - let exit_section = self.index_of_next_opcode(); - self.byte_code[stop_position] = BrilligOpcode::Jump { location: exit_section }; // TODO: this _seems_ like an abstraction leak, we need to know about the reserved // TODO: registers in order to do this. @@ -161,6 +155,8 @@ impl BrilligArtifact { /// Brillig artifact (self). pub(crate) fn link_with(&mut self, func_label: Label, obj: &BrilligArtifact) { // Add the unresolved jumps of the linked function to this artifact. + println!("Linking with {}", func_label); + println!("Current bytecode: {:?}", self.byte_code); self.add_unresolved_jumps_and_calls(obj); let mut byte_code = obj.byte_code.clone(); diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 27053716859..8c2d1143863 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -2,7 +2,9 @@ use std::collections::HashMap; -use crate::brillig::{brillig_ir::artifact::BrilligArtifact, Brillig}; +use crate::brillig::{ + brillig_gen::brillig_fn::FunctionContext, brillig_ir::artifact::BrilligArtifact, Brillig, +}; use self::acir_ir::{ acir_variable::{AcirContext, AcirType, AcirVar}, @@ -217,7 +219,11 @@ impl Context { let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg)); // Create the entry point artifact - let mut entry_point = BrilligArtifact::to_entry_point_artifact(&brillig[*id]); + let mut entry_point = BrilligArtifact::new_entry_point_artifact( + FunctionContext::parameters(func), + FunctionContext::return_values(func), + FunctionContext::function_id_to_function_label(*id), + ); // Link the entry point with all dependencies while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() { let artifact = &brillig.find_by_function_label(unresolved_fn_label.clone()).expect("Cannot find linked fn {unresolved_fn_label}"); From 517a73dc85772b830547eee962592a2ec1744a07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Tue, 4 Jul 2023 10:43:45 +0200 Subject: [PATCH 2/5] Update crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs --- crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index b794898a608..cadfae4779f 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -155,8 +155,6 @@ impl BrilligArtifact { /// Brillig artifact (self). pub(crate) fn link_with(&mut self, func_label: Label, obj: &BrilligArtifact) { // Add the unresolved jumps of the linked function to this artifact. - println!("Linking with {}", func_label); - println!("Current bytecode: {:?}", self.byte_code); self.add_unresolved_jumps_and_calls(obj); let mut byte_code = obj.byte_code.clone(); From 1c59db059cc8444ecdfb0ed9d4bb3244426739ce Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 4 Jul 2023 08:53:20 +0000 Subject: [PATCH 3/5] fix: removed redundant clone --- .../noirc_evaluator/src/brillig/brillig_ir/artifact.rs | 3 +-- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 9 +++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index cadfae4779f..7dd38c5fb2a 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -79,8 +79,7 @@ impl BrilligArtifact { return_parameters: Vec, target_function: Label, ) -> BrilligArtifact { - let mut entry_point_artifact = - BrilligArtifact::new(arguments.clone(), return_parameters.clone()); + let mut entry_point_artifact = BrilligArtifact::new(arguments, return_parameters); entry_point_artifact.entry_point_instruction(); entry_point_artifact diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 8c2d1143863..9641ee14167 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -3,7 +3,8 @@ use std::collections::HashMap; use crate::brillig::{ - brillig_gen::brillig_fn::FunctionContext, brillig_ir::artifact::BrilligArtifact, Brillig, + brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, + brillig_ir::artifact::BrilligArtifact, Brillig, }; use self::acir_ir::{ @@ -220,9 +221,9 @@ impl Context { // Create the entry point artifact let mut entry_point = BrilligArtifact::new_entry_point_artifact( - FunctionContext::parameters(func), - FunctionContext::return_values(func), - FunctionContext::function_id_to_function_label(*id), + BrilligFunctionContext::parameters(func), + BrilligFunctionContext::return_values(func), + BrilligFunctionContext::function_id_to_function_label(*id), ); // Link the entry point with all dependencies while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() { From 6f206bf49c9ec21ccbb40278d7185f667e5e9476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Tue, 4 Jul 2023 16:30:10 +0200 Subject: [PATCH 4/5] Update crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs --- crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 7dd38c5fb2a..2eaeee8f636 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -73,7 +73,7 @@ impl BrilligArtifact { } } - /// Creates an entry point artifact wrapping the bytecode of the function provided. + /// Creates an entry point artifact that will jump to the function label provided. pub(crate) fn new_entry_point_artifact( arguments: Vec, return_parameters: Vec, From ed011866bcf0ae7f8f69ab6ab9d5f702440b78fd Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 4 Jul 2023 17:02:42 +0000 Subject: [PATCH 5/5] fix: merge change --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index cd6172d59fb..307bcce5a35 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -344,7 +344,7 @@ impl Context { let mut entry_point = BrilligArtifact::new_entry_point_artifact( BrilligFunctionContext::parameters(func), BrilligFunctionContext::return_values(func), - BrilligFunctionContext::function_id_to_function_label(*id), + BrilligFunctionContext::function_id_to_function_label(func.id()), ); // Link the entry point with all dependencies while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() {