Remove Hash from Resolved, add module specific variants

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<FinalResolved>;
+
+impl From<ResolvedValueId> for FinalValueId {
+    fn from(value: ResolvedValueId) -> Self {
+        ValueId::new(value.raw())
+    }
+}
 
 /// Converting an SSA function into Brillig bytecode.
 pub(crate) fn convert_ssa_function(

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<F: AcirField + DebugToString, Registers: RegisterAllocator>(
+pub(super) fn convert_black_box_call<F: AcirField + DebugToString, Registers: RegisterAllocator>(
     brillig_context: &mut BrilligContext<F, Registers>,
     bb_func: &BlackBoxFunc,
     function_arguments: &[BrilligVariable],

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<FieldElement, Stack>,
+    pub(super) brillig_context: &'block mut BrilligContext<FieldElement, Stack>,
     /// 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<InstructionId, HashSet<ResolvedValueId>>,
+    pub(super) last_uses: HashMap<InstructionId, HashSet<FinalValueId>>,
 }
 
 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<FieldElement, Stack>,
         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<R>,
         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")

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<ResolvedValueId>,
+    available_variables: HashSet<FinalValueId>,
 }
 
 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<ResolvedValueId>) -> Self {
+    pub(super) fn new(live_in: HashSet<FinalValueId>) -> 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<BrilligVariable> {
@@ -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<FieldElement, Stack>,
         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<FieldElement, Stack>,
@@ -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<FieldElement, Stack>,
     ) {
@@ -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<F, Registers: RegisterAllocator>(
-    value_id: ResolvedValueId,
+pub(super) fn allocate_value<F, Registers: RegisterAllocator>(
+    value_id: FinalValueId,
     brillig_context: &mut BrilligContext<F, Registers>,
     dfg: &DataFlowGraph,
 ) -> BrilligVariable {

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<ResolvedValueId, BrilligVariable>,
+    pub(super) ssa_value_allocations: HashMap<FinalValueId, BrilligVariable>,
     /// The block ids of the function in reverse post order.
-    pub(crate) blocks: Vec<BasicBlockId>,
+    pub(super) blocks: Vec<BasicBlockId>,
     /// 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();

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<ResolvedValueId, HashMap<BasicBlockId, Vec<InstructionLocation>>>,
-    allocation_points: HashMap<BasicBlockId, HashMap<InstructionLocation, Vec<ResolvedValueId>>>,
+pub(super) struct ConstantAllocation {
+    constant_usage: HashMap<FinalValueId, HashMap<BasicBlockId, Vec<InstructionLocation>>>,
+    allocation_points: HashMap<BasicBlockId, HashMap<InstructionLocation, Vec<FinalValueId>>>,
     dominator_tree: DominatorTree,
     blocks_within_loops: HashSet<BasicBlockId>,
 }
 
 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,25 +44,25 @@ impl ConstantAllocation {
         instance
     }
 
-    pub(crate) fn allocated_in_block(&self, block_id: BasicBlockId) -> Vec<ResolvedValueId> {
+    pub(super) fn allocated_in_block(&self, block_id: BasicBlockId) -> Vec<FinalValueId> {
         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<ResolvedValueId> {
+    ) -> Vec<FinalValueId> {
         self.allocation_points.get(&block_id).map_or(Vec::default(), |allocations| {
             allocations.get(&location).map_or(Vec::default(), |constants| constants.clone())
         })
     }
 
     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 { .. })
 }