Merge 2bbed37 into 14a7e37

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

@@ -19,7 +19,7 @@ use super::{
     ir::{
         basic_block::BasicBlock,
         call_stack::{CallStack, CallStackId},
-        dfg::InsertInstructionResult,
+        dfg::{GlobalsGraph, InsertInstructionResult},
         function::RuntimeType,
         instruction::{ConstrainError, InstructionId, Intrinsic},
         types::NumericType,
@@ -73,6 +73,13 @@ impl FunctionBuilder {
         self.current_function.set_runtime(runtime);
     }
 
+    pub(crate) fn set_globals(&mut self, globals: Arc<GlobalsGraph>) {
+        for (_, value) in globals.values_iter() {
+            self.current_function.dfg.make_global(value.get_type().into_owned());
+        }
+        self.current_function.set_globals(globals);
+    }
+
     /// Finish the current function and create a new function.
     ///
     /// A FunctionBuilder can always only work on one function at a time, so care

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

@@ -1,4 +1,4 @@
-use std::borrow::Cow;
+use std::{borrow::Cow, sync::Arc};
 
 use crate::ssa::{function_builder::data_bus::DataBus, ir::instruction::SimplifyResult};
 
@@ -102,6 +102,31 @@ pub(crate) struct DataFlowGraph {
 
     #[serde(skip)]
     pub(crate) data_bus: DataBus,
+
+    pub(crate) globals: Arc<GlobalsGraph>,
+}
+
+/// 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.
+#[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>,
+}
+
+impl GlobalsGraph {
+    pub(crate) fn from_dfg(dfg: DataFlowGraph) -> Self {
+        Self { values: dfg.values, instructions: dfg.instructions }
+    }
+
+    /// Iterate over every Value in this DFG in no particular order, including unused Values
+    pub(crate) fn values_iter(&self) -> impl ExactSizeIterator<Item = (ValueId, &Value)> {
+        self.values.iter()
+    }
 }
 
 impl DataFlowGraph {
@@ -511,7 +536,7 @@ impl DataFlowGraph {
         &self,
         value: ValueId,
     ) -> Option<(FieldElement, NumericType)> {
-        match &self.values[self.resolve(value)] {
+        match &self[self.resolve(value)] {
             Value::NumericConstant { constant, typ } => Some((*constant, *typ)),
             _ => None,
         }
@@ -520,13 +545,15 @@ impl DataFlowGraph {
     /// Returns the Value::Array associated with this ValueId if it refers to an array constant.
     /// Otherwise, this returns None.
     pub(crate) fn get_array_constant(&self, value: ValueId) -> Option<(im::Vector<ValueId>, Type)> {
-        match &self.values[self.resolve(value)] {
-            Value::Instruction { instruction, .. } => match &self.instructions[*instruction] {
+        let value = self.resolve(value);
+        if let Some(instruction) = self.get_local_or_global_instruction(value) {
+            match instruction {
                 Instruction::MakeArray { elements, typ } => Some((elements.clone(), typ.clone())),
                 _ => None,
-            },
+            }
+        } else {
             // Arrays are shared, so cloning them is cheap
-            _ => None,
+            None
         }
     }
 
@@ -619,17 +646,23 @@ impl DataFlowGraph {
 
     /// True if the given ValueId refers to a (recursively) constant value
     pub(crate) fn is_constant(&self, argument: ValueId) -> bool {
-        match &self[self.resolve(argument)] {
+        let argument = self.resolve(argument);
+        match &self[argument] {
             Value::Param { .. } => false,
-            Value::Instruction { instruction, .. } => match &self[*instruction] {
-                Instruction::MakeArray { elements, .. } => {
-                    elements.iter().all(|element| self.is_constant(*element))
+            Value::Instruction { .. } => {
+                let Some(instruction) = self.get_local_or_global_instruction(argument) else {
+                    return false;
+                };
+                match &instruction {
+                    Instruction::MakeArray { elements, .. } => {
+                        elements.iter().all(|element| self.is_constant(*element))
+                    }
+                    _ => false,
                 }
-                _ => false,
-            },
-            // TODO: Make this true and handle instruction simplifications with globals.
-            // Currently all globals are inlined as a temporary measure so this is fine to have as false.
-            Value::Global(_) => false,
+            }
+            Value::Global(_) => {
+                unreachable!("The global value should have been indexed from the global space");
+            }
             _ => true,
         }
     }
@@ -642,6 +675,29 @@ impl DataFlowGraph {
             false
         }
     }
+
+    pub(crate) fn is_global(&self, value: ValueId) -> bool {
+        matches!(self.values[value], Value::Global(_))
+    }
+
+    /// Uses value information to determine whether an instruction is from
+    /// this function's DFG or the global space's DFG.
+    pub(crate) fn get_local_or_global_instruction(&self, value: ValueId) -> Option<&Instruction> {
+        match &self[value] {
+            Value::Instruction { instruction, .. } => {
+                let instruction = if self.is_global(value) {
+                    let instruction = &self.globals[*instruction];
+                    // We expect to only have MakeArray instructions in the global space
+                    assert!(matches!(instruction, Instruction::MakeArray { .. }));
+                    instruction
+                } else {
+                    &self[*instruction]
+                };
+                Some(instruction)
+            }
+            _ => None,
+        }
+    }
 }
 
 impl std::ops::Index<InstructionId> for DataFlowGraph {
@@ -660,7 +716,11 @@ impl std::ops::IndexMut<InstructionId> for DataFlowGraph {
 impl std::ops::Index<ValueId> for DataFlowGraph {
     type Output = Value;
     fn index(&self, id: ValueId) -> &Self::Output {
-        &self.values[id]
+        let value = &self.values[id];
+        if matches!(value, Value::Global(_)) {
+            return &self.globals[id];
+        }
+        value
     }
 }
 
@@ -678,6 +738,20 @@ impl std::ops::IndexMut<BasicBlockId> for DataFlowGraph {
     }
 }
 
+impl std::ops::Index<ValueId> for GlobalsGraph {
+    type Output = Value;
+    fn index(&self, id: ValueId) -> &Self::Output {
+        &self.values[id]
+    }
+}
+
+impl std::ops::Index<InstructionId> for GlobalsGraph {
+    type Output = Instruction;
+    fn index(&self, id: InstructionId) -> &Self::Output {
+        &self.instructions[id]
+    }
+}
+
 // The result of calling DataFlowGraph::insert_instruction can
 // be a list of results or a single ValueId if the instruction was simplified
 // to an existing value.

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

@@ -1,11 +1,12 @@
 use std::collections::BTreeSet;
+use std::sync::Arc;
 
 use iter_extended::vecmap;
 use noirc_frontend::monomorphization::ast::InlineType;
 use serde::{Deserialize, Serialize};
 
 use super::basic_block::BasicBlockId;
-use super::dfg::DataFlowGraph;
+use super::dfg::{DataFlowGraph, GlobalsGraph};
 use super::instruction::TerminatorInstruction;
 use super::map::Id;
 use super::types::Type;
@@ -112,6 +113,7 @@ impl Function {
     pub(crate) fn clone_signature(id: FunctionId, another: &Function) -> Self {
         let mut new_function = Function::new(another.name.clone(), id);
         new_function.set_runtime(another.runtime());
+        new_function.set_globals(another.dfg.globals.clone());
         new_function
     }
 
@@ -136,6 +138,10 @@ impl Function {
         self.dfg.set_runtime(runtime);
     }
 
+    pub(crate) fn set_globals(&mut self, globals: Arc<GlobalsGraph>) {
+        self.dfg.globals = globals;
+    }
+
     pub(crate) fn is_no_predicates(&self) -> bool {
         match self.runtime() {
             RuntimeType::Acir(inline_type) => matches!(inline_type, InlineType::NoPredicates),

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

@@ -1093,28 +1093,27 @@ fn try_optimize_array_get_from_previous_set(
     // Arbitrary number of maximum tries just to prevent this optimization from taking too long.
     let max_tries = 5;
     for _ in 0..max_tries {
-        match &dfg[array_id] {
-            Value::Instruction { instruction, .. } => {
-                match &dfg[*instruction] {
-                    Instruction::ArraySet { array, index, value, .. } => {
-                        if let Some(constant) = dfg.get_numeric_constant(*index) {
-                            if constant == target_index {
-                                return SimplifyResult::SimplifiedTo(*value);
-                            }
-
-                            array_id = *array; // recur
-                        } else {
-                            return SimplifyResult::None;
+        if let Some(instruction) = dfg.get_local_or_global_instruction(array_id) {
+            match instruction {
+                Instruction::ArraySet { array, index, value, .. } => {
+                    if let Some(constant) = dfg.get_numeric_constant(*index) {
+                        if constant == target_index {
+                            return SimplifyResult::SimplifiedTo(*value);
                         }
+
+                        array_id = *array; // recur
+                    } else {
+                        return SimplifyResult::None;
                     }
-                    Instruction::MakeArray { elements: array, typ: _ } => {
-                        elements = Some(array.clone());
-                        break;
-                    }
-                    _ => return SimplifyResult::None,
                 }
+                Instruction::MakeArray { elements: array, typ: _ } => {
+                    elements = Some(array.clone());
+                    break;
+                }
+                _ => return SimplifyResult::None,
             }
-            _ => return SimplifyResult::None,
+        } else {
+            return SimplifyResult::None;
         }
     }
 

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

@@ -85,7 +85,13 @@ fn value(dfg: &DataFlowGraph, id: ValueId) -> String {
         Value::Function(id) => id.to_string(),
         Value::Intrinsic(intrinsic) => intrinsic.to_string(),
         Value::ForeignFunction(function) => function.clone(),
-        Value::Param { .. } | Value::Instruction { .. } => id.to_string(),
+        Value::Param { .. } | Value::Instruction { .. } => {
+            if dfg.is_global(id) {
+                format!("g{}", id.to_u32())
+            } else {
+                id.to_string()
+            }
+        }
         Value::Global(_) => {
             format!("g{}", id.to_u32())
         }

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

@@ -278,8 +278,10 @@ fn create_apply_function(
     function_ids: Vec<FunctionId>,
 ) -> FunctionId {
     assert!(!function_ids.is_empty());
+    let globals = ssa.functions[&function_ids[0]].dfg.globals.clone();
     ssa.add_fn(|id| {
         let mut function_builder = FunctionBuilder::new("apply".to_string(), id);
+        function_builder.set_globals(globals);
         let target_id = function_builder.add_parameter(Type::field());
         let params_ids = vecmap(signature.params, |typ| function_builder.add_parameter(typ));
 

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

@@ -364,6 +364,8 @@ impl InlineContext {
         let source = &ssa.functions[&entry_point];
         let mut builder = FunctionBuilder::new(source.name().to_owned(), entry_point);
         builder.set_runtime(source.runtime());
+        builder.current_function.set_globals(source.dfg.globals.clone());
+
         Self {
             builder,
             recursion_level: 0,
@@ -382,7 +384,7 @@ impl InlineContext {
         let mut context = PerFunctionContext::new(&mut self, entry_point, &ssa.globals);
         context.inlining_entry = true;
 
-        for (_, value) in ssa.globals.dfg.values_iter() {
+        for (_, value) in entry_point.dfg.globals.values_iter() {
             context.context.builder.current_function.dfg.make_global(value.get_type().into_owned());
         }
 
@@ -476,13 +478,30 @@ impl<'function> PerFunctionContext<'function> {
         }
 
         let new_value = match &self.source_function.dfg[id] {
-            value @ Value::Instruction { .. } => {
+            value @ Value::Instruction { instruction, .. } => {
+                // TODO: Inlining the global into the function is only a temporary measure
+                // until Brillig gen with globals is working end to end
+                if self.source_function.dfg.is_global(id) {
+                    let Instruction::MakeArray { elements, typ } = &self.globals.dfg[*instruction]
+                    else {
+                        panic!("Only expect Instruction::MakeArray for a global");
+                    };
+                    let elements = elements
+                        .iter()
+                        .map(|element| self.translate_value(*element))
+                        .collect::<im::Vector<_>>();
+                    return self.context.builder.insert_make_array(elements, typ.clone());
+                }
                 unreachable!("All Value::Instructions should already be known during inlining after creating the original inlined instruction. Unknown value {id} = {value:?}")
             }
             value @ Value::Param { .. } => {
                 unreachable!("All Value::Params should already be known from previous calls to translate_block. Unknown value {id} = {value:?}")
             }
             Value::NumericConstant { constant, typ } => {
+                // TODO: Inlining the global into the function is only a temporary measure
+                // until Brillig gen with globals is working end to end.
+                // The dfg indexes a global's inner value directly, so we will need to check here
+                // whether we have a global.
                 self.context.builder.numeric_constant(*constant, *typ)
             }
             Value::Function(function) => self.context.builder.import_function(*function),
@@ -491,26 +510,7 @@ impl<'function> PerFunctionContext<'function> {
                 self.context.builder.import_foreign_function(function)
             }
             Value::Global(_) => {
-                // TODO: Inlining the global into the function is only a temporary measure
-                // until Brillig gen with globals is working end to end
-                match &self.globals.dfg[id] {
-                    Value::Instruction { instruction, .. } => {
-                        let Instruction::MakeArray { elements, typ } =
-                            &self.globals.dfg[*instruction]
-                        else {
-                            panic!("Only expect Instruction::MakeArray for a global");
-                        };
-                        let elements = elements
-                            .iter()
-                            .map(|element| self.translate_value(*element))
-                            .collect::<im::Vector<_>>();
-                        self.context.builder.insert_make_array(elements, typ.clone())
-                    }
-                    Value::NumericConstant { constant, typ } => {
-                        self.context.builder.numeric_constant(*constant, *typ)
-                    }
-                    _ => panic!("Expected only an instruction or a numeric constant"),
-                }
+                panic!("Expected a global to be resolved to its inner value");
             }
         };