chore(refactor): Remove globals field on Ssa object and use only the …

…shared globals graph (#7156)

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);

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.
     ///

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<FunctionId>,
     /// 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<ValueId, BrilligVariable>,
     /// The block ids of the function in reverse post order.
@@ -42,14 +45,18 @@ 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,
             constant_allocation: constants,
         }
     }
 
+    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(_) => {

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs

@@ -1,22 +1,23 @@
 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<ValueId>,
 ) -> (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();
     // 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();
 

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;
 

compiler/noirc_evaluator/src/ssa/ir/dfg.rs

@@ -109,21 +109,28 @@ 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
     values: DenseMap<Value>,
     /// All of the instructions in the global value space.
     /// These are expected to all be Instruction::MakeArray
     instructions: DenseMap<Instruction>,
-
+    #[serde_as(as = "HashMap<DisplayFromStr, _>")]
+    results: HashMap<InstructionId, smallvec::SmallVec<[ValueId; 1]>>,
     #[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<GlobalsGraph> 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 {

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)?;
         }
 

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);
                     });

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");
                         };

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

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();

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

compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs

@@ -48,9 +48,10 @@ pub(crate) fn generate_ssa(program: Program) -> Result<Ssa, RuntimeError> {
     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<Ssa, RuntimeError> {
         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> {

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<FunctionId, Function>,
-    pub(crate) globals: Function,
     pub(crate) used_global_values: HashSet<ValueId>,
     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(),