diff --git a/.github/benchmark_projects.yml b/.github/benchmark_projects.yml index 2035a18ee44..17eb7aa96a6 100644 --- a/.github/benchmark_projects.yml +++ b/.github/benchmark_projects.yml @@ -25,7 +25,7 @@ projects: path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset num_runs: 5 timeout: 250 - compilation-timeout: 7 + compilation-timeout: 8 execution-timeout: 0.35 compilation-memory-limit: 750 execution-memory-limit: 300 @@ -74,7 +74,7 @@ projects: num_runs: 1 timeout: 60 compilation-timeout: 100 - execution-timeout: 35 + execution-timeout: 40 compilation-memory-limit: 7000 execution-memory-limit: 1500 rollup-merge: diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs index 04c43df9d7e..036ff8b746f 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use acvm::FieldElement; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; @@ -6,9 +6,9 @@ use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; use super::{ BrilligArtifact, BrilligBlock, BrilligVariable, Function, FunctionContext, Label, ValueId, }; -use crate::brillig::{ - brillig_ir::BrilligContext, called_functions_vec, Brillig, BrilligOptions, DataFlowGraph, - FunctionId, Instruction, Value, +use crate::{ + brillig::{brillig_ir::BrilligContext, Brillig, BrilligOptions, DataFlowGraph, FunctionId}, + ssa::opt::brillig_entry_points::{build_inner_call_to_entry_points, get_brillig_entry_points}, }; /// Context structure for generating Brillig globals @@ -24,12 +24,12 @@ pub(crate) struct BrilligGlobals { /// Maps a Brillig entry point to all functions called in that entry point. /// This includes any nested calls as well, as we want to be able to associate /// any Brillig function with the appropriate global allocations. - brillig_entry_points: HashMap>, + brillig_entry_points: BTreeMap>, /// Maps an inner call to its Brillig entry point /// This is simply used to simplify fetching global allocations when compiling /// individual Brillig functions. - inner_call_to_entry_point: HashMap>, + inner_call_to_entry_point: HashMap>, /// Final map that associated an entry point with its Brillig global allocations entry_point_globals_map: HashMap, } @@ -43,106 +43,27 @@ impl BrilligGlobals { mut used_globals: HashMap>, main_id: FunctionId, ) -> Self { - let mut brillig_entry_points = HashMap::default(); - let acir_functions = functions.iter().filter(|(_, func)| func.runtime().is_acir()); - for (_, function) in acir_functions { - for block_id in function.reachable_blocks() { - for instruction_id in function.dfg[block_id].instructions() { - let instruction = &function.dfg[*instruction_id]; - let Instruction::Call { func: func_id, arguments: _ } = instruction else { - continue; - }; - - let func_value = &function.dfg[*func_id]; - let Value::Function(func_id) = func_value else { continue }; - - let called_function = &functions[func_id]; - if called_function.runtime().is_acir() { - continue; - } - - // We have now found a Brillig entry point. - // Let's recursively build a call graph to determine any functions - // whose parent is this entry point and any globals used in those internal calls. - brillig_entry_points.insert(*func_id, HashSet::default()); - Self::mark_entry_points_calls_recursive( - functions, - *func_id, - called_function, - &mut used_globals, - &mut brillig_entry_points, - im::HashSet::new(), - ); - } + let brillig_entry_points = get_brillig_entry_points(functions, main_id); + + // Mark any globals used in a Brillig entry point. + // Using the information collected we can determine which globals + // an entry point must initialize. + for (entry_point, entry_point_inner_calls) in brillig_entry_points.iter() { + for inner_call in entry_point_inner_calls.iter() { + let inner_globals = used_globals + .get(inner_call) + .expect("Should have a slot for each function") + .clone(); + used_globals + .get_mut(entry_point) + .expect("ICE: should have func") + .extend(inner_globals); } } - // If main has been marked as Brillig, it is itself an entry point. - // Run the same analysis from above on main. - let main_func = &functions[&main_id]; - if main_func.runtime().is_brillig() { - brillig_entry_points.insert(main_id, HashSet::default()); - Self::mark_entry_points_calls_recursive( - functions, - main_id, - main_func, - &mut used_globals, - &mut brillig_entry_points, - im::HashSet::new(), - ); - } + let inner_call_to_entry_point = build_inner_call_to_entry_points(&brillig_entry_points); - // NB: Temporary fix to override entry point analysis - let merged_set = - used_globals.values().flat_map(|set| set.iter().copied()).collect::>(); - for set in used_globals.values_mut() { - *set = merged_set.clone(); - } - - Self { used_globals, brillig_entry_points, ..Default::default() } - } - - /// Recursively mark any functions called in an entry point as well as - /// any globals used in those functions. - /// Using the information collected we can determine which globals - /// an entry point must initialize. - fn mark_entry_points_calls_recursive( - functions: &BTreeMap, - entry_point: FunctionId, - called_function: &Function, - used_globals: &mut HashMap>, - brillig_entry_points: &mut HashMap>, - mut explored_functions: im::HashSet, - ) { - if explored_functions.insert(called_function.id()).is_some() { - return; - } - - let inner_calls = called_functions_vec(called_function).into_iter().collect::>(); - - for inner_call in inner_calls { - let inner_globals = used_globals - .get(&inner_call) - .expect("Should have a slot for each function") - .clone(); - used_globals - .get_mut(&entry_point) - .expect("ICE: should have func") - .extend(inner_globals); - - if let Some(inner_calls) = brillig_entry_points.get_mut(&entry_point) { - inner_calls.insert(inner_call); - } - - Self::mark_entry_points_calls_recursive( - functions, - entry_point, - &functions[&inner_call], - used_globals, - brillig_entry_points, - explored_functions.clone(), - ); - } + Self { used_globals, brillig_entry_points, inner_call_to_entry_point, ..Default::default() } } pub(crate) fn declare_globals( @@ -151,18 +72,11 @@ impl BrilligGlobals { brillig: &mut Brillig, options: &BrilligOptions, ) { - // Map for fetching the correct entry point globals when compiling any function - let mut inner_call_to_entry_point: HashMap> = - HashMap::default(); let mut entry_point_globals_map = HashMap::default(); // We only need to generate globals for entry points - for (entry_point, entry_point_inner_calls) in self.brillig_entry_points.iter() { + for (entry_point, _) in self.brillig_entry_points.iter() { let entry_point = *entry_point; - for inner_call in entry_point_inner_calls { - inner_call_to_entry_point.entry(*inner_call).or_default().push(entry_point); - } - let used_globals = self.used_globals.remove(&entry_point).unwrap_or_default(); let (artifact, brillig_globals, globals_size) = convert_ssa_globals(options, globals_dfg, &used_globals, entry_point); @@ -173,7 +87,6 @@ impl BrilligGlobals { brillig.globals_memory_size.insert(entry_point, globals_size); } - self.inner_call_to_entry_point = inner_call_to_entry_point; self.entry_point_globals_map = entry_point_globals_map; } @@ -181,38 +94,34 @@ impl BrilligGlobals { /// by any given Brillig function (non-entry point or entry point). /// The allocations available to a function are determined by its entry point. /// For a given function id input, this function will search for that function's - /// entry point (or multiple entry points) and fetch the global allocations - /// associated with those entry points. + /// entry point and fetch the global allocations associated with that entry point. /// These allocations can then be used when compiling the Brillig function /// and resolving global variables. pub(crate) fn get_brillig_globals( &self, brillig_function_id: FunctionId, - ) -> SsaToBrilligGlobals { - let entry_points = self.inner_call_to_entry_point.get(&brillig_function_id); + ) -> Option<&SsaToBrilligGlobals> { + if let Some(globals) = self.entry_point_globals_map.get(&brillig_function_id) { + // Check whether `brillig_function_id` is itself an entry point. + // If so, return the global allocations directly from `self.entry_point_globals_map`. + return Some(globals); + } - let mut globals_allocations = HashMap::default(); - if let Some(entry_points) = entry_points { - // A Brillig function is used by multiple entry points. Fetch both globals allocations - // in case one is used by the internal call. - let entry_point_allocations = entry_points - .iter() - .flat_map(|entry_point| self.entry_point_globals_map.get(entry_point)) - .collect::>(); - for map in entry_point_allocations { - globals_allocations.extend(map); - } - } else if let Some(globals) = self.entry_point_globals_map.get(&brillig_function_id) { - // If there is no mapping from an inner call to an entry point, that means `brillig_function_id` - // is itself an entry point and we can fetch the global allocations directly from `self.entry_point_globals_map`. - // vec![globals] - globals_allocations.extend(globals); - } else { + let entry_points = self.inner_call_to_entry_point.get(&brillig_function_id); + let Some(entry_points) = entry_points else { unreachable!( "ICE: Expected global allocation to be set for function {brillig_function_id}" ); + }; + + // Sanity check: We should have guaranteed earlier that an inner call has only a single entry point + assert_eq!(entry_points.len(), 1, "{brillig_function_id} has multiple entry points"); + let entry_point = entry_points.first().expect("ICE: Inner call should have an entry point"); + if let Some(globals) = self.entry_point_globals_map.get(entry_point) { + return Some(globals); } - globals_allocations + + None } } @@ -312,11 +221,10 @@ mod tests { if func_id.to_u32() == 1 { assert_eq!( artifact.byte_code.len(), - 2, + 1, "Expected just a `Return`, but got more than a single opcode" ); - // TODO: Bring this back (https://github.com/noir-lang/noir/issues/7306) - // assert!(matches!(&artifact.byte_code[0], Opcode::Return)); + assert!(matches!(&artifact.byte_code[0], Opcode::Return)); } else if func_id.to_u32() == 2 { assert_eq!( artifact.byte_code.len(), @@ -430,17 +338,16 @@ mod tests { if func_id.to_u32() == 1 { assert_eq!( artifact.byte_code.len(), - 30, + 2, "Expected enough opcodes to initialize the globals" ); - // TODO: Bring this back (https://github.com/noir-lang/noir/issues/7306) - // let Opcode::Const { destination, bit_size, value } = &artifact.byte_code[0] else { - // panic!("First opcode is expected to be `Const`"); - // }; - // assert_eq!(destination.unwrap_direct(), GlobalSpace::start()); - // assert!(matches!(bit_size, BitSize::Field)); - // assert_eq!(*value, FieldElement::from(1u128)); - // assert!(matches!(&artifact.byte_code[1], Opcode::Return)); + let Opcode::Const { destination, bit_size, value } = &artifact.byte_code[0] else { + panic!("First opcode is expected to be `Const`"); + }; + assert_eq!(destination.unwrap_direct(), GlobalSpace::start()); + assert!(matches!(bit_size, BitSize::Field)); + assert_eq!(*value, FieldElement::from(1u128)); + assert!(matches!(&artifact.byte_code[1], Opcode::Return)); } else if func_id.to_u32() == 2 || func_id.to_u32() == 3 { // We want the entry point which uses globals (f2) and the entry point which calls f2 function internally (f3 through f4) // to have the same globals initialized. diff --git a/compiler/noirc_evaluator/src/brillig/mod.rs b/compiler/noirc_evaluator/src/brillig/mod.rs index a7e188602ce..8ec1b554932 100644 --- a/compiler/noirc_evaluator/src/brillig/mod.rs +++ b/compiler/noirc_evaluator/src/brillig/mod.rs @@ -17,10 +17,8 @@ use crate::ssa::{ ir::{ dfg::DataFlowGraph, function::{Function, FunctionId}, - instruction::Instruction, - value::{Value, ValueId}, + value::ValueId, }, - opt::inlining::called_functions_vec, ssa_gen::Ssa, }; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; @@ -122,10 +120,13 @@ impl Ssa { brillig_globals.declare_globals(&globals_dfg, &mut brillig, options); for brillig_function_id in brillig_reachable_function_ids { - let globals_allocations = brillig_globals.get_brillig_globals(brillig_function_id); + let empty_allocations = HashMap::default(); + let globals_allocations = brillig_globals + .get_brillig_globals(brillig_function_id) + .unwrap_or(&empty_allocations); let func = &self.functions[&brillig_function_id]; - brillig.compile(func, options, &globals_allocations); + brillig.compile(func, options, globals_allocations); } brillig diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 6d051da4550..74196d9a766 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -131,8 +131,8 @@ pub(crate) fn optimize_into_acir( .run_pass(|ssa| ssa.fold_constants_with_brillig(&brillig), "Inlining Brillig Calls Inlining") // It could happen that we inlined all calls to a given brillig function. // In that case it's unused so we can remove it. This is what we check next. - .run_pass(Ssa::remove_unreachable_functions, "Removing Unreachable Functions (3rd)") - .run_pass(Ssa::dead_instruction_elimination, "Dead Instruction Elimination (2nd)") + .run_pass(Ssa::remove_unreachable_functions, "Removing Unreachable Functions (4th)") + .run_pass(Ssa::dead_instruction_elimination, "Dead Instruction Elimination (3rd)") .finish(); if !options.skip_underconstrained_check { @@ -217,6 +217,12 @@ fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result Ssa { + if self.main().runtime().is_brillig() { + return self; + } + + let brillig_entry_points = get_brillig_entry_points(&self.functions, self.main_id); + + let functions_to_clone_map = build_functions_to_clone(&brillig_entry_points); + + let calls_to_update = build_calls_to_update(&mut self, functions_to_clone_map); + + let mut new_functions_map = HashMap::default(); + for (entry_point, inner_calls) in brillig_entry_points { + let new_entry_point = + new_functions_map.get(&entry_point).copied().unwrap_or(entry_point); + + let function = + self.functions.get_mut(&new_entry_point).expect("ICE: Function does not exist"); + update_function_calls(function, entry_point, &mut new_functions_map, &calls_to_update); + + for inner_call in inner_calls { + let new_inner_call = + new_functions_map.get(&inner_call).copied().unwrap_or(inner_call); + + let function = + self.functions.get_mut(&new_inner_call).expect("ICE: Function does not exist"); + update_function_calls( + function, + entry_point, + &mut new_functions_map, + &calls_to_update, + ); + } + } + + self + } +} + +/// For every call site, we can determine the entry point for a given callee. +/// Once we know that we can determine which functions are in need of duplication. +/// We duplicate when the following occurs: +/// 1. A function is called from two different entry points +/// 2. An entry point function is called from another entry point +fn build_functions_to_clone( + brillig_entry_points: &BTreeMap>, +) -> HashMap> { + let inner_call_to_entry_point = build_inner_call_to_entry_points(brillig_entry_points); + let entry_points = brillig_entry_points.keys().copied().collect::>(); + + let mut functions_to_clone_map: HashMap> = HashMap::default(); + + for (inner_call, inner_call_entry_points) in inner_call_to_entry_point { + let should_clone = inner_call_entry_points.len() > 1 || entry_points.contains(&inner_call); + if should_clone { + for entry_point in inner_call_entry_points { + functions_to_clone_map.entry(entry_point).or_default().push(inner_call); + } + } + } + + functions_to_clone_map +} + +/// Clones new functions and returns a mapping representing the calls to update. +/// +/// Returns a map of (entry point, callee function) -> new callee function id. +fn build_calls_to_update( + ssa: &mut Ssa, + functions_to_clone_map: HashMap>, +) -> HashMap<(FunctionId, FunctionId), FunctionId> { + let mut calls_to_update: HashMap<(FunctionId, FunctionId), FunctionId> = HashMap::default(); + + for (entry_point, functions_to_clone) in functions_to_clone_map { + for old_id in functions_to_clone { + let function = ssa.functions[&old_id].clone(); + ssa.add_fn(|id| { + calls_to_update.insert((entry_point, old_id), id); + Function::clone_with_id(id, &function) + }); + } + } + + calls_to_update +} + +fn update_function_calls( + function: &mut Function, + entry_point: FunctionId, + new_functions_map: &mut HashMap, + // Maps (entry point, callee function) -> new callee function id + calls_to_update: &HashMap<(FunctionId, FunctionId), FunctionId>, +) { + for block_id in function.reachable_blocks() { + #[allow(clippy::unnecessary_to_owned)] // clippy is wrong here + for instruction_id in function.dfg[block_id].instructions().to_vec() { + let instruction = function.dfg[instruction_id].clone(); + let Instruction::Call { func: func_id, arguments } = instruction else { + continue; + }; + + let func_value = &function.dfg[func_id]; + let Value::Function(func_id) = func_value else { continue }; + let Some(new_id) = calls_to_update.get(&(entry_point, *func_id)) else { + continue; + }; + + new_functions_map.insert(*func_id, *new_id); + let new_function_value_id = function.dfg.import_function(*new_id); + function.dfg[instruction_id] = + Instruction::Call { func: new_function_value_id, arguments }; + } + } +} + +/// Returns a map of Brillig entry points to all functions called in that entry point. +/// This includes any nested calls as well, as we want to be able to associate +/// any Brillig function with the appropriate global allocations. +pub(crate) fn get_brillig_entry_points( + functions: &BTreeMap, + main_id: FunctionId, +) -> BTreeMap> { + let mut brillig_entry_points = BTreeMap::default(); + let acir_functions = functions.iter().filter(|(_, func)| func.runtime().is_acir()); + for (_, function) in acir_functions { + for block_id in function.reachable_blocks() { + for instruction_id in function.dfg[block_id].instructions() { + let instruction = &function.dfg[*instruction_id]; + let Instruction::Call { func: func_id, arguments: _ } = instruction else { + continue; + }; + + let func_value = &function.dfg[*func_id]; + let Value::Function(func_id) = func_value else { continue }; + + let called_function = &functions[func_id]; + if called_function.runtime().is_acir() { + continue; + } + + // We have now found a Brillig entry point. + brillig_entry_points.insert(*func_id, BTreeSet::default()); + build_entry_points_map_recursive( + functions, + *func_id, + *func_id, + &mut brillig_entry_points, + im::HashSet::new(), + ); + } + } + } + + // If main has been marked as Brillig, it is itself an entry point. + // Run the same analysis from above on main. + let main_func = &functions[&main_id]; + if main_func.runtime().is_brillig() { + brillig_entry_points.insert(main_id, BTreeSet::default()); + build_entry_points_map_recursive( + functions, + main_id, + main_id, + &mut brillig_entry_points, + im::HashSet::new(), + ); + } + + brillig_entry_points +} + +/// Recursively mark any functions called in an entry point +fn build_entry_points_map_recursive( + functions: &BTreeMap, + entry_point: FunctionId, + called_function: FunctionId, + brillig_entry_points: &mut BTreeMap>, + mut explored_functions: im::HashSet, +) { + if explored_functions.insert(called_function).is_some() { + return; + } + + let inner_calls: HashSet = + called_functions_vec(&functions[&called_function]).into_iter().collect(); + + for inner_call in inner_calls { + if let Some(inner_calls) = brillig_entry_points.get_mut(&entry_point) { + inner_calls.insert(inner_call); + } + + build_entry_points_map_recursive( + functions, + entry_point, + inner_call, + brillig_entry_points, + explored_functions.clone(), + ); + } +} + +/// Builds a mapping from a [`FunctionId`] to the set of [`FunctionId`s][`FunctionId`] of all the brillig entrypoints +/// from which this function is reachable. +pub(crate) fn build_inner_call_to_entry_points( + brillig_entry_points: &BTreeMap>, +) -> HashMap> { + // Map for fetching the correct entry point globals when compiling any function + let mut inner_call_to_entry_point: HashMap> = + HashMap::default(); + + // We only need to generate globals for entry points + for (entry_point, entry_point_inner_calls) in brillig_entry_points.iter() { + for inner_call in entry_point_inner_calls { + inner_call_to_entry_point.entry(*inner_call).or_default().insert(*entry_point); + } + } + + inner_call_to_entry_point +} + +#[cfg(test)] +mod tests { + use crate::ssa::opt::assert_normalized_ssa_equals; + + use super::Ssa; + + #[test] + fn duplicate_inner_call_with_multiple_entry_points() { + let src = " + g0 = Field 1 + g1 = Field 2 + g2 = Field 3 + + acir(inline) fn main f0 { + b0(v3: Field, v4: Field): + call f1(v3, v4) + call f2(v3, v4) + return + } + brillig(inline) fn entry_point_one f1 { + b0(v3: Field, v4: Field): + v5 = add g0, v3 + v6 = add v5, v4 + constrain v6 == Field 2 + call f3(v3, v4) + return + } + brillig(inline) fn entry_point_two f2 { + b0(v3: Field, v4: Field): + v5 = add g1, v3 + v6 = add v5, v4 + constrain v6 == Field 3 + call f3(v3, v4) + return + } + brillig(inline) fn inner_func f3 { + b0(v3: Field, v4: Field): + v5 = add g2, v3 + v6 = add v5, v4 + constrain v6 == Field 4 + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.brillig_entry_point_analysis(); + let ssa = ssa.remove_unreachable_functions(); + + // We expect `inner_func` to be duplicated + let expected = " + g0 = Field 1 + g1 = Field 2 + g2 = Field 3 + + acir(inline) fn main f0 { + b0(v3: Field, v4: Field): + call f1(v3, v4) + call f2(v3, v4) + return + } + brillig(inline) fn entry_point_one f1 { + b0(v3: Field, v4: Field): + v5 = add Field 1, v3 + v6 = add v5, v4 + constrain v6 == Field 2 + call f3(v3, v4) + return + } + brillig(inline) fn entry_point_two f2 { + b0(v3: Field, v4: Field): + v5 = add Field 2, v3 + v6 = add v5, v4 + constrain v6 == Field 3 + call f4(v3, v4) + return + } + brillig(inline) fn inner_func f3 { + b0(v3: Field, v4: Field): + v5 = add Field 3, v3 + v6 = add v5, v4 + constrain v6 == Field 4 + return + } + brillig(inline) fn inner_func f4 { + b0(v3: Field, v4: Field): + v5 = add Field 3, v3 + v6 = add v5, v4 + constrain v6 == Field 4 + return + } + "; + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn duplicate_inner_call_with_multiple_entry_points_nested() { + let src = " + g0 = Field 2 + g1 = Field 3 + + acir(inline) fn main f0 { + b0(v2: Field, v3: Field): + call f1(v2, v3) + call f2(v2, v3) + return + } + brillig(inline) fn entry_point_one f1 { + b0(v2: Field, v3: Field): + v4 = add g0, v2 + v5 = add v4, v3 + constrain v5 == Field 3 + call f3(v2, v3) + return + } + brillig(inline) fn entry_point_two f2 { + b0(v2: Field, v3: Field): + v4 = add g0, v2 + v5 = add v4, v3 + constrain v5 == Field 3 + call f3(v2, v3) + return + } + brillig(inline) fn inner_func f3 { + b0(v2: Field, v3: Field): + v4 = add g0, v2 + v5 = add v4, v3 + constrain v5 == Field 3 + call f4(v2, v3) + return + } + brillig(inline) fn nested_inner_func f4 { + b0(v2: Field, v3: Field): + v4 = add g1, v2 + v5 = add v4, v3 + constrain v5 == Field 4 + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.brillig_entry_point_analysis(); + let ssa = ssa.remove_unreachable_functions(); + + // We expect both `inner_func` and `nested_inner_func` to be duplicated + let expected = " + g0 = Field 2 + g1 = Field 3 + + acir(inline) fn main f0 { + b0(v2: Field, v3: Field): + call f1(v2, v3) + call f2(v2, v3) + return + } + brillig(inline) fn entry_point_one f1 { + b0(v2: Field, v3: Field): + v4 = add Field 2, v2 + v5 = add v4, v3 + constrain v5 == Field 3 + call f4(v2, v3) + return + } + brillig(inline) fn entry_point_two f2 { + b0(v2: Field, v3: Field): + v4 = add Field 2, v2 + v5 = add v4, v3 + constrain v5 == Field 3 + call f6(v2, v3) + return + } + brillig(inline) fn nested_inner_func f3 { + b0(v2: Field, v3: Field): + v4 = add Field 3, v2 + v5 = add v4, v3 + constrain v5 == Field 4 + return + } + brillig(inline) fn inner_func f4 { + b0(v2: Field, v3: Field): + v4 = add Field 2, v2 + v5 = add v4, v3 + constrain v5 == Field 3 + call f3(v2, v3) + return + } + brillig(inline) fn nested_inner_func f5 { + b0(v2: Field, v3: Field): + v4 = add Field 3, v2 + v5 = add v4, v3 + constrain v5 == Field 4 + return + } + brillig(inline) fn inner_func f6 { + b0(v2: Field, v3: Field): + v4 = add Field 2, v2 + v5 = add v4, v3 + constrain v5 == Field 3 + call f5(v2, v3) + return + } + "; + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn duplicate_entry_point_called_from_entry_points() { + // Check that we duplicate entry points that are also called from another entry point. + // In this test the entry points used in other entry points are f2 and f3. + // These functions are also called within the wrapper function f4, as we also want to make sure + // that we duplicate entry points called from another entry point's inner calls. + let src = " + g0 = Field 2 + g1 = Field 3 + g2 = Field 1 + + acir(inline) fn main f0 { + b0(v3: Field, v4: Field): + call f1(v3, v4) + call f2(v3, v4) + call f3(v3, v4) + return + } + brillig(inline) fn entry_point_inner_func_globals f1 { + b0(v3: Field, v4: Field): + call f4(v3, v4) + return + } + brillig(inline) fn entry_point_one_global f2 { + b0(v3: Field, v4: Field): + v5 = add g0, v3 + v6 = add v5, v4 + constrain v6 == Field 3 + return + } + brillig(inline) fn entry_point_one_diff_global f3 { + b0(v3: Field, v4: Field): + v5 = add g1, v3 + v6 = add v5, v4 + constrain v6 == Field 4 + return + } + brillig(inline) fn wrapper f4 { + b0(v3: Field, v4: Field): + v5 = add g2, v3 + v6 = add v5, v4 + constrain v6 == Field 2 + call f2(v3, v4) + call f3(v4, v3) + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.brillig_entry_point_analysis(); + + // We expect `entry_point_one_global` and `entry_point_one_diff_global` to be duplicated + let expected = " + g0 = Field 2 + g1 = Field 3 + g2 = Field 1 + + acir(inline) fn main f0 { + b0(v3: Field, v4: Field): + call f1(v3, v4) + call f2(v3, v4) + call f3(v3, v4) + return + } + brillig(inline) fn entry_point_inner_func_globals f1 { + b0(v3: Field, v4: Field): + call f4(v3, v4) + return + } + brillig(inline) fn entry_point_one_global f2 { + b0(v3: Field, v4: Field): + v5 = add Field 2, v3 + v6 = add v5, v4 + constrain v6 == Field 3 + return + } + brillig(inline) fn entry_point_one_diff_global f3 { + b0(v3: Field, v4: Field): + v5 = add Field 3, v3 + v6 = add v5, v4 + constrain v6 == Field 4 + return + } + brillig(inline) fn wrapper f4 { + b0(v3: Field, v4: Field): + v5 = add Field 1, v3 + v6 = add v5, v4 + constrain v6 == Field 2 + call f5(v3, v4) + call f6(v4, v3) + return + } + brillig(inline) fn entry_point_one_global f5 { + b0(v3: Field, v4: Field): + v5 = add Field 2, v3 + v6 = add v5, v4 + constrain v6 == Field 3 + return + } + brillig(inline) fn entry_point_one_diff_global f6 { + b0(v3: Field, v4: Field): + v5 = add Field 3, v3 + v6 = add v5, v4 + constrain v6 == Field 4 + return + } + "; + assert_normalized_ssa_equals(ssa, expected); + } +} diff --git a/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/mod.rs index 7ec419890c0..4d8a652b94d 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -7,6 +7,7 @@ mod array_set; mod as_slice_length; mod assert_constant; +pub(crate) mod brillig_entry_points; mod constant_folding; mod defunctionalize; mod die; diff --git a/test_programs/execution_success/global_var_entry_point_used_in_another_entry/Nargo.toml b/test_programs/execution_success/global_var_entry_point_used_in_another_entry/Nargo.toml new file mode 100644 index 00000000000..2327a6e1097 --- /dev/null +++ b/test_programs/execution_success/global_var_entry_point_used_in_another_entry/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "global_var_entry_point_used_in_another_entry" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/global_var_entry_point_used_in_another_entry/Prover.toml b/test_programs/execution_success/global_var_entry_point_used_in_another_entry/Prover.toml new file mode 100644 index 00000000000..4c144083f00 --- /dev/null +++ b/test_programs/execution_success/global_var_entry_point_used_in_another_entry/Prover.toml @@ -0,0 +1,2 @@ +x = 0 +y = 1 diff --git a/test_programs/execution_success/global_var_entry_point_used_in_another_entry/src/main.nr b/test_programs/execution_success/global_var_entry_point_used_in_another_entry/src/main.nr new file mode 100644 index 00000000000..364c27cc4ab --- /dev/null +++ b/test_programs/execution_success/global_var_entry_point_used_in_another_entry/src/main.nr @@ -0,0 +1,44 @@ +global ONE: Field = 1; +global TWO: Field = 2; +global THREE: Field = 3; + +fn main(x: Field, y: pub Field) { + // Safety: testing context + unsafe { + entry_point_no_global(x, y); + entry_point_inner_func_globals(x, y); + entry_point_one_global(x, y); + entry_point_one_diff_global(x, y); + } +} + +unconstrained fn entry_point_no_global(x: Field, y: Field) { + assert(x + y != 100); +} + +unconstrained fn entry_point_one_global(x: Field, y: Field) { + let z = TWO + x + y; + assert(z == 3); +} + +unconstrained fn entry_point_inner_func_globals(x: Field, y: Field) { + wrapper(x, y); +} + +// Test that we duplicate Brillig entry points called within +// another entry point's inner calls +unconstrained fn wrapper(x: Field, y: Field) { + let z = ONE + x + y; + assert(z == 2); + entry_point_one_global(x, y); + // Test that we handle repeated entry point calls + // `entry_point_one_diff_global` should be duplicated and the duplicated function + // should use the globals from `entry_point_inner_func_globals` + entry_point_one_diff_global(y, x); +} + +unconstrained fn entry_point_one_diff_global(x: Field, y: Field) { + let z = THREE + x + y; + assert(z == 4); +} + diff --git a/test_programs/execution_success/global_var_func_with_multiple_entry_points/Nargo.toml b/test_programs/execution_success/global_var_func_with_multiple_entry_points/Nargo.toml new file mode 100644 index 00000000000..4d2bd864e8b --- /dev/null +++ b/test_programs/execution_success/global_var_func_with_multiple_entry_points/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "global_var_func_with_multiple_entry_points" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/global_var_func_with_multiple_entry_points/Prover.toml b/test_programs/execution_success/global_var_func_with_multiple_entry_points/Prover.toml new file mode 100644 index 00000000000..4c144083f00 --- /dev/null +++ b/test_programs/execution_success/global_var_func_with_multiple_entry_points/Prover.toml @@ -0,0 +1,2 @@ +x = 0 +y = 1 diff --git a/test_programs/execution_success/global_var_func_with_multiple_entry_points/src/main.nr b/test_programs/execution_success/global_var_func_with_multiple_entry_points/src/main.nr new file mode 100644 index 00000000000..6ae68e21e79 --- /dev/null +++ b/test_programs/execution_success/global_var_func_with_multiple_entry_points/src/main.nr @@ -0,0 +1,28 @@ +global ONE: Field = 1; +global TWO: Field = 2; +global THREE: Field = 3; + +fn main(x: Field, y: pub Field) { + // Safety: testing context + unsafe { + entry_point_one(x, y); + entry_point_two(x, y); + } +} + +unconstrained fn entry_point_one(x: Field, y: Field) { + let z = ONE + x + y; + assert(z == 2); + inner_func(x, y); +} + +unconstrained fn entry_point_two(x: Field, y: Field) { + let z = TWO + x + y; + assert(z == 3); + inner_func(x, y); +} + +unconstrained fn inner_func(x: Field, y: Field) { + let z = THREE + x + y; + assert(z == 4); +} diff --git a/test_programs/execution_success/global_var_multiple_entry_points_nested/Nargo.toml b/test_programs/execution_success/global_var_multiple_entry_points_nested/Nargo.toml new file mode 100644 index 00000000000..e7ef4a6f36d --- /dev/null +++ b/test_programs/execution_success/global_var_multiple_entry_points_nested/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "global_var_multiple_entry_points_nested" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/global_var_multiple_entry_points_nested/Prover.toml b/test_programs/execution_success/global_var_multiple_entry_points_nested/Prover.toml new file mode 100644 index 00000000000..4c144083f00 --- /dev/null +++ b/test_programs/execution_success/global_var_multiple_entry_points_nested/Prover.toml @@ -0,0 +1,2 @@ +x = 0 +y = 1 diff --git a/test_programs/execution_success/global_var_multiple_entry_points_nested/src/main.nr b/test_programs/execution_success/global_var_multiple_entry_points_nested/src/main.nr new file mode 100644 index 00000000000..57e6b872ac3 --- /dev/null +++ b/test_programs/execution_success/global_var_multiple_entry_points_nested/src/main.nr @@ -0,0 +1,34 @@ +global TWO: Field = 2; +global THREE: Field = 3; + +fn main(x: Field, y: pub Field) { + // Safety: testing context + unsafe { + entry_point_one(x, y); + entry_point_two(x, y); + } +} + +unconstrained fn entry_point_one(x: Field, y: Field) { + let z = TWO + x + y; + assert(z == 3); + inner_func(x, y); +} + +// Identical to `entry_point_one` +unconstrained fn entry_point_two(x: Field, y: Field) { + let z = TWO + x + y; + assert(z == 3); + inner_func(x, y); +} + +unconstrained fn inner_func(x: Field, y: Field) { + let z = TWO + x + y; + assert(z == 3); + nested_inner_func(x, y); +} + +unconstrained fn nested_inner_func(x: Field, y: Field) { + let z = THREE + x + y; + assert(z == 4); +} diff --git a/test_programs/execution_success/global_var_regression_entry_points/src/consts.nr b/test_programs/execution_success/global_var_regression_entry_points/src/consts.nr index 7ad6a4a54d1..43b580cf781 100644 --- a/test_programs/execution_success/global_var_regression_entry_points/src/consts.nr +++ b/test_programs/execution_success/global_var_regression_entry_points/src/consts.nr @@ -1,4 +1,4 @@ -global EXPONENTIATE: [[Field; 257]; 257] = [ +pub(crate) global EXPONENTIATE: [[Field; 257]; 257] = [ [ 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,