Merge a792f6c into 9e39938

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

@@ -86,8 +86,15 @@ impl<'block, 'global, Registers: RegisterAllocator> BrilligBlock<'block, 'global
         brillig_block.convert_block(dfg);
     }
 
-    pub(crate) fn compile_globals(&mut self, globals: &DataFlowGraph) {
+    pub(crate) fn compile_globals(
+        &mut self,
+        globals: &DataFlowGraph,
+        used_globals: &HashSet<ValueId>,
+    ) {
         for (id, value) in globals.values_iter() {
+            if !used_globals.contains(&id) {
+                continue;
+            }
             match value {
                 Value::NumericConstant { .. } => {
                     self.convert_ssa_value(id, globals);

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

@@ -1,5 +1,5 @@
 use acvm::FieldElement;
-use fxhash::FxHashMap as HashMap;
+use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
 
 use super::{
     BrilligArtifact, BrilligBlock, BrilligVariable, Function, FunctionContext, Label, ValueId,
@@ -9,6 +9,7 @@ use crate::brillig::{brillig_ir::BrilligContext, DataFlowGraph};
 pub(crate) fn convert_ssa_globals(
     enable_debug_trace: bool,
     globals: &Function,
+    used_globals: &HashSet<ValueId>,
 ) -> (BrilligArtifact<FieldElement>, HashMap<ValueId, BrilligVariable>) {
     let mut brillig_context = BrilligContext::new_for_global_init(enable_debug_trace);
     // The global space does not have globals itself
@@ -28,7 +29,7 @@ pub(crate) fn convert_ssa_globals(
         building_globals: true,
     };
 
-    brillig_block.compile_globals(&globals.dfg);
+    brillig_block.compile_globals(&globals.dfg, used_globals);
 
     brillig_context.return_instruction();
 

compiler/noirc_evaluator/src/brillig/mod.rs

@@ -84,7 +84,8 @@ impl Ssa {
 
         let mut brillig = Brillig::default();
 
-        let (artifact, brillig_globals) = convert_ssa_globals(enable_debug_trace, &self.globals);
+        let (artifact, brillig_globals) =
+            convert_ssa_globals(enable_debug_trace, &self.globals, &self.used_global_values);
         brillig.globals = artifact;
 
         for brillig_function_id in brillig_reachable_function_ids {

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

@@ -182,6 +182,10 @@ impl DataFlowGraph {
         self.values.iter()
     }
 
+    pub(crate) fn values_rev_iter(&self) -> impl ExactSizeIterator<Item = (ValueId, &Value)> {
+        self.values.rev_iter()
+    }
+
     /// Returns the parameters of the given block
     pub(crate) fn block_parameters(&self, block: BasicBlockId) -> &[ValueId] {
         self.blocks[block].parameters()

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

@@ -194,6 +194,11 @@ impl<T> DenseMap<T> {
         let ids_iter = (0..self.storage.len() as u32).map(|idx| Id::new(idx));
         ids_iter.zip(self.storage.iter())
     }
+
+    pub(crate) fn rev_iter(&self) -> impl ExactSizeIterator<Item = (Id<T>, &T)> {
+        let ids_iter = (0..self.storage.len() as u32).map(|idx| Id::new(idx));
+        ids_iter.zip(self.storage.iter()).rev()
+    }
 }
 
 impl<T> Default for DenseMap<T> {

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

@@ -24,7 +24,25 @@ impl Ssa {
     /// unused results.
     #[tracing::instrument(level = "trace", skip(self))]
     pub(crate) fn dead_instruction_elimination(mut self) -> Ssa {
-        self.functions.par_iter_mut().for_each(|(_, func)| func.dead_instruction_elimination(true));
+        let mut used_global_values: HashSet<_> = self
+            .functions
+            .par_iter_mut()
+            .flat_map(|(_, func)| func.dead_instruction_elimination(true))
+            .collect();
+
+        // Check which globals are used across all functions
+        for (id, value) in self.globals.dfg.values_rev_iter() {
+            if used_global_values.contains(&id) {
+                if let Value::Instruction { instruction, .. } = &value {
+                    let instruction = &self.globals.dfg[*instruction];
+                    instruction.for_each_value(|value_id| {
+                        used_global_values.insert(value_id);
+                    });
+                }
+            }
+        }
+
+        self.used_global_values = used_global_values;
 
         self
     }
@@ -37,7 +55,13 @@ impl Function {
     /// instructions that reference results from an instruction in another block are evaluated first.
     /// If we did not iterate blocks in this order we could not safely say whether or not the results
     /// of its instructions are needed elsewhere.
-    pub(crate) fn dead_instruction_elimination(&mut self, insert_out_of_bounds_checks: bool) {
+    ///
+    /// 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.
+    pub(crate) fn dead_instruction_elimination(
+        &mut self,
+        insert_out_of_bounds_checks: bool,
+    ) -> HashSet<ValueId> {
         let mut context = Context::default();
         for call_data in &self.dfg.data_bus.call_data {
             context.mark_used_instruction_results(&self.dfg, call_data.array_id);
@@ -58,11 +82,12 @@ impl Function {
         // instructions (we don't want to remove those checks, or instructions that are
         // dependencies of those checks)
         if inserted_out_of_bounds_checks {
-            self.dead_instruction_elimination(false);
-            return;
+            return self.dead_instruction_elimination(false);
         }
 
         context.remove_rc_instructions(&mut self.dfg);
+
+        context.used_values.into_iter().filter(|value| self.dfg.is_global(*value)).collect()
     }
 }
 
@@ -195,15 +220,17 @@ impl Context {
     /// Inspects a value and marks all instruction results as used.
     fn mark_used_instruction_results(&mut self, dfg: &DataFlowGraph, value_id: ValueId) {
         let value_id = dfg.resolve(value_id);
-        if matches!(&dfg[value_id], Value::Instruction { .. } | Value::Param { .. }) {
+        if matches!(&dfg[value_id], Value::Instruction { .. } | Value::Param { .. })
+            || dfg.is_global(value_id)
+        {
             self.used_values.insert(value_id);
         }
     }
 
-    fn remove_rc_instructions(self, dfg: &mut DataFlowGraph) {
+    fn remove_rc_instructions(&self, dfg: &mut DataFlowGraph) {
         let unused_rc_values_by_block: HashMap<BasicBlockId, HashSet<InstructionId>> =
-            self.rc_instructions.into_iter().fold(HashMap::default(), |mut acc, (rc, block)| {
-                let value = match &dfg[rc] {
+            self.rc_instructions.iter().fold(HashMap::default(), |mut acc, (rc, block)| {
+                let value = match &dfg[*rc] {
                     Instruction::IncrementRc { value } => *value,
                     Instruction::DecrementRc { value } => *value,
                     other => {
@@ -214,7 +241,7 @@ impl Context {
                 };
 
                 if !self.used_values.contains(&value) {
-                    acc.entry(block).or_default().insert(rc);
+                    acc.entry(*block).or_default().insert(*rc);
                 }
                 acc
             });

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

@@ -90,7 +90,10 @@ impl Ssa {
 
             if has_unrolled {
                 if let Some((orig_function, max_incr_pct)) = orig_func_and_max_incr_pct {
-                    let (_, brillig_globals) = convert_ssa_globals(false, &self.globals);
+                    // 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) =
+                        convert_ssa_globals(false, &self.globals, used_globals);
 
                     let new_size = brillig_bytecode_size(function, &brillig_globals);
                     let orig_size = brillig_bytecode_size(&orig_function, &brillig_globals);

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

@@ -1,6 +1,7 @@
 use std::collections::BTreeMap;
 
 use acvm::acir::circuit::ErrorSelector;
+use fxhash::FxHashSet as HashSet;
 use iter_extended::btree_map;
 use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
@@ -11,13 +12,16 @@ use crate::ssa::ir::{
 };
 use noirc_frontend::hir_def::types::Type as HirType;
 
+use super::ValueId;
+
 /// Contains the entire SSA representation of the program.
 #[serde_as]
 #[derive(Serialize, Deserialize)]
 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)]
     pub(crate) next_id: AtomicCounter<Function>,
@@ -54,9 +58,12 @@ impl Ssa {
             next_id: AtomicCounter::starting_after(max_id),
             entry_point_to_generated_index: BTreeMap::new(),
             error_selector_to_type: error_types,
-            // This field should be set afterwards as globals are generated
+            // 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(),
         }
     }