diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 3d7e0b06d5b..154d485b143 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/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) { + 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 diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 6eae291ca47..173669031a2 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/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, +} + +/// 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, + /// All of the instructions in the global value space. + /// These are expected to all be Instruction::MakeArray + instructions: DenseMap, +} + +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 { + 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, 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 for DataFlowGraph { @@ -660,7 +716,11 @@ impl std::ops::IndexMut for DataFlowGraph { impl std::ops::Index 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 for DataFlowGraph { } } +impl std::ops::Index for GlobalsGraph { + type Output = Value; + fn index(&self, id: ValueId) -> &Self::Output { + &self.values[id] + } +} + +impl std::ops::Index 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. diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index a2068d94661..b59b0c18a10 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/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) { + self.dfg.globals = globals; + } + pub(crate) fn is_no_predicates(&self) -> bool { match self.runtime() { RuntimeType::Acir(inline_type) => matches!(inline_type, InlineType::NoPredicates), diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 786c3671d38..35555c7b13f 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/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; } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 7fe12b83ea9..9f9191eb6bd 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/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()) } diff --git a/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs b/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs index 7d7798fd30a..186f10c53e6 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs @@ -278,8 +278,10 @@ fn create_apply_function( function_ids: Vec, ) -> 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)); diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index b1dd203cfd0..8e0614d15de 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/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::>(); + 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::>(); - 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"); } }; diff --git a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs index 5f21e3816f0..b248f6734a9 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -3,7 +3,6 @@ use std::collections::BTreeMap; use crate::ssa::{ ir::{ basic_block::BasicBlockId, - dfg::DataFlowGraph, function::{Function, FunctionId}, map::SparseMap, post_order::PostOrder, @@ -26,7 +25,7 @@ impl Ssa { let mut context = Context::default(); context.populate_functions(&self.functions); for function in self.functions.values_mut() { - context.normalize_ids(function, &self.globals.dfg); + context.normalize_ids(function); } self.functions = context.functions.into_btree(); } @@ -66,14 +65,14 @@ impl Context { } } - fn normalize_ids(&mut self, old_function: &mut Function, globals: &DataFlowGraph) { + fn normalize_ids(&mut self, old_function: &mut Function) { self.new_ids.blocks.clear(); self.new_ids.values.clear(); let new_function_id = self.new_ids.function_ids[&old_function.id()]; let new_function = &mut self.functions[new_function_id]; - for (_, value) in globals.values_iter() { + for (_, value) in old_function.dfg.globals.values_iter() { new_function.dfg.make_global(value.get_type().into_owned()); } @@ -171,6 +170,11 @@ impl IdMaps { old_value: ValueId, ) -> ValueId { let old_value = old_function.dfg.resolve(old_value); + if old_function.dfg.is_global(old_value) { + // Globals are computed at compile-time and thus are expected to be remain normalized + // between SSA passes + return old_value; + } match &old_function.dfg[old_value] { value @ Value::Instruction { instruction, .. } => { *self.values.get(&old_value).unwrap_or_else(|| { @@ -198,9 +202,7 @@ impl IdMaps { Value::Intrinsic(intrinsic) => new_function.dfg.import_intrinsic(*intrinsic), Value::ForeignFunction(name) => new_function.dfg.import_foreign_function(name), Value::Global(_) => { - // Globals are computed at compile-time and thus are expected to be remain normalized - // between SSA passes - old_value + unreachable!("Should have handled the global case already"); }, } } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 1e7a9b7e8b1..0b778ef9b78 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -20,6 +20,7 @@ use crate::ssa::ir::types::{NumericType, Type}; use crate::ssa::ir::value::ValueId; use super::value::{Tree, Value, Values}; +use super::GlobalsGraph; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; /// The FunctionContext is the main context object for translating a @@ -109,6 +110,7 @@ impl<'a> FunctionContext<'a> { parameters: &Parameters, runtime: RuntimeType, shared_context: &'a SharedContext, + globals: GlobalsGraph, ) -> Self { let function_id = shared_context .pop_next_function_in_queue() @@ -117,10 +119,10 @@ impl<'a> FunctionContext<'a> { let mut builder = FunctionBuilder::new(function_name, function_id); builder.set_runtime(runtime); + builder.set_globals(Arc::new(globals)); let definitions = HashMap::default(); let mut this = Self { definitions, builder, shared_context, loops: Vec::new() }; - this.add_globals(); this.add_parameters_to_scope(parameters); this } @@ -132,23 +134,18 @@ impl<'a> FunctionContext<'a> { /// avoid calling new_function until the previous function is completely finished with ssa-gen. pub(super) fn new_function(&mut self, id: IrFunctionId, func: &ast::Function) { self.definitions.clear(); + + let globals = self.builder.current_function.dfg.globals.clone(); if func.unconstrained { self.builder.new_brillig_function(func.name.clone(), id, func.inline_type); } else { self.builder.new_function(func.name.clone(), id, func.inline_type); } - - self.add_globals(); + self.builder.set_globals(globals); self.add_parameters_to_scope(&func.parameters); } - fn add_globals(&mut self) { - for (_, value) in self.shared_context.globals_context.dfg.values_iter() { - self.builder.current_function.dfg.make_global(value.get_type().into_owned()); - } - } - /// Add each parameter to the current scope, and return the list of parameter types. /// /// The returned parameter type list will be flattened, so any struct parameters will @@ -1087,6 +1084,7 @@ impl SharedContext { &vec![], RuntimeType::Brillig(InlineType::default()), &globals_shared_context, + GlobalsGraph::default(), ); let mut globals = BTreeMap::default(); for (id, global) in program.globals.iter() { diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 529c207a4ad..d73a5946b4c 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -23,6 +23,7 @@ use self::{ value::{Tree, Values}, }; +use super::ir::dfg::GlobalsGraph; use super::ir::instruction::ErrorType; use super::ir::types::NumericType; use super::{ @@ -49,6 +50,8 @@ pub(crate) fn generate_ssa(program: Program) -> Result { let return_location = program.return_location; let context = SharedContext::new(program); + let globals = GlobalsGraph::from_dfg(context.globals_context.dfg.clone()); + let main_id = Program::main_id(); let main = context.program.main(); @@ -60,7 +63,7 @@ pub(crate) fn generate_ssa(program: Program) -> Result { RuntimeType::Acir(main.inline_type) }; let mut function_context = - FunctionContext::new(main.name.clone(), &main.parameters, main_runtime, &context); + FunctionContext::new(main.name.clone(), &main.parameters, main_runtime, &context, globals); // Generate the call_data bus from the relevant parameters. We create it *before* processing the function body let call_data = function_context.builder.call_data_bus(is_databus);