chore: underconstrained check in parallel

Cargo.toml

@@ -154,6 +154,7 @@ color-eyre = "0.6.2"
 rand = "0.8.5"
 proptest = "1.2.0"
 proptest-derive = "0.4.0"
+rayon = "1.8.0"
 
 im = { version = "15.1", features = ["serde"] }
 tracing = "0.1.40"

compiler/noirc_evaluator/Cargo.toml

@@ -26,6 +26,7 @@ serde_json.workspace = true
 serde_with = "3.2.0"
 tracing.workspace = true
 chrono = "0.4.37"
+rayon.workspace = true
 cfg-if.workspace = true
 
 [dev-dependencies]

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -22,7 +22,7 @@ use acvm::{acir::AcirField, FieldElement};
 use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
 use iter_extended::vecmap;
 use num_bigint::BigUint;
-use std::rc::Rc;
+use std::sync::Arc;
 
 use super::brillig_black_box::convert_black_box_call;
 use super::brillig_block_variables::BlockVariables;
@@ -1701,7 +1701,7 @@ impl<'block> BrilligBlock<'block> {
 
     fn initialize_constant_array_runtime(
         &mut self,
-        item_types: Rc<Vec<Type>>,
+        item_types: Arc<Vec<Type>>,
         item_to_repeat: Vec<ValueId>,
         item_count: usize,
         pointer: MemoryAddress,

compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs

@@ -1,34 +1,36 @@
 //! This module defines an SSA pass that detects if the final function has any subgraphs independent from inputs and outputs.
 //! If this is the case, then part of the final circuit can be completely replaced by any other passing circuit, since there are no constraints ensuring connections.
 //! So the compiler informs the developer of this as a bug
-use im::HashMap;
-
 use crate::errors::{InternalBug, SsaReport};
 use crate::ssa::ir::basic_block::BasicBlockId;
 use crate::ssa::ir::function::RuntimeType;
 use crate::ssa::ir::function::{Function, FunctionId};
 use crate::ssa::ir::instruction::{Instruction, InstructionId, Intrinsic};
 use crate::ssa::ir::value::{Value, ValueId};
 use crate::ssa::ssa_gen::Ssa;
+use im::HashMap;
+use rayon::prelude::*;
 use std::collections::{BTreeMap, HashSet};
 
 impl Ssa {
     /// Go through each top-level non-brillig function and detect if it has independent subgraphs
     #[tracing::instrument(level = "trace", skip(self))]
     pub(crate) fn check_for_underconstrained_values(&mut self) -> Vec<SsaReport> {
-        let mut warnings: Vec<SsaReport> = Vec::new();
-        for function in self.functions.values() {
-            match function.runtime() {
-                RuntimeType::Acir { .. } => {
-                    warnings.extend(check_for_underconstrained_values_within_function(
-                        function,
+        let functions_id = self.functions.values().map(|f| f.id().to_usize()).collect::<Vec<_>>();
+        functions_id
+            .iter()
+            .par_bridge()
+            .flat_map(|fid| {
+                let function_to_process = &self.functions[&FunctionId::new(*fid)];
+                match function_to_process.runtime() {
+                    RuntimeType::Acir { .. } => check_for_underconstrained_values_within_function(
+                        function_to_process,
                         &self.functions,
-                    ));
+                    ),
+                    RuntimeType::Brillig => Vec::new(),
                 }
-                RuntimeType::Brillig => (),
-            }
-        }
-        warnings
+            })
+            .collect()
     }
 }
 
@@ -88,9 +90,8 @@ impl Context {
             self.visited_blocks.insert(block);
             self.connect_value_ids_in_block(function, block, all_functions);
         }
-
         // Merge ValueIds into sets, where each original small set of ValueIds is merged with another set if they intersect
-        self.merge_sets();
+        self.value_sets = Self::merge_sets_par(&self.value_sets);
     }
 
     /// Find sets that contain input or output value of the function
@@ -267,14 +268,13 @@ impl Context {
     /// Merge all small sets into larger ones based on whether the sets intersect or not
     ///
     /// If two small sets have a common ValueId, we merge them into one
-    fn merge_sets(&mut self) {
+    fn merge_sets(current: &[HashSet<ValueId>]) -> Vec<HashSet<ValueId>> {
         let mut new_set_id: usize = 0;
         let mut updated_sets: HashMap<usize, HashSet<ValueId>> = HashMap::new();
         let mut value_dictionary: HashMap<ValueId, usize> = HashMap::new();
         let mut parsed_value_set: HashSet<ValueId> = HashSet::new();
 
-        // Go through each set
-        for set in self.value_sets.iter() {
+        for set in current.iter() {
             // Check if the set has any of the ValueIds we've encountered at previous iterations
             let intersection: HashSet<ValueId> =
                 set.intersection(&parsed_value_set).copied().collect();
@@ -327,7 +327,26 @@ impl Context {
             }
             updated_sets.insert(largest_set_index, largest_set);
         }
-        self.value_sets = updated_sets.values().cloned().collect();
+        updated_sets.values().cloned().collect()
+    }
+
+    /// Parallel version of merge_sets
+    /// The sets are merged by chunks, and then the chunks are merged together
+    fn merge_sets_par(sets: &[HashSet<ValueId>]) -> Vec<HashSet<ValueId>> {
+        let mut sets = sets.to_owned();
+        let mut len = sets.len();
+        let mut prev_len = len + 1;
+
+        while len > 1000 && len < prev_len {
+            sets = sets.par_chunks(1000).flat_map(Self::merge_sets).collect();
+
+            prev_len = len;
+            len = sets.len();
+        }
+        // TODO: if prev_len >= len, this means we cannot effectively merge the sets anymore
+        // We should instead partition the sets into disjoint chunks and work on those chunks,
+        // but for now we fallback to the non-parallel implementation
+        Self::merge_sets(&sets)
     }
 }
 #[cfg(test)]

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

@@ -1,5 +1,4 @@
-use std::collections::BTreeMap;
-use std::rc::Rc;
+use std::{collections::BTreeMap, sync::Arc};
 
 use crate::ssa::ir::{types::Type, value::ValueId};
 use acvm::FieldElement;
@@ -155,8 +154,8 @@ impl FunctionBuilder {
         let len = databus.values.len();
 
         let array = if len > 0 {
-            let array =
-                self.array_constant(databus.values, Type::Array(Rc::new(vec![Type::field()]), len));
+            let array = self
+                .array_constant(databus.values, Type::Array(Arc::new(vec![Type::field()]), len));
             Some(array)
         } else {
             None

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

@@ -1,6 +1,6 @@
 pub(crate) mod data_bus;
 
-use std::{borrow::Cow, collections::BTreeMap, rc::Rc};
+use std::{borrow::Cow, collections::BTreeMap, sync::Arc};
 
 use acvm::{acir::circuit::ErrorSelector, FieldElement};
 use noirc_errors::Location;
@@ -189,7 +189,7 @@ impl FunctionBuilder {
     /// given amount of field elements. Returns the result of the allocate instruction,
     /// which is always a Reference to the allocated data.
     pub(crate) fn insert_allocate(&mut self, element_type: Type) -> ValueId {
-        let reference_type = Type::Reference(Rc::new(element_type));
+        let reference_type = Type::Reference(Arc::new(element_type));
         self.insert_instruction(Instruction::Allocate, Some(vec![reference_type])).first()
     }
 
@@ -516,7 +516,7 @@ impl std::ops::Index<BasicBlockId> for FunctionBuilder {
 
 #[cfg(test)]
 mod tests {
-    use std::rc::Rc;
+    use std::sync::Arc;
 
     use acvm::{acir::AcirField, FieldElement};
 
@@ -542,7 +542,7 @@ mod tests {
         let to_bits_id = builder.import_intrinsic_id(Intrinsic::ToBits(Endian::Little));
         let input = builder.numeric_constant(FieldElement::from(7_u128), Type::field());
         let length = builder.numeric_constant(FieldElement::from(8_u128), Type::field());
-        let result_types = vec![Type::Array(Rc::new(vec![Type::bool()]), 8)];
+        let result_types = vec![Type::Array(Arc::new(vec![Type::bool()]), 8)];
         let call_results =
             builder.insert_call(to_bits_id, vec![input, length], result_types).into_owned();
 

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

@@ -1,5 +1,5 @@
 use fxhash::FxHashMap as HashMap;
-use std::{collections::VecDeque, rc::Rc};
+use std::{collections::VecDeque, sync::Arc};
 
 use acvm::{
     acir::{AcirField, BlackBoxFunc},
@@ -561,7 +561,7 @@ fn simplify_black_box_func(
 fn make_constant_array(dfg: &mut DataFlowGraph, results: Vec<FieldElement>, typ: Type) -> ValueId {
     let result_constants = vecmap(results, |element| dfg.make_constant(element, typ.clone()));
 
-    let typ = Type::Array(Rc::new(vec![typ]), result_constants.len());
+    let typ = Type::Array(Arc::new(vec![typ]), result_constants.len());
     dfg.make_array(result_constants.into(), typ)
 }
 
@@ -572,7 +572,7 @@ fn make_constant_slice(
 ) -> (ValueId, ValueId) {
     let result_constants = vecmap(results, |element| dfg.make_constant(element, typ.clone()));
 
-    let typ = Type::Slice(Rc::new(vec![typ]));
+    let typ = Type::Slice(Arc::new(vec![typ]));
     let length = FieldElement::from(result_constants.len() as u128);
     (dfg.make_constant(length, Type::length_type()), dfg.make_array(result_constants.into(), typ))
 }

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

@@ -1,4 +1,4 @@
-use std::rc::Rc;
+use std::sync::Arc;
 
 use acvm::{acir::AcirField, BlackBoxFunctionSolver, BlackBoxResolutionError, FieldElement};
 use iter_extended::vecmap;
@@ -45,7 +45,7 @@ pub(super) fn simplify_ec_add(
             let result_y = dfg.make_constant(result_y, Type::field());
             let result_is_infinity = dfg.make_constant(result_is_infinity, Type::bool());
 
-            let typ = Type::Array(Rc::new(vec![Type::field()]), 3);
+            let typ = Type::Array(Arc::new(vec![Type::field()]), 3);
             let result_array =
                 dfg.make_array(im::vector![result_x, result_y, result_is_infinity], typ);
 

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

@@ -27,7 +27,7 @@
     /// Constructs a new Id for the given index.
     /// This constructor is deliberately private to prevent
     /// constructing invalid IDs.
-    fn new(index: usize) -> Self {
+    pub(crate) fn new(index: usize) -> Self {
         Self { index, _marker: std::marker::PhantomData }
     }
 
@@ -294,7 +294,7 @@
 }
 
 /// A TwoWayMap is a map from both key to value and value to key.
 /// This is accomplished by keeping the map bijective - for every
 /// value there is exactly one key and vice-versa. Any duplicate values
 /// are prevented in the call to insert.
 #[derive(Debug)]

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

@@ -1,5 +1,5 @@
 use serde::{Deserialize, Serialize};
-use std::rc::Rc;
+use std::sync::Arc;
 
 use acvm::{acir::AcirField, FieldElement};
 use iter_extended::vecmap;
@@ -72,13 +72,13 @@ pub(crate) enum Type {
     Numeric(NumericType),
 
     /// A reference to some value, such as an array
-    Reference(Rc<Type>),
+    Reference(Arc<Type>),
 
     /// An immutable array value with the given element type and length
-    Array(Rc<CompositeType>, usize),
+    Array(Arc<CompositeType>, usize),
 
     /// An immutable slice value with a given element type
-    Slice(Rc<CompositeType>),
+    Slice(Arc<CompositeType>),
 
     /// A function that may be called directly
     Function,
@@ -112,7 +112,7 @@ impl Type {
 
     /// Creates the str<N> type, of the given length N
     pub(crate) fn str(length: usize) -> Type {
-        Type::Array(Rc::new(vec![Type::char()]), length)
+        Type::Array(Arc::new(vec![Type::char()]), length)
     }
 
     /// Creates the native field type.
@@ -190,7 +190,7 @@ impl Type {
         }
     }
 
-    pub(crate) fn element_types(self) -> Rc<Vec<Type>> {
+    pub(crate) fn element_types(self) -> Arc<Vec<Type>> {
         match self {
             Type::Array(element_types, _) | Type::Slice(element_types) => element_types,
             other => panic!("element_types: Expected array or slice, found {other}"),

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

@@ -311,7 +311,7 @@ impl Context {
 
 #[cfg(test)]
 mod test {
-    use std::rc::Rc;
+    use std::sync::Arc;
 
     use crate::ssa::{
         function_builder::FunctionBuilder,
@@ -509,7 +509,7 @@ mod test {
         let one = builder.field_constant(1u128);
         let v1 = builder.insert_binary(v0, BinaryOp::Add, one);
 
-        let array_type = Type::Array(Rc::new(vec![Type::field()]), 1);
+        let array_type = Type::Array(Arc::new(vec![Type::field()]), 1);
         let arr = builder.current_function.dfg.make_array(vec![v1].into(), array_type);
         builder.terminate_with_return(vec![arr]);
 
@@ -601,7 +601,7 @@ mod test {
         // Compiling main
         let mut builder = FunctionBuilder::new("main".into(), main_id);
 
-        let v0 = builder.add_parameter(Type::Array(Rc::new(vec![Type::field()]), 4));
+        let v0 = builder.add_parameter(Type::Array(Arc::new(vec![Type::field()]), 4));
         let v1 = builder.add_parameter(Type::unsigned(32));
         let v2 = builder.add_parameter(Type::unsigned(1));
         let v3 = builder.add_parameter(Type::unsigned(1));
@@ -737,7 +737,7 @@ mod test {
         let zero = builder.field_constant(0u128);
         let one = builder.field_constant(1u128);
 
-        let typ = Type::Array(Rc::new(vec![Type::field()]), 2);
+        let typ = Type::Array(Arc::new(vec![Type::field()]), 2);
         let array = builder.array_constant(vec![zero, one].into(), typ);
 
         let _v2 = builder.insert_array_get(array, v1, Type::field());
@@ -787,7 +787,7 @@ mod test {
 
         let v0 = builder.add_parameter(Type::bool());
         let v1 = builder.add_parameter(Type::bool());
-        let v2 = builder.add_parameter(Type::Array(Rc::new(vec![Type::field()]), 2));
+        let v2 = builder.add_parameter(Type::Array(Arc::new(vec![Type::field()]), 2));
 
         let zero = builder.numeric_constant(0u128, Type::length_type());
         let one = builder.numeric_constant(1u128, Type::length_type());

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

@@ -878,7 +878,7 @@ impl<'f> Context<'f> {
 
 #[cfg(test)]
 mod test {
-    use std::rc::Rc;
+    use std::sync::Arc;
 
     use acvm::acir::AcirField;
 
@@ -1016,7 +1016,7 @@ mod test {
         let b2 = builder.insert_block();
 
         let v0 = builder.add_parameter(Type::bool());
-        let v1 = builder.add_parameter(Type::Reference(Rc::new(Type::field())));
+        let v1 = builder.add_parameter(Type::Reference(Arc::new(Type::field())));
 
         builder.terminate_with_jmpif(v0, b1, b2);
 
@@ -1078,7 +1078,7 @@ mod test {
         let b3 = builder.insert_block();
 
         let v0 = builder.add_parameter(Type::bool());
-        let v1 = builder.add_parameter(Type::Reference(Rc::new(Type::field())));
+        let v1 = builder.add_parameter(Type::Reference(Arc::new(Type::field())));
 
         builder.terminate_with_jmpif(v0, b1, b2);
 
@@ -1477,7 +1477,7 @@ mod test {
         let b2 = builder.insert_block();
         let b3 = builder.insert_block();
 
-        let element_type = Rc::new(vec![Type::unsigned(8)]);
+        let element_type = Arc::new(vec![Type::unsigned(8)]);
         let array_type = Type::Array(element_type.clone(), 2);
         let array = builder.add_parameter(array_type);