Skip to content

Commit

Permalink
chore: Replace hashers of hashmaps in dfg with fxhashes (#2490)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ethan-000 authored Sep 5, 2023
1 parent f7dc943 commit 255febd
Show file tree
Hide file tree
Showing 20 changed files with 57 additions and 49 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/noirc_evaluator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ noirc_frontend.workspace = true
noirc_errors.workspace = true
noirc_abi.workspace = true
acvm.workspace = true
fxhash = "0.2.1"
iter-extended.workspace = true
thiserror.workspace = true
num-bigint = "0.4"
Expand Down
13 changes: 6 additions & 7 deletions crates/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,21 @@ pub(crate) mod brillig_directive;
pub(crate) mod brillig_fn;
pub(crate) mod brillig_slice_ops;

use crate::ssa::ir::{function::Function, post_order::PostOrder};

use std::collections::HashMap;

use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext};

use super::brillig_ir::{artifact::BrilligArtifact, BrilligContext};
use crate::ssa::ir::{function::Function, post_order::PostOrder};
use fxhash::FxHashMap as HashMap;

/// Converting an SSA function into Brillig bytecode.
pub(crate) fn convert_ssa_function(func: &Function, enable_debug_trace: bool) -> BrilligArtifact {
let mut reverse_post_order = Vec::new();
reverse_post_order.extend_from_slice(PostOrder::with_function(func).as_slice());
reverse_post_order.reverse();

let mut function_context =
FunctionContext { function_id: func.id(), ssa_value_to_brillig_variable: HashMap::new() };
let mut function_context = FunctionContext {
function_id: func.id(),
ssa_value_to_brillig_variable: HashMap::default(),
};

let mut brillig_context = BrilligContext::new(enable_debug_trace);

Expand Down
3 changes: 1 addition & 2 deletions crates/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::collections::HashMap;

use acvm::brillig_vm::brillig::{HeapArray, HeapVector, RegisterIndex, RegisterOrMemory};
use iter_extended::vecmap;

Expand All @@ -15,6 +13,7 @@ use crate::{
value::ValueId,
},
};
use fxhash::FxHashMap as HashMap;

pub(crate) struct FunctionContext {
pub(crate) function_id: FunctionId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,6 @@ impl<'block> BrilligBlock<'block> {

#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::vec;

use acvm::acir::brillig::{HeapVector, Value};
Expand All @@ -323,11 +322,12 @@ mod tests {
use crate::brillig::brillig_ir::tests::{create_and_run_vm, create_context};
use crate::brillig::brillig_ir::BrilligContext;
use crate::ssa::ir::map::Id;
use fxhash::FxHashMap as HashMap;

fn create_test_environment() -> (FunctionContext, BrilligContext) {
let function_context = FunctionContext {
function_id: Id::test_new(0),
ssa_value_to_brillig_variable: HashMap::new(),
ssa_value_to_brillig_variable: HashMap::default(),
};
let brillig_context = create_context();
(function_context, brillig_context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use acvm::{
FieldElement,
};
use acvm::{BlackBoxFunctionSolver, BlackBoxResolutionError};
use fxhash::FxHashMap as HashMap;
use iter_extended::{try_vecmap, vecmap};
use std::collections::HashMap;
use std::{borrow::Cow, hash::Hash};

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down
9 changes: 5 additions & 4 deletions crates/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! This file holds the pass to convert from Noir's SSA IR to ACIR.
mod acir_ir;

use std::collections::{HashMap, HashSet};
use std::collections::HashSet;
use std::fmt::Debug;
use std::ops::RangeInclusive;

Expand Down Expand Up @@ -29,6 +29,7 @@ use acvm::{
acir::{circuit::opcodes::BlockId, native_types::Expression},
FieldElement,
};
use fxhash::FxHashMap as HashMap;
use iter_extended::{try_vecmap, vecmap};
use noirc_frontend::Distinctness;

Expand Down Expand Up @@ -147,11 +148,11 @@ impl Context {
let current_side_effects_enabled_var = acir_context.add_constant(FieldElement::one());

Context {
ssa_values: HashMap::new(),
ssa_values: HashMap::default(),
current_side_effects_enabled_var,
acir_context,
initialized_arrays: HashSet::new(),
memory_blocks: HashMap::new(),
memory_blocks: HashMap::default(),
max_block_id: 0,
}
}
Expand Down Expand Up @@ -1227,7 +1228,7 @@ mod tests {
let ssa = builder.finish();

let context = Context::new();
let mut acir = context.convert_ssa(ssa, Brillig::default(), &HashMap::new()).unwrap();
let mut acir = context.convert_ssa(ssa, Brillig::default(), &HashMap::default()).unwrap();

let expected_opcodes =
vec![Opcode::Arithmetic(&Expression::one() - &Expression::from(Witness(1)))];
Expand Down
6 changes: 4 additions & 2 deletions crates/noirc_evaluator/src/ssa/ir/cfg.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use std::collections::{BTreeSet, HashMap};
use std::collections::BTreeSet;

use super::{
basic_block::{BasicBlock, BasicBlockId},
function::Function,
};
use fxhash::FxHashMap as HashMap;

/// A container for the successors and predecessors of some Block.
#[derive(Clone, Default)]
Expand Down Expand Up @@ -33,7 +34,8 @@ impl ControlFlowGraph {
// it later comes to describe any edges after calling compute.
let entry_block = func.entry_block();
let empty_node = CfgNode { predecessors: BTreeSet::new(), successors: BTreeSet::new() };
let data = HashMap::from([(entry_block, empty_node)]);
let mut data = HashMap::default();
data.insert(entry_block, empty_node);

let mut cfg = ControlFlowGraph { data };
cfg.compute(func);
Expand Down
3 changes: 2 additions & 1 deletion crates/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow::Cow, collections::HashMap};
use std::borrow::Cow;

use crate::ssa::ir::instruction::SimplifyResult;

Expand All @@ -14,6 +14,7 @@ use super::{
};

use acvm::FieldElement;
use fxhash::FxHashMap as HashMap;
use iter_extended::vecmap;
use noirc_errors::Location;

Expand Down
5 changes: 3 additions & 2 deletions crates/noirc_evaluator/src/ssa/ir/dom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
//! Dominator trees are useful for tasks such as identifying back-edges in loop analysis or
//! calculating dominance frontiers.
use std::{cmp::Ordering, collections::HashMap};
use std::cmp::Ordering;

use super::{
basic_block::BasicBlockId, cfg::ControlFlowGraph, function::Function, post_order::PostOrder,
};
use fxhash::FxHashMap as HashMap;

/// Dominator tree node. We keep one of these per reachable block.
#[derive(Clone, Default)]
Expand Down Expand Up @@ -121,7 +122,7 @@ impl DominatorTree {
/// Allocate and compute a dominator tree from a pre-computed control flow graph and
/// post-order counterpart.
pub(crate) fn with_cfg_and_post_order(cfg: &ControlFlowGraph, post_order: &PostOrder) -> Self {
let mut dom_tree = DominatorTree { nodes: HashMap::new(), cache: HashMap::new() };
let mut dom_tree = DominatorTree { nodes: HashMap::default(), cache: HashMap::default() };
dom_tree.compute_dominator_tree(cfg, post_order);
dom_tree
}
Expand Down
5 changes: 2 additions & 3 deletions crates/noirc_evaluator/src/ssa/ir/function_inserter.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::collections::HashMap;

use iter_extended::vecmap;

use super::{
Expand All @@ -9,6 +7,7 @@ use super::{
instruction::{Instruction, InstructionId},
value::ValueId,
};
use fxhash::FxHashMap as HashMap;

/// The FunctionInserter can be used to help modify existing Functions
/// and map old values to new values after re-inserting optimized versions
Expand All @@ -21,7 +20,7 @@ pub(crate) struct FunctionInserter<'f> {

impl<'f> FunctionInserter<'f> {
pub(crate) fn new(function: &'f mut Function) -> FunctionInserter<'f> {
Self { function, values: HashMap::new() }
Self { function, values: HashMap::default() }
}

/// Resolves a ValueId to its new, updated value.
Expand Down
6 changes: 3 additions & 3 deletions crates/noirc_evaluator/src/ssa/ir/map.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use fxhash::FxHashMap as HashMap;
use std::{
collections::HashMap,
hash::Hash,
sync::atomic::{AtomicUsize, Ordering},
};
Expand Down Expand Up @@ -218,7 +218,7 @@ impl<T> SparseMap<T> {

impl<T> Default for SparseMap<T> {
fn default() -> Self {
Self { storage: HashMap::new() }
Self { storage: HashMap::default() }
}
}

Expand Down Expand Up @@ -272,7 +272,7 @@ impl<K: Clone + Eq + Hash, V: Clone + Hash + Eq> TwoWayMap<K, V> {

impl<K, V> Default for TwoWayMap<K, V> {
fn default() -> Self {
Self { key_to_value: HashMap::new(), value_to_key: HashMap::new() }
Self { key_to_value: HashMap::default(), value_to_key: HashMap::default() }
}
}

Expand Down
5 changes: 2 additions & 3 deletions crates/noirc_evaluator/src/ssa/opt/array_use.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::collections::HashMap;

use crate::ssa::{
ir::{
basic_block::BasicBlockId,
Expand All @@ -10,13 +8,14 @@ use crate::ssa::{
},
ssa_gen::Ssa,
};
use fxhash::FxHashMap as HashMap;

impl Ssa {
/// Map arrays with the last instruction that uses it
/// For this we simply process all the instructions in execution order
/// and update the map whenever there is a match
pub(crate) fn find_last_array_uses(&self) -> HashMap<ValueId, InstructionId> {
let mut array_use = HashMap::new();
let mut array_use = HashMap::default();
for func in self.functions.values() {
let mut reverse_post_order = PostOrder::with_function(func).into_vec();
reverse_post_order.reverse();
Expand Down
5 changes: 3 additions & 2 deletions crates/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::{HashMap, HashSet};
use std::collections::HashSet;

use iter_extended::vecmap;

Expand All @@ -12,6 +12,7 @@ use crate::ssa::{
},
ssa_gen::Ssa,
};
use fxhash::FxHashMap as HashMap;

impl Ssa {
/// Performs constant folding on each instruction.
Expand Down Expand Up @@ -56,7 +57,7 @@ impl Context {
let instructions = function.dfg[block].take_instructions();

// Cache of instructions without any side-effects along with their outputs.
let mut cached_instruction_results: HashMap<Instruction, Vec<ValueId>> = HashMap::new();
let mut cached_instruction_results: HashMap<Instruction, Vec<ValueId>> = HashMap::default();

for instruction_id in instructions {
self.push_instruction(function, block, instruction_id, &mut cached_instruction_results);
Expand Down
5 changes: 3 additions & 2 deletions crates/noirc_evaluator/src/ssa/opt/defunctionalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{BTreeMap, BTreeSet, HashMap, HashSet};
use std::collections::{BTreeMap, BTreeSet, HashSet};

use acvm::FieldElement;
use iter_extended::vecmap;
Expand All @@ -20,6 +20,7 @@ use crate::ssa::{
},
ssa_gen::Ssa,
};
use fxhash::FxHashMap as HashMap;

/// Represents an 'apply' function created by this pass to dispatch higher order functions to.
/// Pseudocode of an `apply` function is given below:
Expand Down Expand Up @@ -245,7 +246,7 @@ fn create_apply_functions(
ssa: &mut Ssa,
variants_map: BTreeMap<Signature, Vec<FunctionId>>,
) -> HashMap<Signature, ApplyFunction> {
let mut apply_functions = HashMap::new();
let mut apply_functions = HashMap::default();
for (signature, variants) in variants_map.into_iter() {
assert!(
!variants.is_empty(),
Expand Down
7 changes: 4 additions & 3 deletions crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@
//! v11 = mul v4, Field 12
//! v12 = add v10, v11
//! store v12 at v5 (new store)
use std::collections::{BTreeMap, HashMap, HashSet};
use fxhash::FxHashMap as HashMap;
use std::collections::{BTreeMap, HashSet};

use acvm::FieldElement;
use iter_extended::vecmap;
Expand Down Expand Up @@ -218,7 +219,7 @@ fn flatten_function_cfg(function: &mut Function) {
let mut context = Context {
inserter: FunctionInserter::new(function),
cfg,
store_values: HashMap::new(),
store_values: HashMap::default(),
local_allocations: HashSet::new(),
branch_ends,
conditions: Vec::new(),
Expand Down Expand Up @@ -587,7 +588,7 @@ impl<'f> Context<'f> {
// args that will be merged by inline_branch_end. Since jmpifs don't have
// block arguments, it is safe to use the jmpif block here.
last_block: jmpif_block,
store_values: HashMap::new(),
store_values: HashMap::default(),
local_allocations: HashSet::new(),
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
//!
//! This algorithm will remember each join point found in `find_join_point_of_branches` and
//! the resulting map from each split block to each join block is returned.
use std::collections::HashMap;
use crate::ssa::ir::{basic_block::BasicBlockId, cfg::ControlFlowGraph, function::Function};
use fxhash::FxHashMap as HashMap;

/// Returns a `HashMap` mapping blocks that start a branch (i.e. blocks terminated with jmpif) to
/// their corresponding blocks that end the branch.
Expand Down Expand Up @@ -61,7 +61,7 @@ struct Context<'cfg> {

impl<'cfg> Context<'cfg> {
fn new(cfg: &'cfg ControlFlowGraph) -> Self {
Self { cfg, branch_ends: HashMap::new() }
Self { cfg, branch_ends: HashMap::default() }
}

fn find_join_point_of_branches(
Expand Down
9 changes: 5 additions & 4 deletions crates/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//! The purpose of this pass is to inline the instructions of each function call
//! within the function caller. If all function calls are known, there will only
//! be a single function remaining when the pass finishes.
use std::collections::{BTreeSet, HashMap, HashSet};
use std::collections::{BTreeSet, HashSet};

use iter_extended::{btree_map, vecmap};

Expand All @@ -17,6 +17,7 @@ use crate::ssa::{
},
ssa_gen::Ssa,
};
use fxhash::FxHashMap as HashMap;

/// An arbitrary limit to the maximum number of recursive call
/// frames at any point in time.
Expand Down Expand Up @@ -189,9 +190,9 @@ impl<'function> PerFunctionContext<'function> {
Self {
context,
source_function,
blocks: HashMap::new(),
instructions: HashMap::new(),
values: HashMap::new(),
blocks: HashMap::default(),
instructions: HashMap::default(),
values: HashMap::default(),
inlining_entry: false,
}
}
Expand Down
Loading

0 comments on commit 255febd

Please sign in to comment.