From 203242c0c05e9333caaa8df55a4ed9a02e000882 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Sat, 4 Jan 2025 06:04:45 -0500 Subject: [PATCH] fix: Non-determinism from under constrained checks (#6945) --- .../check_for_underconstrained_values.rs | 38 +++++++++---------- tooling/nargo_cli/build.rs | 2 +- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 106b01833cd..d61dd27d02a 100644 --- a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -10,7 +10,7 @@ use crate::ssa::ir::value::{Value, ValueId}; use crate::ssa::ssa_gen::Ssa; use im::HashMap; use rayon::prelude::*; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use tracing::trace; impl Ssa { @@ -73,7 +73,7 @@ fn check_for_underconstrained_values_within_function( context.compute_sets_of_connected_value_ids(function, all_functions); - let all_brillig_generated_values: HashSet = + let all_brillig_generated_values: BTreeSet = context.brillig_return_to_argument.keys().copied().collect(); let connected_sets_indices = @@ -81,7 +81,7 @@ fn check_for_underconstrained_values_within_function( // Go through each disconnected set, find brillig calls that caused it and form warnings for set_index in - HashSet::from_iter(0..(context.value_sets.len())).difference(&connected_sets_indices) + BTreeSet::from_iter(0..(context.value_sets.len())).difference(&connected_sets_indices) { let current_set = &context.value_sets[*set_index]; warnings.append(&mut context.find_disconnecting_brillig_calls_with_results_in_set( @@ -104,7 +104,7 @@ struct DependencyContext { array_elements: HashMap, // Map of brillig call ids to sets of the value ids descending // from their arguments and results - tainted: HashMap, + tainted: BTreeMap, } /// Structure keeping track of value ids descending from Brillig calls' @@ -434,7 +434,7 @@ impl DependencyContext { struct Context { visited_blocks: HashSet, block_queue: Vec, - value_sets: Vec>, + value_sets: Vec>, brillig_return_to_argument: HashMap>, brillig_return_to_instruction_id: HashMap, } @@ -467,7 +467,7 @@ impl Context { fn find_sets_connected_to_function_inputs_or_outputs( &mut self, function: &Function, - ) -> HashSet { + ) -> BTreeSet { let variable_parameters_and_return_values = function .parameters() .iter() @@ -475,7 +475,7 @@ impl Context { .filter(|id| function.dfg.get_numeric_constant(**id).is_none()) .map(|value_id| function.dfg.resolve(*value_id)); - let mut connected_sets_indices: HashSet = HashSet::new(); + let mut connected_sets_indices: BTreeSet = BTreeSet::default(); // Go through each parameter and each set and check if the set contains the parameter // If it's the case, then that set doesn't present an issue @@ -492,8 +492,8 @@ impl Context { /// Find which Brillig calls separate this set from others and return bug warnings about them fn find_disconnecting_brillig_calls_with_results_in_set( &self, - current_set: &HashSet, - all_brillig_generated_values: &HashSet, + current_set: &BTreeSet, + all_brillig_generated_values: &BTreeSet, function: &Function, ) -> Vec { let mut warnings = Vec::new(); @@ -503,7 +503,7 @@ impl Context { // Go through all Brillig outputs in the set for brillig_output_in_set in intersection { // Get the inputs that correspond to the output - let inputs: HashSet = + let inputs: BTreeSet = self.brillig_return_to_argument[&brillig_output_in_set].iter().copied().collect(); // Check if any of them are not in the set @@ -532,7 +532,7 @@ impl Context { let instructions = function.dfg[block].instructions(); for instruction in instructions.iter() { - let mut instruction_arguments_and_results = HashSet::new(); + let mut instruction_arguments_and_results = BTreeSet::new(); // Insert non-constant instruction arguments function.dfg[*instruction].for_each_value(|value_id| { @@ -638,15 +638,15 @@ 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(current: &[HashSet]) -> Vec> { + fn merge_sets(current: &[BTreeSet]) -> Vec> { let mut new_set_id: usize = 0; - let mut updated_sets: HashMap> = HashMap::new(); - let mut value_dictionary: HashMap = HashMap::new(); - let mut parsed_value_set: HashSet = HashSet::new(); + let mut updated_sets: BTreeMap> = BTreeMap::default(); + let mut value_dictionary: HashMap = HashMap::default(); + let mut parsed_value_set: BTreeSet = BTreeSet::default(); for set in current.iter() { // Check if the set has any of the ValueIds we've encountered at previous iterations - let intersection: HashSet = + let intersection: BTreeSet = set.intersection(&parsed_value_set).copied().collect(); parsed_value_set.extend(set.iter()); @@ -663,7 +663,7 @@ impl Context { } // If there is an intersection, we have to join the sets - let mut joining_sets_ids: HashSet = + let mut joining_sets_ids: BTreeSet = intersection.iter().map(|x| value_dictionary[x]).collect(); let mut largest_set_size = usize::MIN; let mut largest_set_index = usize::MAX; @@ -677,7 +677,7 @@ impl Context { joining_sets_ids.remove(&largest_set_index); let mut largest_set = - updated_sets.extract(&largest_set_index).expect("Set should be in the hashmap").0; + updated_sets.remove(&largest_set_index).expect("Set should be in the hashmap"); // For each of other sets that need to be joined for set_id in joining_sets_ids.iter() { @@ -702,7 +702,7 @@ impl Context { /// Parallel version of merge_sets /// The sets are merged by chunks, and then the chunks are merged together - fn merge_sets_par(sets: &[HashSet]) -> Vec> { + fn merge_sets_par(sets: &[BTreeSet]) -> Vec> { let mut sets = sets.to_owned(); let mut len = sets.len(); let mut prev_len = len + 1; diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index 17ce6d4dbfd..d041cf3e53f 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -208,6 +208,7 @@ fn test_{test_name}(force_brillig: ForceBrillig, inliner_aggressiveness: Inliner nargo.arg("--program-dir").arg(test_program_dir); nargo.arg("{test_command}").arg("--force"); nargo.arg("--inliner-aggressiveness").arg(inliner_aggressiveness.0.to_string()); + nargo.arg("--check-non-determinism"); if force_brillig.0 {{ nargo.arg("--force-brillig"); @@ -217,7 +218,6 @@ fn test_{test_name}(force_brillig: ForceBrillig, inliner_aggressiveness: Inliner nargo.arg("50"); // Check whether the test case is non-deterministic - nargo.arg("--check-non-determinism"); }} {test_content}