initial work to hoist shared constants across function to the global …

…brillig space. Done on the Brillig side

compiler/noirc_evaluator/src/brillig/brillig_gen.rs

@@ -4,7 +4,7 @@ pub(crate) mod brillig_block_variables;
 pub(crate) mod brillig_fn;
 pub(crate) mod brillig_globals;
 pub(crate) mod brillig_slice_ops;
-mod constant_allocation;
+pub(crate) mod constant_allocation;
 mod variable_liveness;
 
 use acvm::FieldElement;
@@ -20,14 +20,15 @@ use super::{
 };
 use crate::{
     errors::InternalError,
-    ssa::ir::{call_stack::CallStack, function::Function},
+    ssa::ir::{call_stack::CallStack, function::Function, types::NumericType},
 };
 
 /// Converting an SSA function into Brillig bytecode.
 pub(crate) fn convert_ssa_function(
     func: &Function,
     enable_debug_trace: bool,
     globals: &HashMap<ValueId, BrilligVariable>,
+    hoisted_global_constants: &HashMap<(FieldElement, NumericType), BrilligVariable>,
 ) -> BrilligArtifact<FieldElement> {
     let mut brillig_context = BrilligContext::new(enable_debug_trace);
 
@@ -44,6 +45,7 @@ pub(crate) fn convert_ssa_function(
             block,
             &func.dfg,
             globals,
+            hoisted_global_constants,
         );
     }
 

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

@@ -27,7 +27,7 @@ use num_bigint::BigUint;
 use std::sync::Arc;
 
 use super::brillig_black_box::convert_black_box_call;
-use super::brillig_block_variables::BlockVariables;
+use super::brillig_block_variables::{allocate_value_with_type, BlockVariables};
 use super::brillig_fn::FunctionContext;
 use super::constant_allocation::InstructionLocation;
 
@@ -44,6 +44,8 @@ pub(crate) struct BrilligBlock<'block, Registers: RegisterAllocator> {
     pub(crate) last_uses: HashMap<InstructionId, HashSet<ValueId>>,
 
     pub(crate) globals: &'block HashMap<ValueId, BrilligVariable>,
+    pub(crate) hoisted_global_constants:
+        &'block HashMap<(FieldElement, NumericType), BrilligVariable>,
 
     pub(crate) building_globals: bool,
 }
@@ -56,11 +58,18 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
         block_id: BasicBlockId,
         dfg: &DataFlowGraph,
         globals: &'block HashMap<ValueId, BrilligVariable>,
+        hoisted_global_constants: &'block HashMap<(FieldElement, NumericType), BrilligVariable>,
     ) {
         let live_in = function_context.liveness.get_live_in(&block_id);
 
         let mut live_in_no_globals = HashSet::default();
         for value in live_in {
+            // TODO: Move this check into a helper function
+            if let Value::NumericConstant { constant, typ } = dfg[*value] {
+                if hoisted_global_constants.get(&(constant, typ)).is_some() {
+                    continue;
+                }
+            }
             if !dfg.is_global(*value) {
                 live_in_no_globals.insert(*value);
             }
@@ -84,6 +93,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
             variables,
             last_uses,
             globals,
+            hoisted_global_constants,
             building_globals: false,
         };
 
@@ -94,7 +104,8 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
         &mut self,
         globals: &DataFlowGraph,
         used_globals: &HashSet<ValueId>,
-    ) {
+        hoisted_global_consts: &HashSet<(FieldElement, NumericType)>,
+    ) -> HashMap<(FieldElement, NumericType), BrilligVariable> {
         for (id, value) in globals.values_iter() {
             if !used_globals.contains(&id) {
                 continue;
@@ -113,6 +124,18 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                 }
             }
         }
+
+        let mut hoisted_global_constants = HashMap::default();
+        for (constant, typ) in hoisted_global_consts {
+            let new_variable = allocate_value_with_type(self.brillig_context, Type::Numeric(*typ));
+
+            self.brillig_context.const_instruction(new_variable.extract_single_addr(), *constant);
+
+            if hoisted_global_constants.insert((*constant, *typ), new_variable).is_some() {
+                unreachable!("ICE: ({constant:?}, {typ:?}) was already in cache");
+            }
+        }
+        hoisted_global_constants
     }
 
     fn convert_block(&mut self, dfg: &DataFlowGraph) {
@@ -903,6 +926,11 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                 .expect("Last uses for instruction should have been computed");
 
             for dead_variable in dead_variables {
+                if let Value::NumericConstant { constant, typ } = &dfg[*dead_variable] {
+                    if self.hoisted_global_constants.get(&(*constant, *typ)).is_some() {
+                        continue;
+                    }
+                }
                 // Globals are reserved throughout the entirety of the program
                 if !dfg.is_global(*dead_variable) {
                     self.variables.remove_variable(
@@ -1609,6 +1637,14 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
 
     fn initialize_constants(&mut self, constants: &[ValueId], dfg: &DataFlowGraph) {
         for &constant_id in constants {
+            // if let Value::NumericConstant { constant, typ } = dfg[constant_id] {
+            //     if let Some(variable) = self.hoisted_global_constants.get(&(constant, typ)) {
+            //         if self.function_context.ssa_value_allocations.insert(constant_id, *variable).is_some() {
+            //             unreachable!("ICE: ValueId {constant:?} was already in cache");
+            //         }
+            //         self.variables.available_variables.insert(constant_id);
+            //     }
+            // }
             self.convert_ssa_value(constant_id, dfg);
         }
     }
@@ -1622,6 +1658,21 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
         let value_id = dfg.resolve(value_id);
         let value = &dfg[value_id];
 
+        if !self.building_globals {
+            if let Value::NumericConstant { constant, typ } = value {
+                if let Some(variable) = self.hoisted_global_constants.get(&(*constant, *typ)) {
+                    // if self.function_context.ssa_value_allocations.insert(value_id, *variable).is_some() {
+                    //     unreachable!("ICE: ValueId {constant:?} was already in cache");
+                    // }
+                    // self.variables.available_variables.insert(value_id);
+                    return *variable;
+                }
+                // else {
+                //     panic!("ICE: should have variable already allocated");
+                // }
+            }
+        }
+
         match value {
             Value::Global(_) => {
                 unreachable!("Expected global value to be resolve to its inner value");

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

@@ -21,7 +21,7 @@ use super::brillig_fn::FunctionContext;
 
 #[derive(Debug, Default)]
 pub(crate) struct BlockVariables {
-    available_variables: HashSet<ValueId>,
+    pub(crate) available_variables: HashSet<ValueId>,
 }
 
 impl BlockVariables {

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

@@ -4,14 +4,22 @@ use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
 use super::{BrilligArtifact, BrilligBlock, BrilligVariable, FunctionContext, Label, ValueId};
 use crate::{
     brillig::{brillig_ir::BrilligContext, DataFlowGraph},
-    ssa::ir::dfg::GlobalsGraph,
+    ssa::ir::{dfg::GlobalsGraph, types::NumericType},
 };
 
+pub(crate) type BrilligGlobalsArtifact = (
+    BrilligArtifact<FieldElement>,
+    HashMap<ValueId, BrilligVariable>,
+    usize,
+    HashMap<(FieldElement, NumericType), BrilligVariable>,
+);
+
 pub(crate) fn convert_ssa_globals(
     enable_debug_trace: bool,
     globals: GlobalsGraph,
     used_globals: &HashSet<ValueId>,
-) -> (BrilligArtifact<FieldElement>, HashMap<ValueId, BrilligVariable>, usize) {
+    hoisted_global_consts: &HashSet<(FieldElement, NumericType)>,
+) -> BrilligGlobalsArtifact {
     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();
@@ -28,16 +36,18 @@ pub(crate) fn convert_ssa_globals(
         variables: Default::default(),
         last_uses: HashMap::default(),
         globals: &empty_globals,
+        hoisted_global_constants: &HashMap::default(),
         building_globals: true,
     };
 
     let globals_dfg = DataFlowGraph::from(globals);
-    brillig_block.compile_globals(&globals_dfg, used_globals);
+    let hoisted_global_constants =
+        brillig_block.compile_globals(&globals_dfg, used_globals, hoisted_global_consts);
 
     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, globals_size)
+    (artifact, function_context.ssa_value_allocations, globals_size, hoisted_global_constants)
 }

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

@@ -179,6 +179,7 @@ mod tests {
     use crate::ssa::function_builder::FunctionBuilder;
     use crate::ssa::ir::function::RuntimeType;
     use crate::ssa::ir::map::Id;
+    use crate::ssa::ir::types::NumericType;
     use crate::ssa::ssa_gen::Ssa;
 
     fn create_test_environment() -> (Ssa, FunctionContext, BrilligContext<FieldElement, Stack>) {
@@ -197,6 +198,7 @@ mod tests {
         function_context: &'a mut FunctionContext,
         brillig_context: &'a mut BrilligContext<FieldElement, Stack>,
         globals: &'a HashMap<ValueId, BrilligVariable>,
+        hoisted_global_constants: &'a HashMap<(FieldElement, NumericType), BrilligVariable>,
     ) -> BrilligBlock<'a, Stack> {
         let variables = BlockVariables::default();
         BrilligBlock {
@@ -206,6 +208,7 @@ mod tests {
             variables,
             last_uses: Default::default(),
             globals,
+            hoisted_global_constants,
             building_globals: false,
         }
     }
@@ -249,8 +252,13 @@ mod tests {
             let target_vector = BrilligVector { pointer: context.allocate_register() };
 
             let brillig_globals = HashMap::default();
-            let mut block =
-                create_brillig_block(&mut function_context, &mut context, &brillig_globals);
+            let hoisted_globals = HashMap::default();
+            let mut block = create_brillig_block(
+                &mut function_context,
+                &mut context,
+                &brillig_globals,
+                &hoisted_globals,
+            );
 
             if push_back {
                 block.slice_push_back_operation(
@@ -367,8 +375,13 @@ mod tests {
             };
 
             let brillig_globals = HashMap::default();
-            let mut block =
-                create_brillig_block(&mut function_context, &mut context, &brillig_globals);
+            let hoisted_globals = HashMap::default();
+            let mut block = create_brillig_block(
+                &mut function_context,
+                &mut context,
+                &brillig_globals,
+                &hoisted_globals,
+            );
 
             if pop_back {
                 block.slice_pop_back_operation(
@@ -475,8 +488,13 @@ mod tests {
             let target_vector = BrilligVector { pointer: context.allocate_register() };
 
             let brillig_globals = HashMap::default();
-            let mut block =
-                create_brillig_block(&mut function_context, &mut context, &brillig_globals);
+            let hoisted_globals = HashMap::default();
+            let mut block = create_brillig_block(
+                &mut function_context,
+                &mut context,
+                &brillig_globals,
+                &hoisted_globals,
+            );
 
             block.slice_insert_operation(
                 target_vector,
@@ -617,8 +635,13 @@ mod tests {
             };
 
             let brillig_globals = HashMap::default();
-            let mut block =
-                create_brillig_block(&mut function_context, &mut context, &brillig_globals);
+            let hoisted_globals = HashMap::default();
+            let mut block = create_brillig_block(
+                &mut function_context,
+                &mut context,
+                &brillig_globals,
+                &hoisted_globals,
+            );
 
             block.slice_remove_operation(
                 target_vector,

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

@@ -24,7 +24,7 @@ pub(crate) enum InstructionLocation {
 
 #[derive(Default)]
 pub(crate) struct ConstantAllocation {
-    constant_usage: HashMap<ValueId, HashMap<BasicBlockId, Vec<InstructionLocation>>>,
+    pub(crate) constant_usage: HashMap<ValueId, HashMap<BasicBlockId, Vec<InstructionLocation>>>,
     allocation_points: HashMap<BasicBlockId, HashMap<InstructionLocation, Vec<ValueId>>>,
     dominator_tree: DominatorTree,
     blocks_within_loops: HashSet<BasicBlockId>,

compiler/noirc_evaluator/src/brillig/mod.rs

@@ -2,7 +2,7 @@ pub(crate) mod brillig_gen;
 pub(crate) mod brillig_ir;
 
 use acvm::FieldElement;
-use brillig_gen::brillig_globals::convert_ssa_globals;
+use brillig_gen::{brillig_globals::convert_ssa_globals, constant_allocation::ConstantAllocation};
 use brillig_ir::{artifact::LabelType, brillig_variable::BrilligVariable, registers::GlobalSpace};
 
 use self::{
@@ -16,11 +16,12 @@ use crate::ssa::{
     ir::{
         dfg::DataFlowGraph,
         function::{Function, FunctionId},
+        types::NumericType,
         value::ValueId,
     },
     ssa_gen::Ssa,
 };
-use fxhash::FxHashMap as HashMap;
+use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
 use std::{borrow::Cow, collections::BTreeSet};
 
 pub use self::brillig_ir::procedures::ProcedureId;
@@ -42,8 +43,9 @@ impl Brillig {
         func: &Function,
         enable_debug_trace: bool,
         globals: &HashMap<ValueId, BrilligVariable>,
+        hoisted_global_constants: &HashMap<(FieldElement, NumericType), BrilligVariable>,
     ) {
-        let obj = convert_ssa_function(func, enable_debug_trace, globals);
+        let obj = convert_ssa_function(func, enable_debug_trace, globals, hoisted_global_constants);
         self.ssa_function_to_brillig.insert(func.id(), obj);
     }
 
@@ -89,17 +91,37 @@ impl Ssa {
             return brillig;
         }
 
+        // We can potentially have multiple local constants with the same value and type
+        let mut hoisted_global_consts: HashSet<(FieldElement, NumericType)> = HashSet::default();
+        for brillig_function_id in brillig_reachable_function_ids.iter() {
+            let function = &self.functions[brillig_function_id];
+            let constants = ConstantAllocation::from_function(function);
+            for (constant, _) in constants.constant_usage {
+                let value = function.dfg.get_numeric_constant(constant);
+                let value = value.unwrap();
+                let typ = function.dfg.type_of_value(constant);
+                if !function.dfg.is_global(constant) {
+                    hoisted_global_consts.insert((value, typ.unwrap_numeric()));
+                }
+            }
+        }
+
         // 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, globals, &self.used_global_values);
+        let (artifact, brillig_globals, globals_size, hoisted_global_constants) =
+            convert_ssa_globals(
+                enable_debug_trace,
+                globals,
+                &self.used_global_values,
+                &hoisted_global_consts,
+            );
         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];
-            brillig.compile(func, enable_debug_trace, &brillig_globals);
+            brillig.compile(func, enable_debug_trace, &brillig_globals, &hoisted_global_constants);
         }
 
         brillig

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

@@ -60,6 +60,7 @@ impl Ssa {
     /// See [`constant_folding`][self] module for more information.
     #[tracing::instrument(level = "trace", skip(self))]
     pub(crate) fn fold_constants(mut self) -> Ssa {
+        // let mut constants = HashSet::default();
         for function in self.functions.values_mut() {
             function.constant_fold(false, None);
         }