diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs index 786a03031d6..30920fd300d 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs @@ -13,7 +13,24 @@ use super::brillig_ir::{ artifact::{BrilligArtifact, Label}, BrilligContext, }; -use crate::ssa::ir::function::Function; +use crate::ssa::ir::{ + function::Function, + value::{IsResolved, ResolvedValueId, ValueId}, +}; + +/// Private resolution type, to limit the scope of storing them in data structures. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +struct FinalResolved {} + +impl IsResolved for FinalResolved {} + +type FinalValueId = ValueId; + +impl From for FinalValueId { + fn from(value: ResolvedValueId) -> Self { + ValueId::new(value.raw()) + } +} /// Converting an SSA function into Brillig bytecode. pub(crate) fn convert_ssa_function( diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs index 3685c9540f3..ba6c75bc38b 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs @@ -14,7 +14,7 @@ use crate::brillig::brillig_ir::{ /// Transforms SSA's black box function calls into the corresponding brillig instructions /// Extracting arguments and results from the SSA function call /// And making any necessary type conversions to adapt noir's blackbox calls to brillig's -pub(crate) fn convert_black_box_call( +pub(super) fn convert_black_box_call( brillig_context: &mut BrilligContext, bb_func: &BlackBoxFunc, function_arguments: &[BrilligVariable], diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 7d4a1189c0a..076a3bbca87 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -31,23 +31,24 @@ use super::brillig_black_box::convert_black_box_call; use super::brillig_block_variables::BlockVariables; use super::brillig_fn::FunctionContext; use super::constant_allocation::InstructionLocation; +use super::FinalValueId; /// Generate the compilation artifacts for compiling a function into brillig bytecode. -pub(crate) struct BrilligBlock<'block> { - pub(crate) function_context: &'block mut FunctionContext, +pub(super) struct BrilligBlock<'block> { + pub(super) function_context: &'block mut FunctionContext, /// The basic block that is being converted - pub(crate) block_id: BasicBlockId, + pub(super) block_id: BasicBlockId, /// Context for creating brillig opcodes - pub(crate) brillig_context: &'block mut BrilligContext, + pub(super) brillig_context: &'block mut BrilligContext, /// Tracks the available variable during the codegen of the block - pub(crate) variables: BlockVariables, + pub(super) variables: BlockVariables, /// For each instruction, the set of values that are not used anymore after it. - pub(crate) last_uses: HashMap>, + pub(super) last_uses: HashMap>, } impl<'block> BrilligBlock<'block> { /// Converts an SSA Basic block into a sequence of Brillig opcodes - pub(crate) fn compile( + pub(super) fn compile( function_context: &'block mut FunctionContext, brillig_context: &'block mut BrilligContext, block_id: BasicBlockId, @@ -150,7 +151,7 @@ impl<'block> BrilligBlock<'block> { let target_block = &dfg[*destination_block]; for (src, dest) in arguments.iter().zip(target_block.parameters()) { // Destinations are block parameters so they should have been allocated previously. - let dest = dfg.resolve(*dest); + let dest = dfg.resolve(*dest).into(); let destination = self.variables.get_allocation(self.function_context, dest); let source = self.convert_ssa_value(*src, dfg); self.brillig_context @@ -762,7 +763,7 @@ impl<'block> BrilligBlock<'block> { } Instruction::MakeArray { elements: array, typ } => { let value_id = dfg.instruction_results(instruction_id)[0]; - if !self.variables.is_allocated(dfg.resolve(value_id)) { + if !self.variables.is_allocated(dfg.resolve(value_id).into()) { let new_variable = self.variables.define_variable( self.function_context, self.brillig_context, @@ -1498,7 +1499,7 @@ impl<'block> BrilligBlock<'block> { } } - fn initialize_constants(&mut self, constants: &[ResolvedValueId], dfg: &DataFlowGraph) { + fn initialize_constants(&mut self, constants: &[FinalValueId], dfg: &DataFlowGraph) { for &constant_id in constants { self.convert_ssa_value(constant_id, dfg); } @@ -1510,7 +1511,7 @@ impl<'block> BrilligBlock<'block> { value_id: ValueId, dfg: &DataFlowGraph, ) -> BrilligVariable { - let value_id = dfg.resolve(value_id); + let value_id = dfg.resolve(value_id).into(); let value = &dfg[value_id]; match value { @@ -1849,7 +1850,7 @@ impl<'block> BrilligBlock<'block> { } /// Returns the type of the operation considering the types of the operands -pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type, op: BinaryOp) -> Type { +pub(super) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type, op: BinaryOp) -> Type { match (lhs_type, rhs_type) { (_, Type::Function) | (Type::Function, _) => { unreachable!("Functions are invalid in binary operations") diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs index 4c102ae82ef..2a449b36527 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs @@ -13,27 +13,27 @@ use crate::{ ssa::ir::{ dfg::DataFlowGraph, types::{CompositeType, Type}, - value::{ResolvedValueId, ValueId}, + value::ValueId, }, }; -use super::brillig_fn::FunctionContext; +use super::{brillig_fn::FunctionContext, FinalValueId}; #[derive(Debug, Default)] -pub(crate) struct BlockVariables { +pub(super) struct BlockVariables { // Since we're generating Brillig bytecode here, there shouldn't be any more value ID replacements // and we can use the final resolved ID. - available_variables: HashSet, + available_variables: HashSet, } impl BlockVariables { /// Creates a BlockVariables instance. It uses the variables that are live in to the block and the global available variables (block parameters) - pub(crate) fn new(live_in: HashSet) -> Self { + pub(super) fn new(live_in: HashSet) -> Self { BlockVariables { available_variables: live_in } } /// Returns all variables that have not been removed at this point. - pub(crate) fn get_available_variables( + pub(super) fn get_available_variables( &self, function_context: &FunctionContext, ) -> Vec { @@ -50,14 +50,14 @@ impl BlockVariables { } /// For a given SSA value id, define the variable and return the corresponding cached allocation. - pub(crate) fn define_variable( + pub(super) fn define_variable( &mut self, function_context: &mut FunctionContext, brillig_context: &mut BrilligContext, value_id: ValueId, dfg: &DataFlowGraph, ) -> BrilligVariable { - let value_id = dfg.resolve(value_id); + let value_id = dfg.resolve(value_id).into(); let variable = allocate_value(value_id, brillig_context, dfg); if function_context.ssa_value_allocations.insert(value_id, variable).is_some() { @@ -70,7 +70,7 @@ impl BlockVariables { } /// Defines a variable that fits in a single register and returns the allocated register. - pub(crate) fn define_single_addr_variable( + pub(super) fn define_single_addr_variable( &mut self, function_context: &mut FunctionContext, brillig_context: &mut BrilligContext, @@ -82,9 +82,9 @@ impl BlockVariables { } /// Removes a variable so it's not used anymore within this block. - pub(crate) fn remove_variable( + pub(super) fn remove_variable( &mut self, - value_id: ResolvedValueId, + value_id: FinalValueId, function_context: &mut FunctionContext, brillig_context: &mut BrilligContext, ) { @@ -97,15 +97,15 @@ impl BlockVariables { } /// Checks if a variable is allocated. - pub(crate) fn is_allocated(&self, value_id: ResolvedValueId) -> bool { + pub(super) fn is_allocated(&self, value_id: FinalValueId) -> bool { self.available_variables.contains(&value_id) } /// For a given SSA value id, return the corresponding cached allocation. - pub(crate) fn get_allocation( + pub(super) fn get_allocation( &mut self, function_context: &FunctionContext, - value_id: ResolvedValueId, + value_id: FinalValueId, ) -> BrilligVariable { assert!( self.available_variables.contains(&value_id), @@ -120,13 +120,13 @@ impl BlockVariables { } /// Computes the length of an array. This will match with the indexes that SSA will issue -pub(crate) fn compute_array_length(item_typ: &CompositeType, elem_count: usize) -> usize { +pub(super) fn compute_array_length(item_typ: &CompositeType, elem_count: usize) -> usize { item_typ.len() * elem_count } /// For a given value_id, allocates the necessary registers to hold it. -pub(crate) fn allocate_value( - value_id: ResolvedValueId, +pub(super) fn allocate_value( + value_id: FinalValueId, brillig_context: &mut BrilligContext, dfg: &DataFlowGraph, ) -> BrilligVariable { diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs index 2f1a641d4bf..ec5aa5b0783 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs @@ -10,28 +10,29 @@ use crate::{ function::{Function, FunctionId}, post_order::PostOrder, types::Type, - value::ResolvedValueId, }, }; use fxhash::FxHashMap as HashMap; -use super::{constant_allocation::ConstantAllocation, variable_liveness::VariableLiveness}; +use super::{ + constant_allocation::ConstantAllocation, variable_liveness::VariableLiveness, FinalValueId, +}; pub(crate) struct FunctionContext { - pub(crate) function_id: FunctionId, + pub(super) function_id: 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, + pub(super) ssa_value_allocations: HashMap, /// The block ids of the function in reverse post order. - pub(crate) blocks: Vec, + pub(super) blocks: Vec, /// Liveness information for each variable in the function. - pub(crate) liveness: VariableLiveness, + pub(super) liveness: VariableLiveness, /// Information on where to allocate constants - pub(crate) constant_allocation: ConstantAllocation, + pub(super) constant_allocation: ConstantAllocation, } impl FunctionContext { /// Creates a new function context. It will allocate parameters for all blocks and compute the liveness of every variable. - pub(crate) fn new(function: &Function) -> Self { + pub(super) fn new(function: &Function) -> Self { let id = function.id(); let mut reverse_post_order = Vec::new(); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/constant_allocation.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/constant_allocation.rs index 028fa8a6890..9a7af265bf7 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/constant_allocation.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/constant_allocation.rs @@ -4,33 +4,30 @@ use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; use crate::ssa::ir::{ - basic_block::BasicBlockId, - cfg::ControlFlowGraph, - dfg::DataFlowGraph, - dom::DominatorTree, - function::Function, - instruction::InstructionId, - post_order::PostOrder, - value::{ResolvedValueId, Value}, + basic_block::BasicBlockId, cfg::ControlFlowGraph, dfg::DataFlowGraph, dom::DominatorTree, + function::Function, instruction::InstructionId, post_order::PostOrder, value::Value, }; -use super::variable_liveness::{collect_variables_of_value, variables_used_in_instruction}; +use super::{ + variable_liveness::{collect_variables_of_value, variables_used_in_instruction}, + FinalValueId, +}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub(crate) enum InstructionLocation { +pub(super) enum InstructionLocation { Instruction(InstructionId), Terminator, } -pub(crate) struct ConstantAllocation { - constant_usage: HashMap>>, - allocation_points: HashMap>>, +pub(super) struct ConstantAllocation { + constant_usage: HashMap>>, + allocation_points: HashMap>>, dominator_tree: DominatorTree, blocks_within_loops: HashSet, } impl ConstantAllocation { - pub(crate) fn from_function(func: &Function) -> Self { + pub(super) fn from_function(func: &Function) -> Self { let cfg = ControlFlowGraph::with_function(func); let post_order = PostOrder::with_function(func); let mut dominator_tree = DominatorTree::with_cfg_and_post_order(&cfg, &post_order); @@ -47,17 +44,17 @@ impl ConstantAllocation { instance } - pub(crate) fn allocated_in_block(&self, block_id: BasicBlockId) -> Vec { + pub(super) fn allocated_in_block(&self, block_id: BasicBlockId) -> Vec { self.allocation_points.get(&block_id).map_or(Vec::default(), |allocations| { allocations.iter().flat_map(|(_, constants)| constants.iter()).copied().collect() }) } - pub(crate) fn allocated_at_location( + pub(super) fn allocated_at_location( &self, block_id: BasicBlockId, location: InstructionLocation, - ) -> Vec { + ) -> Vec { self.allocation_points.get(&block_id).map_or(Vec::default(), |allocations| { allocations.get(&location).map_or(Vec::default(), |constants| constants.clone()) }) @@ -65,7 +62,7 @@ impl ConstantAllocation { fn collect_constant_usage(&mut self, func: &Function) { let mut record_if_constant = - |block_id: BasicBlockId, value_id: ResolvedValueId, location: InstructionLocation| { + |block_id: BasicBlockId, value_id: FinalValueId, location: InstructionLocation| { if is_constant_value(value_id, &func.dfg) { self.constant_usage .entry(value_id) @@ -126,7 +123,7 @@ impl ConstantAllocation { fn decide_allocation_point( &self, - constant_id: ResolvedValueId, + constant_id: FinalValueId, blocks_where_is_used: &[BasicBlockId], func: &Function, ) -> BasicBlockId { @@ -164,7 +161,7 @@ impl ConstantAllocation { } } -pub(crate) fn is_constant_value(id: ResolvedValueId, dfg: &DataFlowGraph) -> bool { +pub(super) fn is_constant_value(id: FinalValueId, dfg: &DataFlowGraph) -> bool { matches!(&dfg[id], Value::NumericConstant { .. }) } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs index 478679087cb..1562404734b 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs @@ -9,12 +9,12 @@ use crate::ssa::ir::{ function::Function, instruction::{Instruction, InstructionId}, post_order::PostOrder, - value::{ResolvedValueId, Value, ValueId}, + value::{Value, ValueId}, }; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; -use super::constant_allocation::ConstantAllocation; +use super::{constant_allocation::ConstantAllocation, FinalValueId}; /// A back edge is an edge from a node to one of its ancestors. It denotes a loop in the CFG. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -45,11 +45,11 @@ fn find_back_edges( } /// Collects the underlying variables inside a value id. It might be more than one, for example in constant arrays that are constructed with multiple vars. -pub(crate) fn collect_variables_of_value( +pub(super) fn collect_variables_of_value( value_id: ValueId, dfg: &DataFlowGraph, -) -> Option { - let value_id = dfg.resolve(value_id); +) -> Option { + let value_id = dfg.resolve(value_id).into(); let value = &dfg[value_id]; match value { @@ -61,7 +61,7 @@ pub(crate) fn collect_variables_of_value( } } -pub(crate) fn variables_used_in_instruction( +pub(super) fn variables_used_in_instruction( instruction: &Instruction, dfg: &DataFlowGraph, ) -> Variables { @@ -86,7 +86,7 @@ fn variables_used_in_block(block: &BasicBlock, dfg: &DataFlowGraph) -> Variables .collect(); // We consider block parameters used, so they live up to the block that owns them. - used.extend(block.parameters().iter().map(|p| p.resolved())); + used.extend(block.parameters().iter().map(|p| FinalValueId::from(p.resolved()))); if let Some(terminator) = block.terminator() { terminator.for_each_value(|value_id| { @@ -97,7 +97,7 @@ fn variables_used_in_block(block: &BasicBlock, dfg: &DataFlowGraph) -> Variables used } -type Variables = HashSet; +type Variables = HashSet; fn compute_used_before_def( block: &BasicBlock, @@ -113,7 +113,7 @@ fn compute_used_before_def( type LastUses = HashMap; /// A struct representing the liveness of variables throughout a function. -pub(crate) struct VariableLiveness { +pub(super) struct VariableLiveness { cfg: ControlFlowGraph, post_order: PostOrder, dominator_tree: DominatorTree, @@ -127,7 +127,7 @@ pub(crate) struct VariableLiveness { impl VariableLiveness { /// Computes the liveness of variables throughout a function. - pub(crate) fn from_function(func: &Function, constants: &ConstantAllocation) -> Self { + pub(super) fn from_function(func: &Function, constants: &ConstantAllocation) -> Self { let cfg = ControlFlowGraph::with_function(func); let post_order = PostOrder::with_function(func); let dominator_tree = DominatorTree::with_cfg_and_post_order(&cfg, &post_order); @@ -151,12 +151,12 @@ impl VariableLiveness { } /// The set of values that are alive before the block starts executing - pub(crate) fn get_live_in(&self, block_id: &BasicBlockId) -> &Variables { + pub(super) fn get_live_in(&self, block_id: &BasicBlockId) -> &Variables { self.live_in.get(block_id).expect("Live ins should have been calculated") } /// The set of values that are alive after the block has finished executed - pub(crate) fn get_live_out(&self, block_id: &BasicBlockId) -> Variables { + pub(super) fn get_live_out(&self, block_id: &BasicBlockId) -> Variables { let mut live_out = HashSet::default(); for successor_id in self.cfg.successors(*block_id) { live_out.extend(self.get_live_in(&successor_id)); @@ -165,14 +165,14 @@ impl VariableLiveness { } /// A map of instruction id to the set of values that die after the instruction has executed - pub(crate) fn get_last_uses(&self, block_id: &BasicBlockId) -> &LastUses { + pub(super) fn get_last_uses(&self, block_id: &BasicBlockId) -> &LastUses { self.last_uses.get(block_id).expect("Last uses should have been calculated") } /// Retrieves the list of block params the given block is defining. /// Block params are defined before the block that owns them (since they are used by the predecessor blocks). They must be defined in the immediate dominator. /// This is the last point where the block param can be allocated without it being allocated in different places in different branches. - pub(crate) fn defined_block_params(&self, block_id: &BasicBlockId) -> Vec { + pub(super) fn defined_block_params(&self, block_id: &BasicBlockId) -> Vec { self.param_definitions.get(block_id).cloned().unwrap_or_default() } @@ -244,13 +244,13 @@ impl VariableLiveness { let mut defined_vars = HashSet::default(); for parameter in self.defined_block_params(&block_id) { - defined_vars.insert(dfg.resolve(parameter)); + defined_vars.insert(dfg.resolve(parameter).into()); } for instruction_id in block.instructions() { let result_values = dfg.instruction_results(*instruction_id); for result_value in result_values { - defined_vars.insert(dfg.resolve(*result_value)); + defined_vars.insert(dfg.resolve(*result_value).into()); } } @@ -338,10 +338,10 @@ mod test { use crate::ssa::ir::map::Id; use crate::ssa::ir::types::Type; - use super::{ResolvedValueId, ValueId}; + use super::{FinalValueId, ValueId}; - fn resolved_set(it: &[ValueId]) -> FxHashSet { - FxHashSet::from_iter(it.iter().map(|v| v.resolved())) + fn resolved_set(it: &[ValueId]) -> FxHashSet { + FxHashSet::from_iter(it.iter().map(|v| v.resolved().into())) } #[test] @@ -637,7 +637,7 @@ mod test { liveness .defined_block_params(&block_id) .iter() - .map(|p| p.resolved()) + .map(|p| p.resolved().into()) .collect::() }; diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 0767b4aaf33..09862c10920 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -10,7 +10,7 @@ use super::{ }, map::DenseMap, types::Type, - value::{RawValueId, Resolution, ResolvedValueId, Value, ValueId}, + value::{IsResolved, RawValueId, Resolution, ResolvedValueId, Unresolved, Value, ValueId}, }; use acvm::{acir::AcirField, FieldElement}; @@ -581,16 +581,19 @@ impl std::ops::IndexMut for DataFlowGraph { /// Indexing the DFG by unresolved value IDs is all over the codebase, /// but it's not obvious whether we should apply resolution. -impl std::ops::Index for DataFlowGraph { +impl std::ops::Index> for DataFlowGraph { type Output = Value; - fn index(&self, id: ValueId) -> &Self::Output { + fn index(&self, id: ValueId) -> &Self::Output { &self.values[id.raw()] } } -impl std::ops::Index for DataFlowGraph { - type Output = Value; // The value can still contain unresolved IDs. - fn index(&self, id: ResolvedValueId) -> &Self::Output { +impl std::ops::Index> for DataFlowGraph +where + R: IsResolved, +{ + type Output = Value; + fn index(&self, id: ValueId) -> &Self::Output { &self.values[id.raw()] } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/value.rs b/compiler/noirc_evaluator/src/ssa/ir/value.rs index a2f757b58d2..3654d7d122d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/value.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/value.rs @@ -15,22 +15,27 @@ use super::{ #[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) enum Unresolved {} -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Hash)] +/// Resolved marker; doesn't implement `Hash` so it can't be stored in maps. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize /* PartialOrd, Ord, Hash */)] pub(crate) enum Resolved {} +pub(crate) trait IsResolved {} + +impl IsResolved for Resolved {} + pub(crate) trait Resolution { fn is_resolved() -> bool; } -impl Resolution for Resolved { +impl Resolution for Unresolved { fn is_resolved() -> bool { - true + false } } -impl Resolution for Unresolved { +impl Resolution for R { fn is_resolved() -> bool { - false + true } } @@ -63,7 +68,7 @@ impl ValueId { /// Demote an ID into an unresolved one. pub(crate) fn unresolved(self) -> ValueId { - ValueId::new(Id::new(self.id.to_usize())) + ValueId::new(self.id) } } @@ -109,8 +114,8 @@ impl AsRef> for ValueId { } /// Demote a resolved ID into an unresolved one. -impl From> for ValueId { - fn from(value: ValueId) -> Self { +impl From> for ValueId { + fn from(value: ValueId) -> Self { value.unresolved() } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/array_set.rs b/compiler/noirc_evaluator/src/ssa/opt/array_set.rs index 74381d634e4..0a023b31e58 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/array_set.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/array_set.rs @@ -7,7 +7,7 @@ use crate::ssa::{ function::{Function, RuntimeType}, instruction::{Instruction, InstructionId, TerminatorInstruction}, types::Type::{Array, Slice}, - value::{RawValueId, ResolvedValueId}, + value::{RawValueId, ResolvedValueId, ValueId}, }, ssa_gen::Ssa, }; @@ -55,9 +55,21 @@ impl Function { } } +/// Private resolution type, to limit the scope of storing them in data structures. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(super) enum ContextResolved {} + +//impl IsResolved for ContextResolved {} + +impl From for ValueId { + fn from(value: ResolvedValueId) -> Self { + ValueId::new(value.raw()) + } +} + struct Context<'f> { dfg: &'f DataFlowGraph, - array_to_last_use: HashMap, + array_to_last_use: HashMap, InstructionId>, instructions_that_can_be_made_mutable: HashSet, // Mapping of an array that comes from a load and whether the address // it was loaded from is a reference parameter passed to the block. @@ -86,14 +98,18 @@ impl<'f> Context<'f> { Instruction::ArrayGet { array, .. } => { let array = self.dfg.resolve(*array); - if let Some(existing) = self.array_to_last_use.insert(array, *instruction_id) { + if let Some(existing) = + self.array_to_last_use.insert(array.into(), *instruction_id) + { self.instructions_that_can_be_made_mutable.remove(&existing); } } Instruction::ArraySet { array, .. } => { let array = self.dfg.resolve(*array); - if let Some(existing) = self.array_to_last_use.insert(array, *instruction_id) { + if let Some(existing) = + self.array_to_last_use.insert(array.into(), *instruction_id) + { self.instructions_that_can_be_made_mutable.remove(&existing); } @@ -135,7 +151,7 @@ impl<'f> Context<'f> { let argument = self.dfg.resolve(*argument); if let Some(existing) = - self.array_to_last_use.insert(argument, *instruction_id) + self.array_to_last_use.insert(argument.into(), *instruction_id) { self.instructions_that_can_be_made_mutable.remove(&existing); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 7b7357cfce7..ff08ce6fa7b 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -32,7 +32,7 @@ use crate::ssa::{ function::Function, instruction::{Instruction, InstructionId}, types::Type, - value::{RawValueId, Resolved, ResolvedValueId, Value, ValueId}, + value::{IsResolved, RawValueId, Value, ValueId}, }, ssa_gen::Ssa, }; @@ -103,13 +103,19 @@ struct Context { dom: DominatorTree, } +/// Private resolution type, to limit the scope of storing them in data structures. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +enum ContextResolved {} + +impl IsResolved for ContextResolved {} + /// HashMap from (Instruction, side_effects_enabled_var) to the results of the instruction. /// Stored as a two-level map to avoid cloning Instructions during the `.get` call. /// /// In addition to each result, the original BasicBlockId is stored as well. This allows us /// to deduplicate instructions across blocks as long as the new block dominates the original. type InstructionResultCache = - HashMap, HashMap, ResultCache>>; + HashMap, HashMap, ResultCache>>; /// Records the results of all duplicate [`Instruction`]s along with the blocks in which they sit. /// @@ -209,7 +215,7 @@ impl Context { instruction_id: InstructionId, dfg: &DataFlowGraph, constraint_simplification_mapping: &HashMap, - ) -> Instruction { + ) -> Instruction { let instruction = dfg[instruction_id].clone(); // Alternate between resolving `value_id` in the `dfg` and checking to see if the resolved value @@ -221,11 +227,11 @@ impl Context { dfg: &DataFlowGraph, cache: &HashMap, value_id: ValueId, - ) -> ResolvedValueId { - let resolved_id = dfg.resolve(value_id); - match cache.get(&resolved_id.raw()) { + ) -> ValueId { + let resolved_id = dfg.resolve(value_id).raw(); + match cache.get(&resolved_id) { Some(cached_value) => resolve_cache(dfg, cache, *cached_value), - None => resolved_id, + None => ValueId::new(resolved_id), } } @@ -266,7 +272,7 @@ impl Context { fn cache_instruction( &mut self, - instruction: Instruction, + instruction: Instruction, instruction_results: Vec, dfg: &DataFlowGraph, side_effects_enabled_var: ValueId, @@ -342,7 +348,7 @@ impl Context { fn get_cached( &mut self, dfg: &DataFlowGraph, - instruction: &Instruction, + instruction: &Instruction, side_effects_enabled_var: ValueId, block: BasicBlockId, ) -> Option { diff --git a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 9167070619b..e082b726d0c 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -66,6 +66,7 @@ mod block; use std::collections::{BTreeMap, BTreeSet}; +use block::ContextResolved; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; use crate::ssa::{ @@ -77,7 +78,7 @@ use crate::ssa::{ instruction::{Instruction, InstructionId, TerminatorInstruction}, post_order::PostOrder, types::Type, - value::{RawValueId, ResolvedValueId, ValueId}, + value::{RawValueId, ValueId}, }, ssa_gen::Ssa, }; @@ -348,7 +349,7 @@ impl<'f> PerFunctionContext<'f> { let first = aliases.first(); let first = first.expect("All parameters alias at least themselves or we early return"); - let expression = Expression::Other(first.resolved()); + let expression = Expression::Other(first.resolved().into()); let previous = references.aliases.insert(expression.clone(), aliases.clone()); assert!(previous.is_none()); @@ -396,7 +397,7 @@ impl<'f> PerFunctionContext<'f> { match &self.inserter.function.dfg[instruction] { Instruction::Load { address } => { - let address = self.inserter.function.dfg.resolve(*address); + let address = ValueId::from(self.inserter.function.dfg.resolve(*address)); let result = self.inserter.function.dfg.instruction_results(instruction)[0]; references.remember_dereference(self.inserter.function, address, result); @@ -413,8 +414,8 @@ impl<'f> PerFunctionContext<'f> { } } Instruction::Store { address, value } => { - let address = self.inserter.function.dfg.resolve(*address); - let value = self.inserter.function.dfg.resolve(*value); + let address = self.inserter.function.dfg.resolve(*address).into(); + let value = self.inserter.function.dfg.resolve(*value).into(); // FIXME: This causes errors in the sha256 tests // @@ -443,12 +444,12 @@ impl<'f> PerFunctionContext<'f> { Instruction::Allocate => { // Register the new reference let result = self.inserter.function.dfg.instruction_results(instruction)[0]; - let expr = Expression::Other(result.resolved()); + let expr = Expression::Other(result.resolved().into()); references.expressions.insert(result.raw(), expr.clone()); references.aliases.insert(expr, AliasSet::known(result)); } Instruction::ArrayGet { array, .. } => { - let array = self.inserter.function.dfg.resolve(*array); + let array = self.inserter.function.dfg.resolve(*array).into(); let result = self.inserter.function.dfg.instruction_results(instruction)[0]; references.mark_value_used(array, self.inserter.function); @@ -460,7 +461,7 @@ impl<'f> PerFunctionContext<'f> { } } Instruction::ArraySet { array, value, .. } => { - let array = self.inserter.function.dfg.resolve(*array); + let array = self.inserter.function.dfg.resolve(*array).into(); references.mark_value_used(array, self.inserter.function); let element_type = self.inserter.function.dfg.type_of_value(*value); @@ -507,7 +508,7 @@ impl<'f> PerFunctionContext<'f> { // as a potential alias to the array itself. if Self::contains_references(typ) { let array = self.inserter.function.dfg.instruction_results(instruction)[0]; - let array = self.inserter.function.dfg.resolve(array); + let array = self.inserter.function.dfg.resolve(array).into(); let expr = Expression::ArrayElement(Box::new(Expression::Other(array))); references.expressions.insert(array.raw(), expr.clone()); @@ -533,7 +534,12 @@ impl<'f> PerFunctionContext<'f> { } } - fn set_aliases(&self, references: &mut Block, address: ResolvedValueId, new_aliases: AliasSet) { + fn set_aliases( + &self, + references: &mut Block, + address: ValueId, + new_aliases: AliasSet, + ) { let expression = references.expressions.entry(address.raw()).or_insert(Expression::Other(address)); let aliases = references.aliases.entry(expression.clone()).or_default(); @@ -543,7 +549,7 @@ impl<'f> PerFunctionContext<'f> { fn mark_all_unknown(&self, values: &[ValueId], references: &mut Block) { for value in values { if self.inserter.function.dfg.value_is_reference(*value) { - let value = self.inserter.function.dfg.resolve(*value); + let value = self.inserter.function.dfg.resolve(*value).into(); references.set_unknown(value); references.mark_value_used(value, self.inserter.function); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs b/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs index 65adce9da19..191a803bb9d 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs @@ -3,11 +3,23 @@ use std::borrow::Cow; use crate::ssa::ir::{ function::Function, instruction::{Instruction, InstructionId}, - value::{RawValueId, Resolved, ResolvedValueId, ValueId}, + value::{IsResolved, RawValueId, ResolvedValueId, ValueId}, }; use super::alias_set::AliasSet; +/// Private resolution type, to limit the scope of storing them in data structures. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(super) enum ContextResolved {} + +impl IsResolved for ContextResolved {} + +impl From for ValueId { + fn from(value: ResolvedValueId) -> Self { + ValueId::new(value.raw()) + } +} + /// A `Block` acts as a per-block context for the mem2reg pass. /// Most notably, it contains the current alias set thought to track each /// reference value if known, and it contains the expected ReferenceValue @@ -33,7 +45,7 @@ pub(super) struct Block { pub(super) references: im::OrdMap, /// The last instance of a `Store` instruction to each address in this block - pub(super) last_stores: im::OrdMap, + pub(super) last_stores: im::OrdMap, InstructionId>, } /// An `Expression` here is used to represent a canonical key @@ -43,14 +55,14 @@ pub(super) struct Block { pub(super) enum Expression { Dereference(Box), ArrayElement(Box), - Other(ValueId), + Other(ValueId), } /// Every reference's value is either Known and can be optimized away, or Unknown. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(super) enum ReferenceValue { Unknown, - Known(ResolvedValueId), + Known(ValueId), } impl ReferenceValue { @@ -65,7 +77,10 @@ impl ReferenceValue { impl Block { /// If the given reference id points to a known value, return the value - pub(super) fn get_known_value(&self, address: ResolvedValueId) -> Option { + pub(super) fn get_known_value( + &self, + address: ValueId, + ) -> Option> { if let Some(expression) = self.expressions.get(&address.raw()) { if let Some(aliases) = self.aliases.get(expression) { // We could allow multiple aliases if we check that the reference @@ -82,15 +97,19 @@ impl Block { } /// If the given address is known, set its value to `ReferenceValue::Known(value)`. - pub(super) fn set_known_value(&mut self, address: ResolvedValueId, value: ResolvedValueId) { + pub(super) fn set_known_value( + &mut self, + address: ValueId, + value: ValueId, + ) { self.set_value(address, ReferenceValue::Known(value)); } - pub(super) fn set_unknown(&mut self, address: ResolvedValueId) { + pub(super) fn set_unknown(&mut self, address: ValueId) { self.set_value(address, ReferenceValue::Unknown); } - fn set_value(&mut self, address: ResolvedValueId, value: ReferenceValue) { + fn set_value(&mut self, address: ValueId, value: ReferenceValue) { let expression = self.expressions.entry(address.raw()).or_insert(Expression::Other(address)); let aliases = self.aliases.entry(expression.clone()).or_default(); @@ -152,7 +171,7 @@ impl Block { pub(super) fn remember_dereference( &mut self, function: &Function, - address: ResolvedValueId, + address: ValueId, result: ValueId, ) { if function.dfg.value_is_reference(result) { @@ -171,7 +190,7 @@ impl Block { /// Iterate through each known alias of the given address and apply the function `f` to each. fn for_each_alias_of( &mut self, - address: ResolvedValueId, + address: ValueId, mut f: impl FnMut(&mut Self, ValueId) -> T, ) { if let Some(expr) = self.expressions.get(&address.raw()) { @@ -183,20 +202,20 @@ impl Block { } } - fn keep_last_stores_for(&mut self, address: ResolvedValueId, function: &Function) { + fn keep_last_stores_for(&mut self, address: ValueId, function: &Function) { self.keep_last_store(address, function); self.for_each_alias_of(address, |t, alias| { - t.keep_last_store(function.dfg.resolve(alias), function); + t.keep_last_store(function.dfg.resolve(alias).into(), function); }); } - fn keep_last_store(&mut self, address: ResolvedValueId, function: &Function) { + fn keep_last_store(&mut self, address: ValueId, function: &Function) { if let Some(instruction) = self.last_stores.remove(&address) { // Whenever we decide we want to keep a store instruction, we also need // to go through its stored value and mark that used as well. match &function.dfg[instruction] { Instruction::Store { value, .. } => { - self.mark_value_used(function.dfg.resolve(*value), function); + self.mark_value_used(function.dfg.resolve(*value).into(), function); } other => { unreachable!("last_store held an id of a non-store instruction: {other:?}") @@ -205,14 +224,14 @@ impl Block { } } - pub(super) fn mark_value_used(&mut self, value: ResolvedValueId, function: &Function) { + pub(super) fn mark_value_used(&mut self, value: ValueId, function: &Function) { self.keep_last_stores_for(value, function); // We must do a recursive check for arrays since they're the only Values which may contain // other ValueIds. if let Some((array, _)) = function.dfg.get_array_constant(value) { for value in array { - self.mark_value_used(function.dfg.resolve(value), function); + self.mark_value_used(function.dfg.resolve(value).into(), function); } } }