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: delete a bunch of dead code from noirc_evaluator #6939

Merged
merged 3 commits into from
Jan 3, 2025
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
3 changes: 0 additions & 3 deletions compiler/noirc_evaluator/src/acir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,6 @@ impl From<NumericType> for AcirType {
pub(crate) struct AcirContext<F: AcirField, B: BlackBoxFunctionSolver<F>> {
blackbox_solver: B,

/// Two-way map that links `AcirVar` to `AcirVarData`.
///
/// The vars object is an instance of the `TwoWayMap`, which provides a bidirectional mapping between `AcirVar` and `AcirVarData`.
vars: HashMap<AcirVar, AcirVarData<F>>,

constant_witnesses: HashMap<F, Witness>,
Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
//! elimination and constant folding.
//!
//! This module heavily borrows from Cranelift
#![allow(dead_code)]

use std::{
collections::{BTreeMap, BTreeSet},
Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ impl FunctionBuilder {
}

/// Insert a numeric constant into the current function of type Field
#[cfg(test)]
pub(crate) fn field_constant(&mut self, value: impl Into<FieldElement>) -> ValueId {
self.numeric_constant(value.into(), NumericType::NativeField)
}
Expand Down Expand Up @@ -328,6 +329,7 @@ impl FunctionBuilder {
.first()
}

#[cfg(test)]
pub(crate) fn insert_mutable_array_set(
&mut self,
array: ValueId,
Expand Down
22 changes: 0 additions & 22 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use super::{
use acvm::{acir::AcirField, FieldElement};
use fxhash::FxHashMap as HashMap;
use iter_extended::vecmap;
use noirc_errors::Location;
use serde::{Deserialize, Serialize};
use serde_with::serde_as;
use serde_with::DisplayFromStr;
Expand Down Expand Up @@ -247,12 +246,6 @@ impl DataFlowGraph {
}
}

/// Insert a value into the dfg's storage and return an id to reference it.
/// Until the value is used in an instruction it is unreachable.
pub(crate) fn make_value(&mut self, value: Value) -> ValueId {
self.values.insert(value)
}

/// Set the value of value_to_replace to refer to the value referred to by new_value.
///
/// This is the preferred method to call for optimizations simplifying
Expand Down Expand Up @@ -437,12 +430,6 @@ impl DataFlowGraph {
value_id
}

/// Returns the number of instructions
/// inserted into functions.
pub(crate) fn num_instructions(&self) -> usize {
self.instructions.len()
}

/// Returns all of result values which are attached to this instruction.
pub(crate) fn instruction_results(&self, instruction_id: InstructionId) -> &[ValueId] {
self.results.get(&instruction_id).expect("expected a list of Values").as_slice()
Expand Down Expand Up @@ -544,15 +531,6 @@ impl DataFlowGraph {
self.locations.get(&instruction).cloned().unwrap_or_default()
}

pub(crate) fn add_location_to_instruction(
&mut self,
instruction: InstructionId,
location: Location,
) {
let call_stack = self.locations.entry(instruction).or_default();
*call_stack = self.call_stack_data.add_child(*call_stack, location);
}

pub(crate) fn get_call_stack(&self, call_stack: CallStackId) -> CallStack {
self.call_stack_data.get_call_stack(call_stack)
}
Expand Down
25 changes: 0 additions & 25 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@
// These can fail.
Constrain(..) | RangeCheck { .. } => true,

// This should never be side-effectful

Check warning on line 410 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (effectful)
MakeArray { .. } | Noop => false,

// Some binary math can overflow or underflow
Expand Down Expand Up @@ -1300,31 +1300,6 @@
}

impl TerminatorInstruction {
/// Map each ValueId in this terminator to a new value.
pub(crate) fn map_values(
&self,
mut f: impl FnMut(ValueId) -> ValueId,
) -> TerminatorInstruction {
use TerminatorInstruction::*;
match self {
JmpIf { condition, then_destination, else_destination, call_stack } => JmpIf {
condition: f(*condition),
then_destination: *then_destination,
else_destination: *else_destination,
call_stack: *call_stack,
},
Jmp { destination, arguments, call_stack } => Jmp {
destination: *destination,
arguments: vecmap(arguments, |value| f(*value)),
call_stack: *call_stack,
},
Return { return_values, call_stack } => Return {
return_values: vecmap(return_values, |value| f(*value)),
call_stack: *call_stack,
},
}
}

/// Mutate each ValueId to a new ValueId using the given mapping function
pub(crate) fn map_values_mut(&mut self, mut f: impl FnMut(ValueId) -> ValueId) {
use TerminatorInstruction::*;
Expand Down
94 changes: 15 additions & 79 deletions compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{collections::VecDeque, sync::Arc};

use acvm::{
acir::{AcirField, BlackBoxFunc},
BlackBoxResolutionError, FieldElement,
FieldElement,
};
use bn254_blackbox_solver::derive_generators;
use iter_extended::vecmap;
Expand Down Expand Up @@ -501,12 +501,20 @@ fn simplify_black_box_func(
}
};
match bb_func {
BlackBoxFunc::Blake2s => {
simplify_hash(dfg, arguments, acvm::blackbox_solver::blake2s, block, call_stack)
}
BlackBoxFunc::Blake3 => {
simplify_hash(dfg, arguments, acvm::blackbox_solver::blake3, block, call_stack)
}
BlackBoxFunc::Blake2s => blackbox::simplify_hash(
dfg,
arguments,
acvm::blackbox_solver::blake2s,
block,
call_stack,
),
BlackBoxFunc::Blake3 => blackbox::simplify_hash(
dfg,
arguments,
acvm::blackbox_solver::blake3,
block,
call_stack,
),
BlackBoxFunc::Keccakf1600 => {
if let Some((array_input, _)) = dfg.get_array_constant(arguments[0]) {
if array_is_constant(dfg, &array_input) {
Expand Down Expand Up @@ -658,78 +666,6 @@ fn array_is_constant(dfg: &DataFlowGraph, values: &im::Vector<Id<Value>>) -> boo
values.iter().all(|value| dfg.get_numeric_constant(*value).is_some())
}

fn simplify_hash(
dfg: &mut DataFlowGraph,
arguments: &[ValueId],
hash_function: fn(&[u8]) -> Result<[u8; 32], BlackBoxResolutionError>,
block: BasicBlockId,
call_stack: CallStackId,
) -> SimplifyResult {
match dfg.get_array_constant(arguments[0]) {
Some((input, _)) if array_is_constant(dfg, &input) => {
let input_bytes: Vec<u8> = to_u8_vec(dfg, input);

let hash = hash_function(&input_bytes)
.expect("Rust solvable black box function should not fail");

let hash_values = hash.iter().map(|byte| FieldElement::from_be_bytes_reduce(&[*byte]));

let u8_type = NumericType::Unsigned { bit_size: 8 };
let result_array = make_constant_array(dfg, hash_values, u8_type, block, call_stack);
SimplifyResult::SimplifiedTo(result_array)
}
_ => SimplifyResult::None,
}
}

type ECDSASignatureVerifier = fn(
hashed_msg: &[u8],
public_key_x: &[u8; 32],
public_key_y: &[u8; 32],
signature: &[u8; 64],
) -> Result<bool, BlackBoxResolutionError>;
fn simplify_signature(
dfg: &mut DataFlowGraph,
arguments: &[ValueId],
signature_verifier: ECDSASignatureVerifier,
) -> SimplifyResult {
match (
dfg.get_array_constant(arguments[0]),
dfg.get_array_constant(arguments[1]),
dfg.get_array_constant(arguments[2]),
dfg.get_array_constant(arguments[3]),
) {
(
Some((public_key_x, _)),
Some((public_key_y, _)),
Some((signature, _)),
Some((hashed_message, _)),
) if array_is_constant(dfg, &public_key_x)
&& array_is_constant(dfg, &public_key_y)
&& array_is_constant(dfg, &signature)
&& array_is_constant(dfg, &hashed_message) =>
{
let public_key_x: [u8; 32] = to_u8_vec(dfg, public_key_x)
.try_into()
.expect("ECDSA public key fields are 32 bytes");
let public_key_y: [u8; 32] = to_u8_vec(dfg, public_key_y)
.try_into()
.expect("ECDSA public key fields are 32 bytes");
let signature: [u8; 64] =
to_u8_vec(dfg, signature).try_into().expect("ECDSA signatures are 64 bytes");
let hashed_message: Vec<u8> = to_u8_vec(dfg, hashed_message);

let valid_signature =
signature_verifier(&hashed_message, &public_key_x, &public_key_y, &signature)
.expect("Rust solvable black box function should not fail");

let valid_signature = dfg.make_constant(valid_signature.into(), NumericType::bool());
SimplifyResult::SimplifiedTo(valid_signature)
}
_ => SimplifyResult::None,
}
}

fn simplify_derive_generators(
dfg: &mut DataFlowGraph,
arguments: &[ValueId],
Expand Down
93 changes: 0 additions & 93 deletions compiler/noirc_evaluator/src/ssa/ir/map.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use fxhash::FxHashMap as HashMap;
use serde::{Deserialize, Serialize};
use std::{
collections::BTreeMap,
Expand Down Expand Up @@ -180,11 +179,6 @@ pub(crate) struct DenseMap<T> {
}

impl<T> DenseMap<T> {
/// Returns the number of elements in the map.
pub(crate) fn len(&self) -> usize {
self.storage.len()
}

/// Adds an element to the map.
/// Returns the identifier/reference to that element.
pub(crate) fn insert(&mut self, element: T) -> Id<T> {
Expand All @@ -193,14 +187,6 @@ impl<T> DenseMap<T> {
id
}

/// Given the Id of the element being created, adds the element
/// returned by the given function to the map
pub(crate) fn insert_with_id(&mut self, f: impl FnOnce(Id<T>) -> T) -> Id<T> {
let id = Id::new(self.storage.len().try_into().unwrap());
self.storage.push(f(id));
id
}

/// Gets an iterator to a reference to each element in the dense map paired with its id.
///
/// The id-element pairs are ordered by the numeric values of the ids.
Expand Down Expand Up @@ -246,19 +232,6 @@ pub(crate) struct SparseMap<T> {
}

impl<T> SparseMap<T> {
/// Returns the number of elements in the map.
pub(crate) fn len(&self) -> usize {
self.storage.len()
}

/// Adds an element to the map.
/// Returns the identifier/reference to that element.
pub(crate) fn insert(&mut self, element: T) -> Id<T> {
let id = Id::new(self.storage.len().try_into().unwrap());
self.storage.insert(id, element);
id
}

/// Given the Id of the element being created, adds the element
/// returned by the given function to the map
pub(crate) fn insert_with_id(&mut self, f: impl FnOnce(Id<T>) -> T) -> Id<T> {
Expand All @@ -267,13 +240,6 @@ impl<T> SparseMap<T> {
id
}

/// Remove an element from the map and return it.
/// This may return None if the element was already
/// previously removed from the map.
pub(crate) fn remove(&mut self, id: Id<T>) -> Option<T> {
self.storage.remove(&id)
}

/// Unwraps the inner storage of this map
pub(crate) fn into_btree(self) -> BTreeMap<Id<T>, T> {
self.storage
Expand All @@ -300,65 +266,6 @@ impl<T> std::ops::IndexMut<Id<T>> for SparseMap<T> {
}
}

/// 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)]
pub(crate) struct TwoWayMap<K, V> {
key_to_value: HashMap<K, V>,
value_to_key: HashMap<V, K>,
}

impl<K: Clone + Eq + Hash, V: Clone + Hash + Eq> TwoWayMap<K, V> {
/// Returns the number of elements in the map.
pub(crate) fn len(&self) -> usize {
self.key_to_value.len()
}

/// Adds an element to the map.
/// Returns the identifier/reference to that element.
pub(crate) fn insert(&mut self, key: K, element: V) -> K {
if let Some(existing) = self.value_to_key.get(&element) {
return existing.clone();
}

self.key_to_value.insert(key.clone(), element.clone());
self.value_to_key.insert(element, key.clone());

key
}

pub(crate) fn get(&self, key: &K) -> Option<&V> {
self.key_to_value.get(key)
}
}

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

// Note that there is no impl for IndexMut<T>,
// if we allowed mutable access to map elements they may be
// mutated such that elements are no longer unique
impl<K: Eq + Hash, V> std::ops::Index<K> for TwoWayMap<K, V> {
type Output = V;

fn index(&self, id: K) -> &Self::Output {
&self.key_to_value[&id]
}
}

impl<K: Eq + Hash, V> std::ops::Index<&K> for TwoWayMap<K, V> {
type Output = V;

fn index(&self, id: &K) -> &Self::Output {
&self.key_to_value[id]
}
}

/// A simple counter to create fresh Ids without any storage.
/// Useful for assigning ids before the storage is created or assigning ids
/// for types that have no single owner.
Expand Down
2 changes: 0 additions & 2 deletions compiler/noirc_evaluator/src/ssa/opt/array_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
impl Function {
pub(crate) fn array_set_optimization(&mut self) {
if matches!(self.runtime(), RuntimeType::Brillig(_)) {
// Brillig is supposed to use refcounting to decide whether to mutate an array;

Check warning on line 32 in compiler/noirc_evaluator/src/ssa/opt/array_set.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (refcounting)
// array mutation was only meant for ACIR. We could use it with Brillig as well,
// but then some of the optimizations that we can do in ACIR around shared
// references have to be skipped, which makes it more cumbersome.
Expand Down Expand Up @@ -62,7 +62,6 @@
// Mapping of an array that comes from a load and whether the address
// it was loaded from is a reference parameter passed to the block.
arrays_from_load: HashMap<ValueId, bool>,
inner_nested_arrays: HashMap<ValueId, InstructionId>,
}

impl<'f> Context<'f> {
Expand All @@ -72,7 +71,6 @@
array_to_last_use: HashMap::default(),
instructions_that_can_be_made_mutable: HashSet::default(),
arrays_from_load: HashMap::default(),
inner_nested_arrays: HashMap::default(),
}
}

Expand Down
Loading
Loading