diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 5c7899b5035..46d0924b322 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -24,12 +24,10 @@ mod big_int; mod brillig_directive; mod generated_acir; +use crate::brillig::brillig_gen::gen_brillig_for; use crate::brillig::{ brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, - brillig_ir::{ - artifact::{BrilligParameter, GeneratedBrillig}, - BrilligContext, - }, + brillig_ir::artifact::{BrilligParameter, GeneratedBrillig}, Brillig, }; use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport}; @@ -518,7 +516,7 @@ impl<'a> Context<'a> { let outputs: Vec = vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into()); - let code = self.gen_brillig_for(main_func, arguments.clone(), brillig)?; + let code = gen_brillig_for(main_func, arguments.clone(), brillig)?; // We specifically do not attempt execution of the brillig code being generated as this can result in it being // replaced with constraints on witnesses to the program outputs. @@ -878,8 +876,7 @@ impl<'a> Context<'a> { None, )? } else { - let code = - self.gen_brillig_for(func, arguments.clone(), brillig)?; + let code = gen_brillig_for(func, arguments.clone(), brillig)?; let generated_pointer = self.shared_context.new_generated_pointer(); let output_values = self.acir_context.brillig_call( @@ -999,47 +996,6 @@ impl<'a> Context<'a> { .collect() } - fn gen_brillig_for( - &self, - func: &Function, - arguments: Vec, - brillig: &Brillig, - ) -> Result, InternalError> { - // Create the entry point artifact - let mut entry_point = BrilligContext::new_entry_point_artifact( - arguments, - BrilligFunctionContext::return_values(func), - func.id(), - ); - entry_point.name = func.name().to_string(); - - // Link the entry point with all dependencies - while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() { - let artifact = &brillig.find_by_label(unresolved_fn_label.clone()); - let artifact = match artifact { - Some(artifact) => artifact, - None => { - return Err(InternalError::General { - message: format!("Cannot find linked fn {unresolved_fn_label}"), - call_stack: CallStack::new(), - }) - } - }; - entry_point.link_with(artifact); - // Insert the range of opcode locations occupied by a procedure - if let Some(procedure_id) = &artifact.procedure { - let num_opcodes = entry_point.byte_code.len(); - let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len(); - // We subtract one as to keep the range inclusive on both ends - entry_point - .procedure_locations - .insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1)); - } - } - // Generate the final bytecode - Ok(entry_point.finish()) - } - /// Handles an ArrayGet or ArraySet instruction. /// To set an index of the array (and create a new array in doing so), pass Some(value) for /// store_value. To just retrieve an index of the array, pass None for store_value. diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs index 786a03031d6..ca4e783aa93 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs @@ -9,11 +9,17 @@ mod variable_liveness; use acvm::FieldElement; use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext}; -use super::brillig_ir::{ - artifact::{BrilligArtifact, Label}, - BrilligContext, +use super::{ + brillig_ir::{ + artifact::{BrilligArtifact, BrilligParameter, GeneratedBrillig, Label}, + BrilligContext, + }, + Brillig, +}; +use crate::{ + errors::InternalError, + ssa::ir::{dfg::CallStack, function::Function}, }; -use crate::ssa::ir::function::Function; /// Converting an SSA function into Brillig bytecode. pub(crate) fn convert_ssa_function( @@ -36,3 +42,43 @@ pub(crate) fn convert_ssa_function( artifact.name = func.name().to_string(); artifact } + +pub(crate) fn gen_brillig_for( + func: &Function, + arguments: Vec, + brillig: &Brillig, +) -> Result, InternalError> { + // Create the entry point artifact + let mut entry_point = BrilligContext::new_entry_point_artifact( + arguments, + FunctionContext::return_values(func), + func.id(), + ); + entry_point.name = func.name().to_string(); + + // Link the entry point with all dependencies + while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() { + let artifact = &brillig.find_by_label(unresolved_fn_label.clone()); + let artifact = match artifact { + Some(artifact) => artifact, + None => { + return Err(InternalError::General { + message: format!("Cannot find linked fn {unresolved_fn_label}"), + call_stack: CallStack::new(), + }) + } + }; + entry_point.link_with(artifact); + // Insert the range of opcode locations occupied by a procedure + if let Some(procedure_id) = &artifact.procedure { + let num_opcodes = entry_point.byte_code.len(); + let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len(); + // We subtract one as to keep the range inclusive on both ends + entry_point + .procedure_locations + .insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1)); + } + } + // Generate the final bytecode + Ok(entry_point.finish()) +} diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 9e11441caf4..f0a6fcd986f 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -140,6 +140,23 @@ pub(crate) fn optimize_into_acir( ssa.to_brillig(options.enable_brillig_logging) }); + let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); + let ssa_gen_span_guard = ssa_gen_span.enter(); + + let ssa = SsaBuilder { + ssa, + print_ssa_passes: options.enable_ssa_logging, + print_codegen_timings: options.print_codegen_timings, + } + .run_pass( + |ssa| ssa.fold_constants_with_brillig(&brillig), + "After Constant Folding with Brillig:", + ) + .run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:") + .finish(); + + drop(ssa_gen_span_guard); + let artifacts = time("SSA to ACIR", options.print_codegen_timings, || { ssa.into_acir(&brillig, options.expression_width) })?; diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 32f66e5a0f0..019bace33a3 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -19,22 +19,35 @@ //! //! This is the only pass which removes duplicated pure [`Instruction`]s however and so is needed when //! different blocks are merged, i.e. after the [`flatten_cfg`][super::flatten_cfg] pass. -use std::collections::{HashSet, VecDeque}; +use std::collections::{BTreeMap, HashSet, VecDeque}; -use acvm::{acir::AcirField, FieldElement}; +use acvm::{ + acir::AcirField, + brillig_vm::{MemoryValue, VMStatus, VM}, + FieldElement, +}; +use bn254_blackbox_solver::Bn254BlackBoxSolver; +use im::Vector; use iter_extended::vecmap; -use crate::ssa::{ - ir::{ - basic_block::BasicBlockId, - dfg::{DataFlowGraph, InsertInstructionResult}, - dom::DominatorTree, - function::Function, - instruction::{Instruction, InstructionId}, - types::Type, - value::{Value, ValueId}, +use crate::{ + brillig::{ + brillig_gen::gen_brillig_for, + brillig_ir::{artifact::BrilligParameter, brillig_variable::get_bit_size_from_ssa_type}, + Brillig, + }, + ssa::{ + ir::{ + basic_block::BasicBlockId, + dfg::{DataFlowGraph, InsertInstructionResult}, + dom::DominatorTree, + function::{Function, FunctionId, RuntimeType}, + instruction::{Instruction, InstructionId}, + types::Type, + value::{Value, ValueId}, + }, + ssa_gen::Ssa, }, - ssa_gen::Ssa, }; use fxhash::FxHashMap as HashMap; @@ -45,7 +58,7 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn fold_constants(mut self) -> Ssa { for function in self.functions.values_mut() { - function.constant_fold(false); + function.constant_fold(false, None); } self } @@ -58,17 +71,82 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn fold_constants_using_constraints(mut self) -> Ssa { for function in self.functions.values_mut() { - function.constant_fold(true); + function.constant_fold(true, None); } self } + + /// Performs constant folding on each instruction while also replacing calls to brillig functions + /// with all constant arguments by trying to evaluate those calls. + #[tracing::instrument(level = "trace", skip(self, brillig))] + pub(crate) fn fold_constants_with_brillig(mut self, brillig: &Brillig) -> Ssa { + // Collect all brillig functions so that later we can find them when processing a call instruction + let mut brillig_functions: BTreeMap = BTreeMap::new(); + for (func_id, func) in &self.functions { + if let RuntimeType::Brillig(..) = func.runtime() { + let cloned_function = Function::clone_with_id(*func_id, func); + brillig_functions.insert(*func_id, cloned_function); + }; + } + + let brillig_info = Some(BrilligInfo { brillig, brillig_functions: &brillig_functions }); + + for function in self.functions.values_mut() { + function.constant_fold(false, brillig_info); + } + + // 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. + self.remove_unused_brillig_functions(brillig_functions) + } + + fn remove_unused_brillig_functions( + mut self, + mut brillig_functions: BTreeMap, + ) -> Ssa { + // Remove from the above map functions that are called + for function in self.functions.values() { + 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 }; + + brillig_functions.remove(func_id); + } + } + } + + // The ones that remain are never called: let's remove them. + for func_id in brillig_functions.keys() { + // We never want to remove the main function (it could be `unconstrained` or it + // could have been turned into brillig if `--force-brillig` was given). + // We also don't want to remove entry points. + if self.main_id == *func_id || self.entry_point_to_generated_index.contains_key(func_id) + { + continue; + } + + self.functions.remove(func_id); + } + + self + } } impl Function { /// The structure of this pass is simple: /// Go through each block and re-insert all instructions. - pub(crate) fn constant_fold(&mut self, use_constraint_info: bool) { - let mut context = Context::new(self, use_constraint_info); + pub(crate) fn constant_fold( + &mut self, + use_constraint_info: bool, + brillig_info: Option, + ) { + let mut context = Context::new(self, use_constraint_info, brillig_info); context.block_queue.push_back(self.entry_block()); while let Some(block) = context.block_queue.pop_front() { @@ -82,8 +160,9 @@ impl Function { } } -struct Context { +struct Context<'a> { use_constraint_info: bool, + brillig_info: Option>, /// Maps pre-folded ValueIds to the new ValueIds obtained by re-inserting the instruction. visited_blocks: HashSet, block_queue: VecDeque, @@ -103,6 +182,12 @@ struct Context { dom: DominatorTree, } +#[derive(Copy, Clone)] +pub(crate) struct BrilligInfo<'a> { + brillig: &'a Brillig, + brillig_functions: &'a BTreeMap, +} + /// HashMap from (Instruction, side_effects_enabled_var) to the results of the instruction. /// Stored as a two-level map to avoid cloning Instructions during the `.get` call. /// @@ -118,10 +203,15 @@ struct ResultCache { result: Option<(BasicBlockId, Vec)>, } -impl Context { - fn new(function: &Function, use_constraint_info: bool) -> Self { +impl<'brillig> Context<'brillig> { + fn new( + function: &Function, + use_constraint_info: bool, + brillig_info: Option>, + ) -> Self { Self { use_constraint_info, + brillig_info, visited_blocks: Default::default(), block_queue: Default::default(), constraint_simplification_mappings: Default::default(), @@ -177,8 +267,25 @@ impl Context { } } - // Otherwise, try inserting the instruction again to apply any optimizations using the newly resolved inputs. - let new_results = Self::push_instruction(id, instruction.clone(), &old_results, block, dfg); + let new_results = + // First try to inline a call to a brillig function with all constant arguments. + Self::try_inline_brillig_call_with_all_constants( + &instruction, + &old_results, + block, + dfg, + self.brillig_info, + ) + .unwrap_or_else(|| { + // Otherwise, try inserting the instruction again to apply any optimizations using the newly resolved inputs. + Self::push_instruction( + id, + instruction.clone(), + &old_results, + block, + dfg, + ) + }); Self::replace_result_ids(dfg, &old_results, &new_results); @@ -342,6 +449,166 @@ impl Context { results_for_instruction.get(&predicate)?.get(block, &mut self.dom) } + + /// Checks if the given instruction is a call to a brillig function with all constant arguments. + /// If so, we can try to evaluate that function and replace the results with the evaluation results. + fn try_inline_brillig_call_with_all_constants( + instruction: &Instruction, + old_results: &[ValueId], + block: BasicBlockId, + dfg: &mut DataFlowGraph, + brillig_info: Option, + ) -> Option> { + let evaluation_result = Self::evaluate_const_brillig_call( + instruction, + brillig_info?.brillig, + brillig_info?.brillig_functions, + dfg, + ); + + match evaluation_result { + EvaluationResult::NotABrilligCall | EvaluationResult::CannotEvaluate(_) => None, + EvaluationResult::Evaluated(memory_values) => { + let mut memory_index = 0; + let new_results = vecmap(old_results, |old_result| { + let typ = dfg.type_of_value(*old_result); + Self::new_value_for_type_and_memory_values( + typ, + block, + &memory_values, + &mut memory_index, + dfg, + ) + }); + Some(new_results) + } + } + } + + /// Tries to evaluate an instruction if it's a call that points to a brillig function, + /// and all its arguments are constant. + /// We do this by directly executing the function with a brillig VM. + fn evaluate_const_brillig_call( + instruction: &Instruction, + brillig: &Brillig, + brillig_functions: &BTreeMap, + dfg: &mut DataFlowGraph, + ) -> EvaluationResult { + let Instruction::Call { func: func_id, arguments } = instruction else { + return EvaluationResult::NotABrilligCall; + }; + + let func_value = &dfg[*func_id]; + let Value::Function(func_id) = func_value else { + return EvaluationResult::NotABrilligCall; + }; + + let Some(func) = brillig_functions.get(func_id) else { + return EvaluationResult::NotABrilligCall; + }; + + if !arguments.iter().all(|argument| dfg.is_constant(*argument)) { + return EvaluationResult::CannotEvaluate(*func_id); + } + + let mut brillig_arguments = Vec::new(); + for argument in arguments { + let typ = dfg.type_of_value(*argument); + let Some(parameter) = type_to_brillig_parameter(&typ) else { + return EvaluationResult::CannotEvaluate(*func_id); + }; + brillig_arguments.push(parameter); + } + + // Check that return value types are supported by brillig + for return_id in func.returns().iter() { + let typ = func.dfg.type_of_value(*return_id); + if type_to_brillig_parameter(&typ).is_none() { + return EvaluationResult::CannotEvaluate(*func_id); + } + } + + let Ok(generated_brillig) = gen_brillig_for(func, brillig_arguments, brillig) else { + return EvaluationResult::CannotEvaluate(*func_id); + }; + + let mut calldata = Vec::new(); + for argument in arguments { + value_id_to_calldata(*argument, dfg, &mut calldata); + } + + let bytecode = &generated_brillig.byte_code; + let foreign_call_results = Vec::new(); + let black_box_solver = Bn254BlackBoxSolver; + let profiling_active = false; + let mut vm = + VM::new(calldata, bytecode, foreign_call_results, &black_box_solver, profiling_active); + let vm_status: VMStatus<_> = vm.process_opcodes(); + let VMStatus::Finished { return_data_offset, return_data_size } = vm_status else { + return EvaluationResult::CannotEvaluate(*func_id); + }; + + let memory = + vm.get_memory()[return_data_offset..(return_data_offset + return_data_size)].to_vec(); + + EvaluationResult::Evaluated(memory) + } + + /// Creates a new value inside this function by reading it from `memory_values` starting at + /// `memory_index` depending on the given Type: if it's an array multiple values will be read + /// and a new `make_array` instruction will be created. + fn new_value_for_type_and_memory_values( + typ: Type, + block_id: BasicBlockId, + memory_values: &[MemoryValue], + memory_index: &mut usize, + dfg: &mut DataFlowGraph, + ) -> ValueId { + match typ { + Type::Numeric(_) => { + let memory = memory_values[*memory_index]; + *memory_index += 1; + + let field_value = match memory { + MemoryValue::Field(field_value) => field_value, + MemoryValue::Integer(u128_value, _) => u128_value.into(), + }; + dfg.make_constant(field_value, typ) + } + Type::Array(types, length) => { + let mut new_array_values = Vector::new(); + for _ in 0..length { + for typ in types.iter() { + let new_value = Self::new_value_for_type_and_memory_values( + typ.clone(), + block_id, + memory_values, + memory_index, + dfg, + ); + new_array_values.push_back(new_value); + } + } + + let instruction = Instruction::MakeArray { + elements: new_array_values, + typ: Type::Array(types, length), + }; + let instruction_id = dfg.make_instruction(instruction, None); + dfg[block_id].instructions_mut().push(instruction_id); + *dfg.instruction_results(instruction_id).first().unwrap() + } + Type::Reference(_) => { + panic!("Unexpected reference type in brillig function result") + } + Type::Slice(_) => { + panic!("Unexpected slice type in brillig function result") + } + Type::Function => { + panic!("Unexpected function type in brillig function result") + } + } + } } impl ResultCache { @@ -376,6 +643,50 @@ enum CacheResult<'a> { NeedToHoistToCommonBlock(BasicBlockId, &'a [ValueId]), } +/// Result of trying to evaluate an instruction (any instruction) in this pass. +enum EvaluationResult { + /// Nothing was done because the instruction wasn't a call to a brillig function, + /// or some arguments to it were not constants. + NotABrilligCall, + /// The instruction was a call to a brillig function, but we couldn't evaluate it. + /// This can occur in the situation where the brillig function reaches a "trap" or a foreign call opcode. + CannotEvaluate(FunctionId), + /// The instruction was a call to a brillig function and we were able to evaluate it, + /// returning evaluation memory values. + Evaluated(Vec>), +} + +/// Similar to FunctionContext::ssa_type_to_parameter but never panics and disallows reference types. +pub(crate) fn type_to_brillig_parameter(typ: &Type) -> Option { + match typ { + Type::Numeric(_) => Some(BrilligParameter::SingleAddr(get_bit_size_from_ssa_type(typ))), + Type::Array(item_type, size) => { + let mut parameters = Vec::with_capacity(item_type.len()); + for item_typ in item_type.iter() { + parameters.push(type_to_brillig_parameter(item_typ)?); + } + Some(BrilligParameter::Array(parameters, *size)) + } + _ => None, + } +} + +fn value_id_to_calldata(value_id: ValueId, dfg: &DataFlowGraph, calldata: &mut Vec) { + if let Some(value) = dfg.get_numeric_constant(value_id) { + calldata.push(value); + return; + } + + if let Some((values, _type)) = dfg.get_array_constant(value_id) { + for value in values { + value_id_to_calldata(value, dfg, calldata); + } + return; + } + + panic!("Expected ValueId to be numeric constant or array constant"); +} + #[cfg(test)] mod test { use std::sync::Arc; @@ -854,4 +1165,180 @@ mod test { let ssa = ssa.fold_constants_using_constraints(); assert_normalized_ssa_equals(ssa, expected); } + + #[test] + fn inlines_brillig_call_without_arguments() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1() -> Field + return v0 + } + + brillig(inline) fn one f1 { + b0(): + v0 = add Field 2, Field 3 + return v0 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + return Field 5 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_two_field_arguments() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1(Field 2, Field 3) -> Field + return v0 + } + + brillig(inline) fn one f1 { + b0(v0: Field, v1: Field): + v2 = add v0, v1 + return v2 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + return Field 5 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_two_i32_arguments() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1(i32 2, i32 3) -> i32 + return v0 + } + + brillig(inline) fn one f1 { + b0(v0: i32, v1: i32): + v2 = add v0, v1 + return v2 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + return i32 5 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_array_return() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1(Field 2, Field 3, Field 4) -> [Field; 3] + return v0 + } + + brillig(inline) fn one f1 { + b0(v0: Field, v1: Field, v2: Field): + v3 = make_array [v0, v1, v2] : [Field; 3] + return v3 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + v3 = make_array [Field 2, Field 3, Field 4] : [Field; 3] + return v3 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_composite_array_return() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1(Field 2, i32 3, Field 4, i32 5) -> [(Field, i32); 2] + return v0 + } + + brillig(inline) fn one f1 { + b0(v0: Field, v1: i32, v2: i32, v3: Field): + v4 = make_array [v0, v1, v2, v3] : [(Field, i32); 2] + return v4 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + v4 = make_array [Field 2, i32 3, Field 4, i32 5] : [(Field, i32); 2] + return v4 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_array_arguments() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = make_array [Field 2, Field 3] : [Field; 2] + v1 = call f1(v0) -> Field + return v1 + } + + brillig(inline) fn one f1 { + b0(v0: [Field; 2]): + inc_rc v0 + v2 = array_get v0, index u32 0 -> Field + v4 = array_get v0, index u32 1 -> Field + v5 = add v2, v4 + dec_rc v0 + return v5 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + v2 = make_array [Field 2, Field 3] : [Field; 2] + return Field 5 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 0c6041029da..ddc3365b551 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -172,6 +172,7 @@ impl<'a> FunctionContext<'a> { /// Always returns a Value::Mutable wrapping the allocate instruction. pub(super) fn new_mutable_variable(&mut self, value_to_store: ValueId) -> Value { let element_type = self.builder.current_function.dfg.type_of_value(value_to_store); + self.builder.increment_array_reference_count(value_to_store); let alloc = self.builder.insert_allocate(element_type); self.builder.insert_store(alloc, value_to_store); let typ = self.builder.type_of_value(value_to_store); @@ -735,7 +736,6 @@ impl<'a> FunctionContext<'a> { // Reference counting in brillig relies on us incrementing reference // counts when arrays/slices are constructed or indexed. // Thus, if we dereference an lvalue which happens to be array/slice we should increment its reference counter. - self.builder.increment_array_reference_count(reference); self.builder.insert_load(reference, element_type).into() }) } @@ -916,7 +916,10 @@ impl<'a> FunctionContext<'a> { let parameters = self.builder.current_function.dfg.block_parameters(entry).to_vec(); for parameter in parameters { - self.builder.increment_array_reference_count(parameter); + // Avoid reference counts for immutable arrays that aren't behind references. + if self.builder.current_function.dfg.value_is_reference(parameter) { + self.builder.increment_array_reference_count(parameter); + } } entry @@ -933,7 +936,9 @@ impl<'a> FunctionContext<'a> { dropped_parameters.retain(|parameter| !terminator_args.contains(parameter)); for parameter in dropped_parameters { - self.builder.decrement_array_reference_count(parameter); + if self.builder.current_function.dfg.value_is_reference(parameter) { + self.builder.decrement_array_reference_count(parameter); + } } } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index c50f0a7f45c..d28236bd360 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -665,12 +665,11 @@ impl<'a> FunctionContext<'a> { values = values.map(|value| { let value = value.eval(self); - // Make sure to increment array reference counts on each let binding - self.builder.increment_array_reference_count(value); - Tree::Leaf(if let_expr.mutable { self.new_mutable_variable(value) } else { + // `new_mutable_variable` already increments rcs internally + self.builder.increment_array_reference_count(value); value::Value::Normal(value) }) }); diff --git a/test_programs/execution_success/brillig_unitialised_arrays/Nargo.toml b/test_programs/execution_success/brillig_uninitialized_arrays/Nargo.toml similarity index 58% rename from test_programs/execution_success/brillig_unitialised_arrays/Nargo.toml rename to test_programs/execution_success/brillig_uninitialized_arrays/Nargo.toml index f23ecc787d0..68bcf9929cc 100644 --- a/test_programs/execution_success/brillig_unitialised_arrays/Nargo.toml +++ b/test_programs/execution_success/brillig_uninitialized_arrays/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "brillig_unitialised_arrays" +name = "brillig_uninitialized_arrays" type = "bin" authors = [""] diff --git a/test_programs/execution_success/brillig_unitialised_arrays/Prover.toml b/test_programs/execution_success/brillig_uninitialized_arrays/Prover.toml similarity index 100% rename from test_programs/execution_success/brillig_unitialised_arrays/Prover.toml rename to test_programs/execution_success/brillig_uninitialized_arrays/Prover.toml diff --git a/test_programs/execution_success/brillig_unitialised_arrays/src/main.nr b/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr similarity index 100% rename from test_programs/execution_success/brillig_unitialised_arrays/src/main.nr rename to test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr diff --git a/tooling/nargo_toml/src/errors.rs b/tooling/nargo_toml/src/errors.rs index 1ee8e90c8e5..7e1003d04f7 100644 --- a/tooling/nargo_toml/src/errors.rs +++ b/tooling/nargo_toml/src/errors.rs @@ -80,6 +80,8 @@ pub enum ManifestError { #[allow(clippy::enum_variant_names)] #[derive(Error, Debug, PartialEq, Eq, Clone)] pub enum SemverError { + #[error("Invalid value for `compiler_version` in package {package_name}. Requirements may only refer to full releases")] + InvalidCompilerVersionRequirement { package_name: CrateName, required_compiler_version: String }, #[error("Incompatible compiler version in package {package_name}. Required compiler version is {required_compiler_version} but the compiler version is {compiler_version_found}.\n Update the compiler_version field in Nargo.toml to >={required_compiler_version} or compile this project with version {required_compiler_version}")] IncompatibleVersion { package_name: CrateName, diff --git a/tooling/nargo_toml/src/semver.rs b/tooling/nargo_toml/src/semver.rs index 253ac82aa34..ececa1b30dd 100644 --- a/tooling/nargo_toml/src/semver.rs +++ b/tooling/nargo_toml/src/semver.rs @@ -3,11 +3,14 @@ use nargo::{ package::{Dependency, Package}, workspace::Workspace, }; -use semver::{Error, Version, VersionReq}; +use noirc_driver::CrateName; +use semver::{Error, Prerelease, Version, VersionReq}; // Parse a semver compatible version string pub(crate) fn parse_semver_compatible_version(version: &str) -> Result { - Version::parse(version) + let mut version = Version::parse(version)?; + version.pre = Prerelease::EMPTY; + Ok(version) } // Check that all of the packages in the workspace are compatible with the current compiler version @@ -25,10 +28,7 @@ pub(crate) fn semver_check_workspace( } // Check that a package and all of its dependencies are compatible with the current compiler version -pub(crate) fn semver_check_package( - package: &Package, - compiler_version: &Version, -) -> Result<(), SemverError> { +fn semver_check_package(package: &Package, compiler_version: &Version) -> Result<(), SemverError> { // Check that this package's compiler version requirements are satisfied if let Some(version) = &package.compiler_required_version { let version_req = match VersionReq::parse(version) { @@ -40,6 +40,9 @@ pub(crate) fn semver_check_package( }) } }; + + validate_compiler_version_requirement(&package.name, &version_req)?; + if !version_req.matches(compiler_version) { return Err(SemverError::IncompatibleVersion { package_name: package.name.clone(), @@ -61,6 +64,20 @@ pub(crate) fn semver_check_package( Ok(()) } +fn validate_compiler_version_requirement( + package_name: &CrateName, + required_compiler_version: &VersionReq, +) -> Result<(), SemverError> { + if required_compiler_version.comparators.iter().any(|comparator| !comparator.pre.is_empty()) { + return Err(SemverError::InvalidCompilerVersionRequirement { + package_name: package_name.clone(), + required_compiler_version: required_compiler_version.to_string(), + }); + } + + Ok(()) +} + // Strip the build meta data from the version string since it is ignored by semver. fn strip_build_meta_data(version: &Version) -> String { let version_string = version.to_string(); @@ -191,6 +208,26 @@ mod tests { }; } + #[test] + fn test_semver_prerelease() { + let compiler_version = parse_semver_compatible_version("1.0.0-beta.0").unwrap(); + + let package = Package { + compiler_required_version: Some(">=0.1.0".to_string()), + root_dir: PathBuf::new(), + package_type: PackageType::Library, + entry_path: PathBuf::new(), + name: CrateName::from_str("test").unwrap(), + dependencies: BTreeMap::new(), + version: Some("1.0".to_string()), + expression_width: None, + }; + + if let Err(err) = semver_check_package(&package, &compiler_version) { + panic!("{err}"); + }; + } + #[test] fn test_semver_build_data() { let compiler_version = Version::parse("0.1.0+this-is-ignored-by-semver").unwrap();