Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(brillig): Set global memory size at program compile time #7151

Merged
merged 2 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub(crate) fn convert_ssa_globals(
enable_debug_trace: bool,
globals: &Function,
used_globals: &HashSet<ValueId>,
) -> (BrilligArtifact<FieldElement>, HashMap<ValueId, BrilligVariable>) {
) -> (BrilligArtifact<FieldElement>, HashMap<ValueId, BrilligVariable>, 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();
Expand All @@ -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)
}
11 changes: 11 additions & 0 deletions compiler/noirc_evaluator/src/brillig/brillig_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ pub(crate) struct BrilligContext<F, Registers> {
/// 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<usize>,
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
}

/// Regular brillig context to codegen user defined functions
Expand All @@ -108,6 +110,7 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
next_section: 1,
debug_show: DebugShow::new(enable_debug_trace),
can_call_procedures: true,
globals_memory_size: None,
}
}
}
Expand Down Expand Up @@ -211,6 +214,7 @@ impl<F: AcirField + DebugToString> BrilligContext<F, ScratchSpace> {
next_section: 1,
debug_show: DebugShow::new(enable_debug_trace),
can_call_procedures: false,
globals_memory_size: None,
}
}
}
Expand All @@ -226,8 +230,14 @@ impl<F: AcirField + DebugToString> BrilligContext<F, GlobalSpace> {
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<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<F, Registers> {
Expand Down Expand Up @@ -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()
Expand Down
27 changes: 16 additions & 11 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
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<F: AcirField + DebugToString> BrilligContext<F, Stack> {
/// Creates an entry point artifact that will jump to the function label provided.
Expand All @@ -24,9 +23,12 @@
return_parameters: Vec<BrilligParameter>,
target_function: FunctionId,
globals_init: bool,
globals_memory_size: usize,
) -> BrilligArtifact<F> {
let mut context = BrilligContext::new(false);

context.globals_memory_size = Some(globals_memory_size);

context.codegen_entry_point(&arguments, &return_parameters);

if globals_init {
Expand All @@ -39,12 +41,15 @@
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
Expand All @@ -70,7 +75,7 @@
// 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()
Expand All @@ -82,7 +87,7 @@
// 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) {
Expand Down Expand Up @@ -158,7 +163,7 @@
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,
);
Expand All @@ -174,15 +179,15 @@
}

for (i, bit_size) in arguments.iter().flat_map(flat_bit_sizes).enumerate() {
// Calldatacopy tags everything with field type, so when downcast when necessary

Check warning on line 182 in compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Calldatacopy)
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,
)),
);
}
Expand Down Expand Up @@ -336,7 +341,7 @@
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) {
Expand Down
31 changes: 24 additions & 7 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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
}
}

Expand All @@ -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
}

Expand All @@ -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());
}

Expand All @@ -199,6 +215,7 @@ impl RegisterAllocator for GlobalSpace {
Self::start(),
vecmap(preallocated_registers, |r| r.unwrap_direct()),
),
max_memory_address: Self::start(),
}
}

Expand Down
4 changes: 3 additions & 1 deletion compiler/noirc_evaluator/src/brillig/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub struct Brillig {
/// Maps SSA function labels to their brillig artifact
ssa_function_to_brillig: HashMap<FunctionId, BrilligArtifact<FieldElement>>,
globals: BrilligArtifact<FieldElement>,
globals_memory_size: usize,
}

impl Brillig {
Expand Down Expand Up @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/unrolling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
// 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);
}
Expand Down Expand Up @@ -1061,7 +1061,7 @@
use super::{is_new_size_ok, BoilerplateStats, Loops};

/// Tries to unroll all loops in each SSA function once, calling the `Function` directly,
/// bypassing the iterative loop done by the SSA which does further optimisations.

Check warning on line 1064 in compiler/noirc_evaluator/src/ssa/opt/unrolling.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (optimisations)
///
/// If any loop cannot be unrolled, it is left as-is or in a partially unrolled state.
fn try_unroll_loops(mut ssa: Ssa) -> (Ssa, Vec<RuntimeError>) {
Expand Down
1 change: 1 addition & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"boilerplates",
"bridgekeeper",
"brillig",
"brillig_",
"bunx",
"bytecount",
"cachix",
Expand Down
Loading