From 808b911fc4a7f7f3b21a2a032796d7b0b824b135 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 13 Jul 2023 07:40:20 +0000 Subject: [PATCH 1/5] feat: compile to brillig reachable acir fns --- .../brillig_acir_as_brillig/Nargo.toml | 5 ++ .../brillig_acir_as_brillig/Prover.toml | 1 + .../brillig_acir_as_brillig/src/main.nr | 43 ++++++++++++++++ .../src/brillig/brillig_gen.rs | 4 -- crates/noirc_evaluator/src/brillig/mod.rs | 50 ++++++++++++++++--- .../src/ssa_refactor/acir_gen/mod.rs | 2 +- 6 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/Prover.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/src/main.nr diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/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_acir_as_brillig/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/Prover.toml new file mode 100644 index 00000000000..11497a473bc --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/Prover.toml @@ -0,0 +1 @@ +x = "0" diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/src/main.nr new file mode 100644 index 00000000000..9d4c5da9dc4 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_acir_as_brillig/src/main.nr @@ -0,0 +1,43 @@ + +fn main(x: u32) { + assert(entry_point(x) == 2); + swap_entry_point(x, x + 1); + assert(deep_entry_point(x) == 4); +} + +fn inner(x : u32) -> u32 { + x + 1 +} + +unconstrained fn entry_point(x : u32) -> u32 { + inner(x + 1) +} + +fn swap(x: u32, y:u32) -> (u32, u32) { + (y, x) +} + +unconstrained fn swap_entry_point(x: u32, y: u32) { + let swapped = swap(x, y); + assert(swapped.0 == y); + assert(swapped.1 == x); + let swapped_twice = swap(swapped.0, swapped.1); + assert(swapped_twice.0 == x); + assert(swapped_twice.1 == y); +} + +fn level_3(x : u32) -> u32 { + x + 1 +} + +fn level_2(x : u32) -> u32 { + level_3(x + 1) +} + +fn level_1(x : u32) -> u32 { + level_2(x + 1) +} + +unconstrained fn deep_entry_point(x : u32) -> u32 { + level_1(x + 1) +} diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen.rs b/crates/noirc_evaluator/src/brillig/brillig_gen.rs index 2cc0c03544e..32c98cb86ff 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen.rs @@ -12,10 +12,6 @@ use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext}; use super::brillig_ir::{artifact::BrilligArtifact, BrilligContext}; /// Converting an SSA function into Brillig bytecode. -/// -/// TODO: Change this to use `dfg.basic_blocks_iter` which will return an -/// TODO iterator of all of the basic blocks. -/// TODO(Jake): what order is this ^ pub(crate) fn convert_ssa_function(func: &Function) -> BrilligArtifact { let mut reverse_post_order = Vec::new(); reverse_post_order.extend_from_slice(PostOrder::with_function(func).as_slice()); diff --git a/crates/noirc_evaluator/src/brillig/mod.rs b/crates/noirc_evaluator/src/brillig/mod.rs index 4ff657435db..a0c8a8275b3 100644 --- a/crates/noirc_evaluator/src/brillig/mod.rs +++ b/crates/noirc_evaluator/src/brillig/mod.rs @@ -1,12 +1,17 @@ pub(crate) mod brillig_gen; pub(crate) mod brillig_ir; +use im::HashSet; + use self::{ brillig_gen::{brillig_fn::FunctionContext, convert_ssa_function}, brillig_ir::artifact::{BrilligArtifact, Label}, }; use crate::ssa_refactor::{ - ir::function::{Function, FunctionId, RuntimeType}, + ir::{ + function::{Function, FunctionId, RuntimeType}, + value::Value, + }, ssa_gen::Ssa, }; use std::collections::HashMap; @@ -46,15 +51,46 @@ impl std::ops::Index for Brillig { } impl Ssa { - /// Generate compilation artifacts for brillig functions + /// Compile to brillig brillig functions and ACIR functions reachable from them pub(crate) fn to_brillig(&self) -> Brillig { - // Collect all of the brillig functions - let brillig_functions = - self.functions.values().filter(|func| func.runtime() == RuntimeType::Brillig); + // Collect all the function ids that are reachable from brillig + // That means all the functions marked as brillig and ACIR functions called by them + let mut reachable_function_ids: HashSet = HashSet::new(); + + // Initialize the queue with all the functions marked as brillig + let mut reachability_queue: Vec = self + .functions + .iter() + .filter_map( + |(id, func)| { + if func.runtime() == RuntimeType::Brillig { + Some(*id) + } else { + None + } + }, + ) + .collect(); + + while !reachability_queue.is_empty() { + let func = &self.functions[&reachability_queue.pop().unwrap()]; + reachable_function_ids.insert(func.id()); + for (_, value) in func.dfg.values_iter() { + // All reachable functions appear as literals after defunctionalization of the SSA + if let Value::Function(function_id) = value { + if !reachable_function_ids.contains(function_id) + && !reachability_queue.contains(function_id) + { + reachability_queue.push(*function_id); + } + } + } + } let mut brillig = Brillig::default(); - for brillig_function in brillig_functions { - brillig.compile(brillig_function); + for brillig_function_id in reachable_function_ids { + let func = &self.functions[&brillig_function_id]; + brillig.compile(func); } brillig 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 f743d975f91..3c365e68e71 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -357,7 +357,7 @@ impl Context { 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}"); + .unwrap_or_else(|| panic!("Cannot find linked fn {unresolved_fn_label}")); entry_point.link_with(artifact); } // Generate the final bytecode From 34b735530cd7832bec7d0f078a3d062e5c06a418 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 13 Jul 2023 07:43:46 +0000 Subject: [PATCH 2/5] refactor: use std hashset --- crates/noirc_evaluator/src/brillig/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/mod.rs b/crates/noirc_evaluator/src/brillig/mod.rs index a0c8a8275b3..a074900862a 100644 --- a/crates/noirc_evaluator/src/brillig/mod.rs +++ b/crates/noirc_evaluator/src/brillig/mod.rs @@ -1,8 +1,6 @@ pub(crate) mod brillig_gen; pub(crate) mod brillig_ir; -use im::HashSet; - use self::{ brillig_gen::{brillig_fn::FunctionContext, convert_ssa_function}, brillig_ir::artifact::{BrilligArtifact, Label}, @@ -14,7 +12,7 @@ use crate::ssa_refactor::{ }, ssa_gen::Ssa, }; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; /// Context structure for the brillig pass. /// It stores brillig-related data required for brillig generation. From d3c8645ebbaa31d1f4670c45dd0a03b84464dc55 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 13 Jul 2023 11:07:30 +0000 Subject: [PATCH 3/5] style: add expect message --- crates/noirc_evaluator/src/brillig/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/brillig/mod.rs b/crates/noirc_evaluator/src/brillig/mod.rs index a074900862a..e23e8526eb5 100644 --- a/crates/noirc_evaluator/src/brillig/mod.rs +++ b/crates/noirc_evaluator/src/brillig/mod.rs @@ -71,7 +71,9 @@ impl Ssa { .collect(); while !reachability_queue.is_empty() { - let func = &self.functions[&reachability_queue.pop().unwrap()]; + let func = &self.functions[&reachability_queue + .pop() + .expect("Queue should have already been checked for emptiness")]; reachable_function_ids.insert(func.id()); for (_, value) in func.dfg.values_iter() { // All reachable functions appear as literals after defunctionalization of the SSA From a31905be24c17397c212ecd1f15129508256545d Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 13 Jul 2023 11:11:20 +0000 Subject: [PATCH 4/5] style: reduce indentation --- crates/noirc_evaluator/src/brillig/mod.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/mod.rs b/crates/noirc_evaluator/src/brillig/mod.rs index e23e8526eb5..bf2dcd0b969 100644 --- a/crates/noirc_evaluator/src/brillig/mod.rs +++ b/crates/noirc_evaluator/src/brillig/mod.rs @@ -77,13 +77,19 @@ impl Ssa { reachable_function_ids.insert(func.id()); for (_, value) in func.dfg.values_iter() { // All reachable functions appear as literals after defunctionalization of the SSA - if let Value::Function(function_id) = value { - if !reachable_function_ids.contains(function_id) - && !reachability_queue.contains(function_id) - { - reachability_queue.push(*function_id); - } + let function_id = match value { + Value::Function(function_id) => function_id, + _ => continue, + }; + + // If the function is already reachable or enqueued, skip it. + if reachable_function_ids.contains(function_id) + || reachability_queue.contains(function_id) + { + continue; } + + reachability_queue.push(*function_id); } } From b466a9f7d6f9f776794b73e871ea718ef76dae6c Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 13 Jul 2023 11:22:53 +0000 Subject: [PATCH 5/5] style: improve readability --- crates/noirc_evaluator/src/brillig/mod.rs | 24 +++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/mod.rs b/crates/noirc_evaluator/src/brillig/mod.rs index bf2dcd0b969..c5ae0a514ed 100644 --- a/crates/noirc_evaluator/src/brillig/mod.rs +++ b/crates/noirc_evaluator/src/brillig/mod.rs @@ -53,7 +53,7 @@ impl Ssa { pub(crate) fn to_brillig(&self) -> Brillig { // Collect all the function ids that are reachable from brillig // That means all the functions marked as brillig and ACIR functions called by them - let mut reachable_function_ids: HashSet = HashSet::new(); + let mut brillig_reachable_function_ids: HashSet = HashSet::new(); // Initialize the queue with all the functions marked as brillig let mut reachability_queue: Vec = self @@ -70,31 +70,31 @@ impl Ssa { ) .collect(); - while !reachability_queue.is_empty() { - let func = &self.functions[&reachability_queue - .pop() - .expect("Queue should have already been checked for emptiness")]; - reachable_function_ids.insert(func.id()); + while let Some(func_id) = reachability_queue.pop() { + let func = &self.functions[&func_id]; + brillig_reachable_function_ids.insert(func.id()); + + // Explore all functions that are reachable from this function for (_, value) in func.dfg.values_iter() { // All reachable functions appear as literals after defunctionalization of the SSA - let function_id = match value { + let reachable_function = match value { Value::Function(function_id) => function_id, _ => continue, }; - // If the function is already reachable or enqueued, skip it. - if reachable_function_ids.contains(function_id) - || reachability_queue.contains(function_id) + // If the function is already reachable by brillig or enqueued, skip it. + if brillig_reachable_function_ids.contains(reachable_function) + || reachability_queue.contains(reachable_function) { continue; } - reachability_queue.push(*function_id); + reachability_queue.push(*reachable_function); } } let mut brillig = Brillig::default(); - for brillig_function_id in reachable_function_ids { + for brillig_function_id in brillig_reachable_function_ids { let func = &self.functions[&brillig_function_id]; brillig.compile(func); }