merge conflicts w/ af/fix-11249

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

@@ -26,27 +26,22 @@ impl Ssa {
     /// This step should come after the flattening of the CFG and mem2reg.
     #[tracing::instrument(level = "trace", skip(self))]
     pub(crate) fn dead_instruction_elimination(self) -> Ssa {
-        self.dead_instruction_elimination_inner(true, false)
+        self.dead_instruction_elimination_inner(true)
     }
 
-    fn dead_instruction_elimination_inner(
-        mut self,
-        flattened: bool,
-        keep_rcs_of_parameters: bool,
-    ) -> Ssa {
+    fn dead_instruction_elimination_inner(mut self, flattened: bool) -> Ssa {
         let mut used_global_values: HashSet<_> = self
             .functions
             .par_iter_mut()
-            .flat_map(|(_, func)| {
-                func.dead_instruction_elimination(true, flattened, keep_rcs_of_parameters)
-            })
+            .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);
                     });
@@ -74,7 +69,6 @@ impl Function {
         &mut self,
         insert_out_of_bounds_checks: bool,
         flattened: bool,
-        keep_rcs_of_parameters: bool,
     ) -> HashSet<ValueId> {
         let mut context = Context { flattened, ..Default::default() };
 
@@ -90,15 +84,14 @@ impl Function {
                 self,
                 *block,
                 insert_out_of_bounds_checks,
-                keep_rcs_of_parameters,
             );
         }
 
         // If we inserted out of bounds check, let's run the pass again with those new
         // instructions (we don't want to remove those checks, or instructions that are
         // dependencies of those checks)
         if inserted_out_of_bounds_checks {
-            return self.dead_instruction_elimination(false, flattened, keep_rcs_of_parameters);
+            return self.dead_instruction_elimination(false, flattened);
         }
 
         context.remove_rc_instructions(&mut self.dfg);
@@ -147,7 +140,6 @@ impl Context {
         function: &mut Function,
         block_id: BasicBlockId,
         insert_out_of_bounds_checks: bool,
-        _keep_rcs_of_parameters: bool,
     ) -> bool {
         let block = &function.dfg[block_id];
         self.mark_terminator_values_as_used(function, block);
@@ -607,6 +599,8 @@ struct RcTracker {
 }
 
 impl RcTracker {
+    // During the preprocessing of functions in isolation we don't want to
+    // get rid of IncRCs arrays that can potentially be mutated outside.
     fn mark_terminator_arrays_as_used(&mut self, function: &Function, block: &BasicBlock) {
         block.unwrap_terminator().for_each_value(|value| {
             let typ = function.dfg.type_of_value(value);
@@ -676,8 +670,6 @@ impl RcTracker {
             }
             Instruction::Call { arguments, .. } => {
                 // Treat any array-type arguments to calls as possible sources of mutation.
-                // During the preprocessing of functions in isolation we don't want to
-                // get rid of IncRCs arrays that can potentially be mutated outside.
                 for arg in arguments {
                     let typ = function.dfg.type_of_value(*arg);
                     if matches!(&typ, Type::Array(_, _) | Type::Slice(_)) {
@@ -940,6 +932,42 @@ mod test {
         assert_normalized_ssa_equals(ssa, src);
     }
 
+    #[test]
+    fn not_remove_inc_rcs_for_input_parameters() {
+        let src = "
+        brillig(inline) fn main f0 {
+          b0(v0: [Field; 2]):
+            inc_rc v0
+            inc_rc v0
+            inc_rc v0
+            v2 = array_get v0, index u32 0 -> Field
+            inc_rc v0
+            return v2
+        }
+        ";
+
+        let ssa = Ssa::from_str(src).unwrap();
+        let main = ssa.main();
+
+        // The instruction count never includes the terminator instruction
+        assert_eq!(main.dfg[main.entry_block()].instructions().len(), 5);
+
+        let expected = "
+        brillig(inline) fn main f0 {
+          b0(v0: [Field; 2]):
+            inc_rc v0
+            v2 = array_get v0, index u32 0 -> Field
+            inc_rc v0
+            return v2
+        }
+        ";
+
+        // We want to be able to switch this on during preprocessing.
+        let keep_rcs_of_parameters = true;
+        let ssa = ssa.dead_instruction_elimination_inner(true, keep_rcs_of_parameters);
+        assert_normalized_ssa_equals(ssa, expected);
+    }
+
     #[test]
     fn remove_inc_rcs_that_are_never_mutably_borrowed() {
         let src = "

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