From 7f9525dfdb4036befbd21b0d2546b83884593aa4 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 22 Jan 2025 15:25:05 -0500 Subject: [PATCH] feat(brillig): Set global memory size at program compile time (#7151) --- .../src/brillig/brillig_gen.rs | 1 + .../brillig/brillig_gen/brillig_globals.rs | 6 ++-- .../noirc_evaluator/src/brillig/brillig_ir.rs | 11 +++++++ .../src/brillig/brillig_ir/entry_point.rs | 27 +++++++++------- .../src/brillig/brillig_ir/registers.rs | 31 ++++++++++++++----- compiler/noirc_evaluator/src/brillig/mod.rs | 4 ++- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 2 +- cspell.json | 1 + 8 files changed, 61 insertions(+), 22 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs index b51a3445a1b..a6117a8f2da 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs @@ -63,6 +63,7 @@ pub(crate) fn gen_brillig_for( FunctionContext::return_values(func), func.id(), true, + brillig.globals_memory_size, ); entry_point.name = func.name().to_string(); 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 99c8ee0fded..f3ce79dd017 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs @@ -10,7 +10,7 @@ pub(crate) fn convert_ssa_globals( enable_debug_trace: bool, globals: &Function, used_globals: &HashSet, -) -> (BrilligArtifact, HashMap) { +) -> (BrilligArtifact, HashMap, usize) { let mut brillig_context = BrilligContext::new_for_global_init(enable_debug_trace); // The global space does not have globals itself let empty_globals = HashMap::default(); @@ -32,8 +32,10 @@ pub(crate) fn convert_ssa_globals( brillig_block.compile_globals(&globals.dfg, used_globals); + let globals_size = brillig_block.brillig_context.global_space_size(); + brillig_context.return_instruction(); let artifact = brillig_context.artifact(); - (artifact, function_context.ssa_value_allocations) + (artifact, function_context.ssa_value_allocations, globals_size) } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 06f61948337..ad09f73e90f 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -95,6 +95,8 @@ pub(crate) struct BrilligContext { /// Whether this context can call procedures or not. /// This is used to prevent a procedure from calling another procedure. can_call_procedures: bool, + + globals_memory_size: Option, } /// Regular brillig context to codegen user defined functions @@ -108,6 +110,7 @@ impl BrilligContext { next_section: 1, debug_show: DebugShow::new(enable_debug_trace), can_call_procedures: true, + globals_memory_size: None, } } } @@ -211,6 +214,7 @@ impl BrilligContext { next_section: 1, debug_show: DebugShow::new(enable_debug_trace), can_call_procedures: false, + globals_memory_size: None, } } } @@ -226,8 +230,14 @@ impl BrilligContext { next_section: 1, debug_show: DebugShow::new(enable_debug_trace), can_call_procedures: false, + globals_memory_size: None, } } + + pub(crate) fn global_space_size(&self) -> usize { + // `GlobalSpace::start()` is inclusive so we must add one to get the accurate total global memory size + (self.registers.max_memory_address() + 1) - GlobalSpace::start() + } } impl BrilligContext { @@ -321,6 +331,7 @@ pub(crate) mod tests { returns, FunctionId::test_new(0), false, + 0, ); entry_point_artifact.link_with(&artifact); while let Some(unresolved_fn_label) = entry_point_artifact.first_unresolved_function_call() diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index b84a15db4ad..030ed7133e8 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -15,7 +15,6 @@ use acvm::acir::{ pub(crate) const MAX_STACK_SIZE: usize = 16 * MAX_STACK_FRAME_SIZE; pub(crate) const MAX_STACK_FRAME_SIZE: usize = 2048; pub(crate) const MAX_SCRATCH_SPACE: usize = 64; -pub(crate) const MAX_GLOBAL_SPACE: usize = 16384; impl BrilligContext { /// Creates an entry point artifact that will jump to the function label provided. @@ -24,9 +23,12 @@ impl BrilligContext { return_parameters: Vec, target_function: FunctionId, globals_init: bool, + globals_memory_size: usize, ) -> BrilligArtifact { let mut context = BrilligContext::new(false); + context.globals_memory_size = Some(globals_memory_size); + context.codegen_entry_point(&arguments, &return_parameters); if globals_init { @@ -39,12 +41,15 @@ impl BrilligContext { context.artifact() } - fn calldata_start_offset() -> usize { - ReservedRegisters::len() + MAX_STACK_SIZE + MAX_SCRATCH_SPACE + MAX_GLOBAL_SPACE + fn calldata_start_offset(&self) -> usize { + ReservedRegisters::len() + + MAX_STACK_SIZE + + MAX_SCRATCH_SPACE + + self.globals_memory_size.expect("The memory size of globals should be set") } - fn return_data_start_offset(calldata_size: usize) -> usize { - Self::calldata_start_offset() + calldata_size + fn return_data_start_offset(&self, calldata_size: usize) -> usize { + self.calldata_start_offset() + calldata_size } /// Adds the instructions needed to handle entry point parameters @@ -70,7 +75,7 @@ impl BrilligContext { // Set initial value of free memory pointer: calldata_start_offset + calldata_size + return_data_size self.const_instruction( SingleAddrVariable::new_usize(ReservedRegisters::free_memory_pointer()), - (Self::calldata_start_offset() + calldata_size + return_data_size).into(), + (self.calldata_start_offset() + calldata_size + return_data_size).into(), ); // Set initial value of stack pointer: ReservedRegisters.len() @@ -82,7 +87,7 @@ impl BrilligContext { // Copy calldata self.copy_and_cast_calldata(arguments); - let mut current_calldata_pointer = Self::calldata_start_offset(); + let mut current_calldata_pointer = self.calldata_start_offset(); // Initialize the variables with the calldata for (argument_variable, argument) in argument_variables.iter_mut().zip(arguments) { @@ -158,7 +163,7 @@ impl BrilligContext { fn copy_and_cast_calldata(&mut self, arguments: &[BrilligParameter]) { let calldata_size = Self::flattened_tuple_size(arguments); self.calldata_copy_instruction( - MemoryAddress::direct(Self::calldata_start_offset()), + MemoryAddress::direct(self.calldata_start_offset()), calldata_size, 0, ); @@ -178,11 +183,11 @@ impl BrilligContext { if bit_size < F::max_num_bits() { self.cast_instruction( SingleAddrVariable::new( - MemoryAddress::direct(Self::calldata_start_offset() + i), + MemoryAddress::direct(self.calldata_start_offset() + i), bit_size, ), SingleAddrVariable::new_field(MemoryAddress::direct( - Self::calldata_start_offset() + i, + self.calldata_start_offset() + i, )), ); } @@ -336,7 +341,7 @@ impl BrilligContext { let return_data_size = Self::flattened_tuple_size(return_parameters); // Return data has a reserved space after calldata - let return_data_offset = Self::return_data_start_offset(calldata_size); + let return_data_offset = self.return_data_start_offset(calldata_size); let mut return_data_index = return_data_offset; for (return_param, returned_variable) in return_parameters.iter().zip(&returned_variables) { diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs index b83c03b297a..88b8a598b10 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs @@ -7,7 +7,7 @@ use crate::brillig::brillig_ir::entry_point::MAX_STACK_SIZE; use super::{ brillig_variable::SingleAddrVariable, - entry_point::{MAX_GLOBAL_SPACE, MAX_SCRATCH_SPACE, MAX_STACK_FRAME_SIZE}, + entry_point::{MAX_SCRATCH_SPACE, MAX_STACK_FRAME_SIZE}, BrilligContext, ReservedRegisters, }; @@ -152,16 +152,32 @@ impl RegisterAllocator for ScratchSpace { /// and is read-only. pub(crate) struct GlobalSpace { storage: DeallocationListAllocator, + max_memory_address: usize, } impl GlobalSpace { - pub(crate) fn new() -> Self { - Self { storage: DeallocationListAllocator::new(Self::start()) } + pub(super) fn new() -> Self { + Self { + storage: DeallocationListAllocator::new(Self::start()), + max_memory_address: Self::start(), + } } fn is_within_bounds(register: MemoryAddress) -> bool { let index = register.unwrap_direct(); - index >= Self::start() && index < Self::end() + index >= Self::start() + } + + fn update_max_address(&mut self, register: MemoryAddress) { + let index = register.unwrap_direct(); + assert!(index >= Self::start(), "Global space malformed"); + if index > self.max_memory_address { + self.max_memory_address = index; + } + } + + pub(super) fn max_memory_address(&self) -> usize { + self.max_memory_address } } @@ -171,12 +187,12 @@ impl RegisterAllocator for GlobalSpace { } fn end() -> usize { - Self::start() + MAX_GLOBAL_SPACE + unreachable!("The global space is set by the program"); } fn allocate_register(&mut self) -> MemoryAddress { let allocated = MemoryAddress::direct(self.storage.allocate_register()); - assert!(Self::is_within_bounds(allocated), "Global space too deep"); + self.update_max_address(allocated); allocated } @@ -185,7 +201,7 @@ impl RegisterAllocator for GlobalSpace { } fn ensure_register_is_allocated(&mut self, register: MemoryAddress) { - assert!(Self::is_within_bounds(register), "Register out of global space bounds"); + self.update_max_address(register); self.storage.ensure_register_is_allocated(register.unwrap_direct()); } @@ -199,6 +215,7 @@ impl RegisterAllocator for GlobalSpace { Self::start(), vecmap(preallocated_registers, |r| r.unwrap_direct()), ), + max_memory_address: Self::start(), } } diff --git a/compiler/noirc_evaluator/src/brillig/mod.rs b/compiler/noirc_evaluator/src/brillig/mod.rs index 3d96a855aa0..8a803c72b67 100644 --- a/compiler/noirc_evaluator/src/brillig/mod.rs +++ b/compiler/noirc_evaluator/src/brillig/mod.rs @@ -32,6 +32,7 @@ pub struct Brillig { /// Maps SSA function labels to their brillig artifact ssa_function_to_brillig: HashMap>, globals: BrilligArtifact, + globals_memory_size: usize, } impl Brillig { @@ -84,9 +85,10 @@ impl Ssa { let mut brillig = Brillig::default(); - let (artifact, brillig_globals) = + let (artifact, brillig_globals, globals_size) = convert_ssa_globals(enable_debug_trace, &self.globals, &self.used_global_values); brillig.globals = artifact; + brillig.globals_memory_size = globals_size; for brillig_function_id in brillig_reachable_function_ids { let func = &self.functions[&brillig_function_id]; diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index a6e5c96d638..d0f77d3cee1 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -82,7 +82,7 @@ impl Ssa { // DIE is run at the end of our SSA optimizations, so we mark all globals as in use here. let used_globals = &self.globals.dfg.values_iter().map(|(id, _)| id).collect(); - let (_, brillig_globals) = + let (_, brillig_globals, _) = convert_ssa_globals(false, &self.globals, used_globals); global_cache = Some(brillig_globals); } diff --git a/cspell.json b/cspell.json index 25a0cc91f52..1174a56dd33 100644 --- a/cspell.json +++ b/cspell.json @@ -32,6 +32,7 @@ "boilerplates", "bridgekeeper", "brillig", + "brillig_", "bunx", "bytecount", "cachix",