Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Replace hashers of hashmaps in dfg with fxhashes #2490

Merged
merged 3 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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