From 624065f4beb9bf902c9d183f08d414ee567534c2 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 27 Jul 2023 15:33:18 +0000 Subject: [PATCH] fix: avoid non-determinism in defunctionalize --- .../src/ssa_refactor/ir/function.rs | 2 +- .../src/ssa_refactor/ir/types.rs | 4 +-- .../src/ssa_refactor/opt/defunctionalize.rs | 27 +++++++++---------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs index aa4b561638e..ab04c1497bd 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs @@ -141,7 +141,7 @@ impl std::fmt::Display for RuntimeType { /// within Call instructions. pub(crate) type FunctionId = Id; -#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)] pub(crate) struct Signature { pub(crate) params: Vec, pub(crate) returns: Vec, diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs index 9bb90e0b96b..7e37a72ff83 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/types.rs @@ -10,7 +10,7 @@ use iter_extended::vecmap; /// /// Fields do not have a notion of ordering, so this distinction /// is reasonable. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Ord, PartialOrd)] pub(crate) enum NumericType { Signed { bit_size: u32 }, Unsigned { bit_size: u32 }, @@ -18,7 +18,7 @@ pub(crate) enum NumericType { } /// All types representable in the IR. -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Ord, PartialOrd)] pub(crate) enum Type { /// Represents numeric types in the IR, including field elements Numeric(NumericType), diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/defunctionalize.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/defunctionalize.rs index 4ef7625ec80..fc3bc5d9aa6 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/defunctionalize.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/defunctionalize.rs @@ -4,7 +4,7 @@ //! with a non-literal target can be replaced with a call to an apply function. //! The apply function is a dispatch function that takes the function id as a parameter //! and dispatches to the correct target. -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use acvm::FieldElement; use iter_extended::vecmap; @@ -47,7 +47,6 @@ struct ApplyFunction { /// And creating apply functions that dispatch to the correct target by runtime comparisons with constants #[derive(Debug, Clone)] struct DefunctionalizationContext { - fn_to_runtime: HashMap, apply_functions: HashMap, } @@ -57,10 +56,8 @@ impl Ssa { let variants = find_variants(&self); let apply_functions = create_apply_functions(&mut self, variants); - let fn_to_runtime = - self.functions.iter().map(|(func_id, func)| (*func_id, func.runtime())).collect(); - let context = DefunctionalizationContext { fn_to_runtime, apply_functions }; + let context = DefunctionalizationContext { apply_functions }; context.defunctionalize_all(&mut self); self @@ -157,23 +154,23 @@ impl DefunctionalizationContext { } /// Collects all functions used as values that can be called by their signatures -fn find_variants(ssa: &Ssa) -> HashMap> { - let mut dynamic_dispatches: HashSet = HashSet::new(); - let mut functions_as_values: HashSet = HashSet::new(); +fn find_variants(ssa: &Ssa) -> BTreeMap> { + let mut dynamic_dispatches: BTreeSet = BTreeSet::new(); + let mut functions_as_values: BTreeSet = BTreeSet::new(); for function in ssa.functions.values() { functions_as_values.extend(find_functions_as_values(function)); dynamic_dispatches.extend(find_dynamic_dispatches(function)); } - let mut signature_to_functions_as_value: HashMap> = HashMap::new(); + let mut signature_to_functions_as_value: BTreeMap> = BTreeMap::new(); for function_id in functions_as_values { let signature = ssa.functions[&function_id].signature(); signature_to_functions_as_value.entry(signature).or_default().push(function_id); } - let mut variants = HashMap::new(); + let mut variants = BTreeMap::new(); for dispatch_signature in dynamic_dispatches { let mut target_fns = vec![]; @@ -189,8 +186,8 @@ fn find_variants(ssa: &Ssa) -> HashMap> { } /// Finds all literal functions used as values in the given function -fn find_functions_as_values(func: &Function) -> HashSet { - let mut functions_as_values: HashSet = HashSet::new(); +fn find_functions_as_values(func: &Function) -> BTreeSet { + let mut functions_as_values: BTreeSet = BTreeSet::new(); let mut process_value = |value_id: ValueId| { if let Value::Function(id) = func.dfg[value_id] { @@ -220,8 +217,8 @@ fn find_functions_as_values(func: &Function) -> HashSet { } /// Finds all dynamic dispatch signatures in the given function -fn find_dynamic_dispatches(func: &Function) -> HashSet { - let mut dispatches = HashSet::new(); +fn find_dynamic_dispatches(func: &Function) -> BTreeSet { + let mut dispatches = BTreeSet::new(); for block_id in func.reachable_blocks() { let block = &func.dfg[block_id]; @@ -246,7 +243,7 @@ fn find_dynamic_dispatches(func: &Function) -> HashSet { fn create_apply_functions( ssa: &mut Ssa, - variants_map: HashMap>, + variants_map: BTreeMap>, ) -> HashMap { let mut apply_functions = HashMap::new(); for (signature, variants) in variants_map.into_iter() {