chore: don't compute used globals during DIE

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

@@ -324,11 +324,7 @@ mod tests {
         ";
 
         let ssa = Ssa::from_str(src).unwrap();
-        // Need to run DIE to generate the used globals map, which is necessary for Brillig globals generation.
-        let mut ssa = ssa.dead_instruction_elimination();
-
-        let used_globals_map = std::mem::take(&mut ssa.used_globals);
-        let brillig = ssa.to_brillig_with_globals(&BrilligOptions::default(), used_globals_map);
+        let brillig = ssa.to_brillig(&BrilligOptions::default());
 
         assert_eq!(
             brillig.globals.len(),
@@ -443,11 +439,8 @@ mod tests {
         let ssa = Ssa::from_str(src).unwrap();
         // Need to run SSA pass that sets up Brillig array gets
         let ssa = ssa.brillig_array_get_and_set();
-        // Need to run DIE to generate the used globals map, which is necessary for Brillig globals generation.
-        let mut ssa = ssa.dead_instruction_elimination();
 
-        let used_globals_map = std::mem::take(&mut ssa.used_globals);
-        let brillig = ssa.to_brillig_with_globals(&BrilligOptions::default(), used_globals_map);
+        let brillig = ssa.to_brillig(&BrilligOptions::default());
 
         assert_eq!(
             brillig.globals.len(),
@@ -527,8 +520,6 @@ mod tests {
         ";
 
         let ssa = Ssa::from_str(src).unwrap();
-        // Need to run DIE to generate the used globals map, which is necessary for Brillig globals generation.
-        let mut ssa = ssa.dead_instruction_elimination();
 
         // Show that the constants in each function have different SSA value IDs
         for (func_id, function) in &ssa.functions {
@@ -557,8 +548,7 @@ mod tests {
             }
         }
 
-        let used_globals_map = std::mem::take(&mut ssa.used_globals);
-        let brillig = ssa.to_brillig_with_globals(&BrilligOptions::default(), used_globals_map);
+        let brillig = ssa.to_brillig(&BrilligOptions::default());
 
         assert_eq!(brillig.globals.len(), 1, "Should have a single entry point");
         for (func_id, artifact) in brillig.globals {
@@ -617,11 +607,8 @@ mod tests {
         ";
 
         let ssa = Ssa::from_str(src).unwrap();
-        // Need to run DIE to generate the used globals map, which is necessary for Brillig globals generation.
-        let mut ssa = ssa.dead_instruction_elimination();
 
-        let used_globals_map = std::mem::take(&mut ssa.used_globals);
-        let brillig = ssa.to_brillig_with_globals(&BrilligOptions::default(), used_globals_map);
+        let brillig = ssa.to_brillig(&BrilligOptions::default());
 
         assert_eq!(
             brillig.globals.len(),

compiler/noirc_evaluator/src/brillig/mod.rs

@@ -23,7 +23,7 @@ use crate::ssa::{
     },
     ssa_gen::Ssa,
 };
-use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
+use fxhash::FxHashMap as HashMap;
 use std::{borrow::Cow, collections::BTreeSet};
 
 pub use self::brillig_ir::procedures::ProcedureId;
@@ -129,18 +129,11 @@ impl std::ops::Index<FunctionId> for Brillig {
 }
 
 impl Ssa {
+    /// Compile Brillig functions and ACIR functions reachable from them
     #[tracing::instrument(level = "trace", skip_all)]
     pub fn to_brillig(&self, options: &BrilligOptions) -> Brillig {
-        self.to_brillig_with_globals(options, HashMap::default())
-    }
+        let used_globals_map = self.used_globals_in_brillig_functions();
 
-    /// Compile Brillig functions and ACIR functions reachable from them
-    #[tracing::instrument(level = "trace", skip_all)]
-    pub(crate) fn to_brillig_with_globals(
-        &self,
-        options: &BrilligOptions,
-        used_globals_map: HashMap<FunctionId, HashSet<ValueId>>,
-    ) -> Brillig {
         // Collect all the function ids that are reachable from brillig
         // That means all the functions marked as brillig and ACIR functions called by them
         let brillig_reachable_function_ids = self

compiler/noirc_evaluator/src/ssa.rs

@@ -230,22 +230,14 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass> {
             .and_then(Ssa::remove_unreachable_functions),
         SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination"),
         SsaPass::new(Ssa::array_set_optimization, "Array Set Optimizations"),
-        // The Brillig globals pass expected that we have the used globals map set for each function.
-        // The used globals map is determined during DIE, so we should duplicate entry points before a DIE pass run.
         SsaPass::new(Ssa::brillig_entry_point_analysis, "Brillig Entry Point Analysis")
             // Remove any potentially unnecessary duplication from the Brillig entry point analysis.
             .and_then(Ssa::remove_unreachable_functions),
         SsaPass::new(Ssa::remove_truncate_after_range_check, "Removing Truncate after RangeCheck"),
         // This pass makes transformations specific to Brillig generation.
         // It must be the last pass to either alter or add new instructions before Brillig generation,
         // as other semantics in the compiler can potentially break (e.g. inserting instructions).
-        // We can safely place the pass before DIE as that pass only removes instructions.
-        // We also need DIE's tracking of used globals in case the array get transformations
-        // end up using an existing constant from the globals space.
-        // This pass might result in otherwise unused global constant becoming used,
-        // because the creation of shifted index constants can reuse their IDs.
         SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get and Set Optimizations"),
-        // Perform another DIE pass to update the used globals after offsetting Brillig indexes.
         SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination")
             // A function can be potentially unreachable post-DIE if all calls to that function were removed.
             .and_then(Ssa::remove_unreachable_functions),
@@ -291,8 +283,6 @@ pub fn minimal_passes() -> Vec<SsaPass<'static>> {
         // This can change which globals are used, because constant creation might result
         // in the (re)use of otherwise unused global values.
         SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get and Set Optimizations"),
-        // We need a DIE pass to populate `used_globals`, otherwise it will panic later.
-        SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination"),
     ]
 }
 
@@ -323,14 +313,13 @@ where
     let mut builder = builder.with_skip_passes(options.skip_passes.clone()).run_passes(primary)?;
     let passed = std::mem::take(&mut builder.passed);
     let files = builder.files;
-    let mut ssa = builder.finish();
+    let ssa = builder.finish();
 
     let mut ssa_level_warnings = vec![];
     drop(ssa_gen_span_guard);
 
-    let used_globals_map = std::mem::take(&mut ssa.used_globals);
     let brillig = time("SSA to Brillig", options.print_codegen_timings, || {
-        ssa.to_brillig_with_globals(&options.brillig_options, used_globals_map)
+        ssa.to_brillig(&options.brillig_options)
     });
 
     let ssa_gen_span = span!(Level::TRACE, "ssa_generation");

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs

@@ -1537,9 +1537,7 @@ mod test {
         }
         ";
         let ssa = Ssa::from_str(src).unwrap();
-        let mut ssa = ssa.dead_instruction_elimination();
-        let used_globals_map = std::mem::take(&mut ssa.used_globals);
-        let brillig = ssa.to_brillig_with_globals(&BrilligOptions::default(), used_globals_map);
+        let brillig = ssa.to_brillig(&BrilligOptions::default());
 
         let ssa = ssa.fold_constants_with_brillig(&brillig);
         let ssa = ssa.remove_unreachable_functions();
@@ -1577,9 +1575,7 @@ mod test {
         }
         ";
         let ssa = Ssa::from_str(src).unwrap();
-        let mut ssa = ssa.dead_instruction_elimination();
-        let used_globals_map = std::mem::take(&mut ssa.used_globals);
-        let brillig = ssa.to_brillig_with_globals(&BrilligOptions::default(), used_globals_map);
+        let brillig = ssa.to_brillig(&BrilligOptions::default());
 
         let ssa = ssa.fold_constants_with_brillig(&brillig);
         let ssa = ssa.remove_unreachable_functions();

compiler/noirc_evaluator/src/ssa/opt/die.rs

@@ -96,45 +96,22 @@ impl Ssa {
         flattened: bool,
         skip_brillig: bool,
     ) -> (Ssa, DIEResult) {
-        let mut result = self
+        let result = self
             .functions
             .par_iter_mut()
             .map(|(id, func)| {
-                let (used_globals, unused_params) =
-                    func.dead_instruction_elimination(flattened, skip_brillig);
+                let unused_params = func.dead_instruction_elimination(flattened, skip_brillig);
                 let mut result = DIEResult::default();
 
-                if func.runtime().is_brillig() {
-                    result.used_globals.insert(*id, used_globals);
-                }
                 result.unused_parameters.insert(*id, unused_params);
 
                 result
             })
             .reduce(DIEResult::default, |mut a, b| {
-                a.used_globals.extend(b.used_globals);
                 a.unused_parameters.extend(b.unused_parameters);
                 a
             });
 
-        let globals = &self.functions[&self.main_id].dfg.globals;
-        for used_global_values in result.used_globals.values_mut() {
-            // DIE only tracks used instruction results, however, globals include constants.
-            // Back track globals for internal values which may be in use.
-            for (id, value) in globals.values_iter().rev() {
-                if used_global_values.contains(&id) {
-                    if let Value::Instruction { instruction, .. } = &value {
-                        let instruction = &globals[*instruction];
-                        instruction.for_each_value(|value_id| {
-                            used_global_values.insert(value_id);
-                        });
-                    }
-                }
-            }
-        }
-
-        self.used_globals = std::mem::take(&mut result.used_globals);
-
         (self, result)
     }
 }
@@ -154,17 +131,16 @@ impl Function {
     /// of its instructions are needed elsewhere.
     ///
     /// # Returns
-    /// - The set of globals that were used in this function.
     ///   After processing all functions, the union of these sets enables determining the unused globals.
     /// - A mapping of (block id -> unused parameters) for the given function.
     ///   This can be used by follow-up passes to prune unused parameters from blocks.
     fn dead_instruction_elimination(
         &mut self,
         flattened: bool,
         skip_brillig: bool,
-    ) -> (HashSet<ValueId>, HashMap<BasicBlockId, Vec<ValueId>>) {
+    ) -> HashMap<BasicBlockId, Vec<ValueId>> {
         if skip_brillig && self.dfg.runtime().is_brillig() {
-            return (HashSet::default(), HashMap::default());
+            return HashMap::default();
         }
 
         let mut context = Context { flattened, ..Default::default() };
@@ -198,16 +174,12 @@ impl Function {
 
         context.remove_rc_instructions(&mut self.dfg);
 
-        (
-            context.used_values.into_iter().filter(|value| self.dfg.is_global(*value)).collect(),
-            unused_params_per_block,
-        )
+        unused_params_per_block
     }
 }
 
 #[derive(Default)]
 struct DIEResult {
-    used_globals: HashMap<FunctionId, HashSet<ValueId>>,
     unused_parameters: HashMap<FunctionId, HashMap<BasicBlockId, Vec<ValueId>>>,
 }
 /// Per function context for tracking unused values and which instructions to remove.

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

@@ -9,6 +9,7 @@ use serde_with::serde_as;
 use crate::ssa::ir::{
     function::{Function, FunctionId},
     map::AtomicCounter,
+    value::Value,
 };
 use noirc_frontend::hir_def::types::Type as HirType;
 
@@ -20,7 +21,6 @@ use super::ValueId;
 pub struct Ssa {
     #[serde_as(as = "Vec<(_, _)>")]
     pub functions: BTreeMap<FunctionId, Function>,
-    pub used_globals: HashMap<FunctionId, HashSet<ValueId>>,
     pub main_id: FunctionId,
     #[serde(skip)]
     pub next_id: AtomicCounter<Function>,
@@ -54,9 +54,6 @@ impl Ssa {
             next_id: AtomicCounter::starting_after(max_id),
             entry_point_to_generated_index: BTreeMap::new(),
             error_selector_to_type: error_types,
-            // This field is set only after running DIE and is utilized
-            // for optimizing implementation of globals post-SSA.
-            used_globals: HashMap::default(),
         }
     }
 
@@ -101,6 +98,77 @@ impl Ssa {
     pub(crate) fn is_entry_point(&self, function: FunctionId) -> bool {
         function == self.main_id || self.functions[&function].runtime().is_entry_point()
     }
+
+    pub(crate) fn used_globals_in_brillig_functions(
+        &self,
+    ) -> HashMap<FunctionId, HashSet<ValueId>> {
+        fn add_value_to_globals_if_global(
+            function: &Function,
+            value_id: ValueId,
+            used_globals: &mut HashSet<ValueId>,
+        ) {
+            if !function.dfg.is_global(value_id) {
+                return;
+            }
+
+            if !used_globals.insert(value_id) {
+                return;
+            }
+
+            // If we found a new global, its value could be an instruction that points to other globals.
+            let globals = &function.dfg.globals;
+            if let Value::Instruction { instruction, .. } = globals[value_id] {
+                let instruction = &globals[instruction];
+                instruction.for_each_value(|value_id| {
+                    add_value_to_globals_if_global(function, value_id, used_globals);
+                });
+            }
+        }
+
+        let mut used_globals = HashMap::default();
+
+        for (function_id, function) in &self.functions {
+            if !function.runtime().is_brillig() {
+                continue;
+            }
+
+            let mut used_globals_in_function = HashSet::default();
+
+            for call_data in &function.dfg.data_bus.call_data {
+                add_value_to_globals_if_global(
+                    function,
+                    call_data.array_id,
+                    &mut used_globals_in_function,
+                );
+            }
+
+            for block_id in function.reachable_blocks() {
+                let block = &function.dfg[block_id];
+                for instruction_id in block.instructions() {
+                    let instruction = &function.dfg[*instruction_id];
+                    instruction.for_each_value(|value_id| {
+                        add_value_to_globals_if_global(
+                            function,
+                            value_id,
+                            &mut used_globals_in_function,
+                        );
+                    });
+                }
+
+                block.unwrap_terminator().for_each_value(|value_id| {
+                    add_value_to_globals_if_global(
+                        function,
+                        value_id,
+                        &mut used_globals_in_function,
+                    );
+                });
+            }
+
+            used_globals.insert(*function_id, used_globals_in_function);
+        }
+
+        used_globals
+    }
 }
 
 #[cfg(test)]