diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 6789cbafb76..46c9681ea8d 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -2896,7 +2896,7 @@ mod test { use std::collections::BTreeMap; use crate::{ - acir::{BrilligStdlibFunc, Function}, + acir::BrilligStdlibFunc, brillig::Brillig, ssa::{ function_builder::FunctionBuilder, @@ -3328,8 +3328,7 @@ mod test { build_basic_foo_with_return(&mut builder, foo_id, true, InlineType::default()); build_basic_foo_with_return(&mut builder, bar_id, true, InlineType::default()); - let mut ssa = builder.finish(); - ssa.globals = Function::new("globals".to_owned(), ssa.main_id); + let ssa = builder.finish(); let brillig = ssa.to_brillig(false); let (acir_functions, brillig_functions, _, _) = ssa @@ -3467,8 +3466,7 @@ mod test { build_basic_foo_with_return(&mut builder, foo_id, true, InlineType::default()); - let mut ssa = builder.finish(); - ssa.globals = Function::new("globals".to_owned(), ssa.main_id); + let ssa = builder.finish(); // We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass. let brillig = ssa.to_brillig(false); println!("{}", ssa); @@ -3557,8 +3555,7 @@ mod test { // Build an ACIR function which has the same logic as the Brillig function above build_basic_foo_with_return(&mut builder, bar_id, false, InlineType::Fold); - let mut ssa = builder.finish(); - ssa.globals = Function::new("globals".to_owned(), ssa.main_id); + let ssa = builder.finish(); // We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass. let brillig = ssa.to_brillig(false); println!("{}", ssa); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 698d4cd05be..ec288a5cfd4 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -142,7 +142,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> { /// Making the assumption that the block ID passed in belongs to this /// function. fn create_block_label_for_current_function(&self, block_id: BasicBlockId) -> Label { - Self::create_block_label(self.function_context.function_id, block_id) + Self::create_block_label(self.function_context.function_id(), block_id) } /// Creates a unique label for a block using the function Id and the block ID. /// diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs index 3dea7b3e7f5..6e406e2b3cb 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs @@ -17,8 +17,11 @@ use fxhash::FxHashMap as HashMap; use super::{constant_allocation::ConstantAllocation, variable_liveness::VariableLiveness}; +#[derive(Default)] pub(crate) struct FunctionContext { - pub(crate) function_id: FunctionId, + /// A `FunctionContext` is necessary for using a Brillig block's code gen, but sometimes + /// such as with globals, we are not within a function and do not have a function id. + function_id: Option, /// Map from SSA values its allocation. Since values can be only defined once in SSA form, we insert them here on when we allocate them at their definition. pub(crate) ssa_value_allocations: HashMap, /// The block ids of the function in reverse post order. @@ -42,7 +45,7 @@ impl FunctionContext { let liveness = VariableLiveness::from_function(function, &constants); Self { - function_id: id, + function_id: Some(id), ssa_value_allocations: HashMap::default(), blocks: reverse_post_order, liveness, @@ -50,6 +53,10 @@ impl FunctionContext { } } + pub(crate) fn function_id(&self) -> FunctionId { + self.function_id.expect("ICE: function_id should already be set") + } + pub(crate) fn ssa_type_to_parameter(typ: &Type) -> BrilligParameter { match typ { Type::Numeric(_) | Type::Reference(_) => { 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 f3ce79dd017..9f9d271283d 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs @@ -1,14 +1,15 @@ use acvm::FieldElement; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; -use super::{ - BrilligArtifact, BrilligBlock, BrilligVariable, Function, FunctionContext, Label, ValueId, +use super::{BrilligArtifact, BrilligBlock, BrilligVariable, FunctionContext, Label, ValueId}; +use crate::{ + brillig::{brillig_ir::BrilligContext, DataFlowGraph}, + ssa::ir::dfg::GlobalsGraph, }; -use crate::brillig::{brillig_ir::BrilligContext, DataFlowGraph}; pub(crate) fn convert_ssa_globals( enable_debug_trace: bool, - globals: &Function, + globals: GlobalsGraph, used_globals: &HashSet, ) -> (BrilligArtifact, HashMap, usize) { let mut brillig_context = BrilligContext::new_for_global_init(enable_debug_trace); @@ -16,7 +17,7 @@ pub(crate) fn convert_ssa_globals( let empty_globals = HashMap::default(); // We can use any ID here as this context is only going to be used for globals which does not differentiate // by functions and blocks. The only Label that should be used in the globals context is `Label::globals_init()` - let mut function_context = FunctionContext::new(globals); + let mut function_context = FunctionContext::default(); brillig_context.enter_context(Label::globals_init()); let block_id = DataFlowGraph::default().make_block(); @@ -30,7 +31,8 @@ pub(crate) fn convert_ssa_globals( building_globals: true, }; - brillig_block.compile_globals(&globals.dfg, used_globals); + let globals_dfg = DataFlowGraph::from(globals); + brillig_block.compile_globals(&globals_dfg, used_globals); let globals_size = brillig_block.brillig_context.global_space_size(); diff --git a/compiler/noirc_evaluator/src/brillig/mod.rs b/compiler/noirc_evaluator/src/brillig/mod.rs index 8a803c72b67..b74c519f61a 100644 --- a/compiler/noirc_evaluator/src/brillig/mod.rs +++ b/compiler/noirc_evaluator/src/brillig/mod.rs @@ -85,8 +85,15 @@ impl Ssa { let mut brillig = Brillig::default(); + if brillig_reachable_function_ids.is_empty() { + return brillig; + } + + // Globals are computed once at compile time and shared across all functions, + // thus we can just fetch globals from the main function. + let globals = (*self.functions[&self.main_id].dfg.globals).clone(); let (artifact, brillig_globals, globals_size) = - convert_ssa_globals(enable_debug_trace, &self.globals, &self.used_global_values); + convert_ssa_globals(enable_debug_trace, globals, &self.used_global_values); brillig.globals = artifact; brillig.globals_memory_size = globals_size; diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 1bbd407cf0f..6b6740c4d2e 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -109,6 +109,7 @@ pub(crate) struct DataFlowGraph { /// The GlobalsGraph contains the actual global data. /// Global data is expected to only be numeric constants or array constants (which are represented by Instruction::MakeArray). /// The global's data will shared across functions and should be accessible inside of a function's DataFlowGraph. +#[serde_as] #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub(crate) struct GlobalsGraph { /// Storage for all of the global values @@ -116,14 +117,20 @@ pub(crate) struct GlobalsGraph { /// All of the instructions in the global value space. /// These are expected to all be Instruction::MakeArray instructions: DenseMap, - + #[serde_as(as = "HashMap")] + results: HashMap>, #[serde(skip)] constants: HashMap<(FieldElement, NumericType), ValueId>, } impl GlobalsGraph { pub(crate) fn from_dfg(dfg: DataFlowGraph) -> Self { - Self { values: dfg.values, instructions: dfg.instructions, constants: dfg.constants } + Self { + values: dfg.values, + instructions: dfg.instructions, + results: dfg.results, + constants: dfg.constants, + } } /// Iterate over every Value in this DFG in no particular order, including unused Values @@ -132,6 +139,17 @@ impl GlobalsGraph { } } +impl From for DataFlowGraph { + fn from(value: GlobalsGraph) -> Self { + DataFlowGraph { + values: value.values, + instructions: value.instructions, + results: value.results, + ..Default::default() + } + } +} + impl DataFlowGraph { /// Runtime type of the function. pub(crate) fn runtime(&self) -> RuntimeType { diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 88bee0799a3..e9c465d264f 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -20,13 +20,16 @@ use super::{ impl Display for Ssa { fn fmt(&self, f: &mut Formatter<'_>) -> Result { - for (id, global_value) in self.globals.dfg.values_iter() { + let globals = (*self.functions[&self.main_id].dfg.globals).clone(); + let globals_dfg = DataFlowGraph::from(globals); + + for (id, global_value) in globals_dfg.values_iter() { match global_value { Value::NumericConstant { constant, typ } => { writeln!(f, "g{} = {typ} {constant}", id.to_u32())?; } Value::Instruction { instruction, .. } => { - display_instruction(&self.globals.dfg, *instruction, true, f)?; + display_instruction(&globals_dfg, *instruction, true, f)?; } Value::Global(_) => { panic!("Value::Global should only be in the function dfg"); @@ -35,7 +38,7 @@ impl Display for Ssa { }; } - if self.globals.dfg.values_iter().next().is_some() { + if globals_dfg.values_iter().next().is_some() { writeln!(f)?; } diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs index 3b5537aceb4..e80769756e8 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -36,11 +36,12 @@ impl Ssa { .flat_map(|(_, func)| func.dead_instruction_elimination(true, flattened)) .collect(); + let globals = &self.functions[&self.main_id].dfg.globals; // Check which globals are used across all functions - for (id, value) in self.globals.dfg.values_iter().rev() { + for (id, value) in globals.values_iter().rev() { if used_global_values.contains(&id) { if let Value::Instruction { instruction, .. } = &value { - let instruction = &self.globals.dfg[*instruction]; + let instruction = &globals[*instruction]; instruction.for_each_value(|value_id| { used_global_values.insert(value_id); }); diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index be40957fc2e..6d677758e04 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -13,7 +13,7 @@ use crate::ssa::{ ir::{ basic_block::BasicBlockId, call_stack::CallStackId, - dfg::InsertInstructionResult, + dfg::{GlobalsGraph, InsertInstructionResult}, function::{Function, FunctionId, RuntimeType}, instruction::{Instruction, InstructionId, TerminatorInstruction}, value::{Value, ValueId}, @@ -150,7 +150,7 @@ struct PerFunctionContext<'function> { /// True if we're currently working on the entry point function. inlining_entry: bool, - globals: &'function Function, + globals: &'function GlobalsGraph, } /// Utility function to find out the direct calls of a function. @@ -562,8 +562,8 @@ impl InlineContext { ) -> Function { let entry_point = &ssa.functions[&self.entry_point]; - let mut context = - PerFunctionContext::new(&mut self, entry_point, entry_point, &ssa.globals); + let globals = &entry_point.dfg.globals; + let mut context = PerFunctionContext::new(&mut self, entry_point, entry_point, globals); context.inlining_entry = true; for (_, value) in entry_point.dfg.globals.values_iter() { @@ -615,7 +615,8 @@ impl InlineContext { } let entry_point = &ssa.functions[&self.entry_point]; - let mut context = PerFunctionContext::new(self, entry_point, source_function, &ssa.globals); + let globals = &source_function.dfg.globals; + let mut context = PerFunctionContext::new(self, entry_point, source_function, globals); let parameters = source_function.parameters(); assert_eq!(parameters.len(), arguments.len()); @@ -639,7 +640,7 @@ impl<'function> PerFunctionContext<'function> { context: &'function mut InlineContext, entry_function: &'function Function, source_function: &'function Function, - globals: &'function Function, + globals: &'function GlobalsGraph, ) -> Self { Self { context, @@ -667,8 +668,7 @@ impl<'function> PerFunctionContext<'function> { value @ Value::Instruction { instruction, .. } => { if self.source_function.dfg.is_global(id) { if self.context.builder.current_function.dfg.runtime().is_acir() { - let Instruction::MakeArray { elements, typ } = - &self.globals.dfg[*instruction] + let Instruction::MakeArray { elements, typ } = &self.globals[*instruction] else { panic!("Only expect Instruction::MakeArray for a global"); }; diff --git a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs index ed57118b840..b248f6734a9 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -25,7 +25,7 @@ impl Ssa { let mut context = Context::default(); context.populate_functions(&self.functions); for function in self.functions.values_mut() { - context.normalize_ids(function, &self.globals); + context.normalize_ids(function); } self.functions = context.functions.into_btree(); } @@ -65,14 +65,14 @@ impl Context { } } - fn normalize_ids(&mut self, old_function: &mut Function, globals: &Function) { + fn normalize_ids(&mut self, old_function: &mut Function) { self.new_ids.blocks.clear(); self.new_ids.values.clear(); let new_function_id = self.new_ids.function_ids[&old_function.id()]; let new_function = &mut self.functions[new_function_id]; - for (_, value) in globals.dfg.values_iter() { + for (_, value) in old_function.dfg.globals.values_iter() { new_function.dfg.make_global(value.get_type().into_owned()); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index d0f77d3cee1..eb0bbd8c532 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -79,11 +79,11 @@ impl Ssa { if has_unrolled && is_brillig { if let Some(max_incr_pct) = max_bytecode_increase_percent { if global_cache.is_none() { + let globals = (*function.dfg.globals).clone(); // 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 used_globals = &globals.values_iter().map(|(id, _)| id).collect(); let (_, brillig_globals, _) = - convert_ssa_globals(false, &self.globals, used_globals); + convert_ssa_globals(false, globals, used_globals); global_cache = Some(brillig_globals); } let brillig_globals = global_cache.as_ref().unwrap(); diff --git a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index 456f2f85af1..37d2cd720f9 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -448,7 +448,6 @@ impl Translator { fn finish(self) -> Ssa { let mut ssa = self.builder.finish(); - ssa.globals = self.globals_function; // Normalize the IDs so we have a better chance of matching the SSA we parsed // after the step-by-step reconstruction done during translation. This assumes diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index fbb2b306bdf..4ee83adf4d8 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -48,9 +48,10 @@ pub(crate) fn generate_ssa(program: Program) -> Result { let is_return_data = matches!(program.return_visibility, Visibility::ReturnData); let return_location = program.return_location; - let context = SharedContext::new(program); + let mut context = SharedContext::new(program); - let globals = GlobalsGraph::from_dfg(context.globals_context.dfg.clone()); + let globals_dfg = std::mem::take(&mut context.globals_context.dfg); + let globals = GlobalsGraph::from_dfg(globals_dfg); let main_id = Program::main_id(); let main = context.program.main(); @@ -124,9 +125,7 @@ pub(crate) fn generate_ssa(program: Program) -> Result { function_context.codegen_function_body(&function.body)?; } - let mut ssa = function_context.builder.finish(); - ssa.globals = context.globals_context; - Ok(ssa) + Ok(function_context.builder.finish()) } impl<'a> FunctionContext<'a> { diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index 7cd5c5c3990..04986bd8db1 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -20,7 +20,6 @@ use super::ValueId; pub(crate) struct Ssa { #[serde_as(as = "Vec<(_, _)>")] pub(crate) functions: BTreeMap, - pub(crate) globals: Function, pub(crate) used_global_values: HashSet, pub(crate) main_id: FunctionId, #[serde(skip)] @@ -58,9 +57,6 @@ impl Ssa { next_id: AtomicCounter::starting_after(max_id), entry_point_to_generated_index: BTreeMap::new(), error_selector_to_type: error_types, - // These fields should be set afterwards as globals are generated - // outside of the FunctionBuilder, which is where the `Ssa` is instantiated. - globals: Function::new_for_globals(), // This field is set only after running DIE and is utilized // for optimizing implementation of globals post-SSA. used_global_values: HashSet::default(),