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

feat: avoid duplicate arrays in SSA #6496

Closed
wants to merge 2 commits into from
Closed
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
12 changes: 11 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ pub(crate) struct DataFlowGraph {
#[serde(skip)]
constants: HashMap<(FieldElement, Type), ValueId>,

/// Each array is unique. Attempting to insert an array with the same
/// list of values will return the same ValueId.
arrays: HashMap<(im::Vector<ValueId>, Type), ValueId>,

/// Contains each function that has been imported into the current function.
/// A unique `ValueId` for each function's [`Value::Function`] is stored so any given FunctionId
/// will always have the same ValueId within this function.
Expand Down Expand Up @@ -268,8 +272,14 @@ impl DataFlowGraph {

/// Create a new constant array value from the given elements
pub(crate) fn make_array(&mut self, array: im::Vector<ValueId>, typ: Type) -> ValueId {
if let Some(id) = self.arrays.get(&(array.clone(), typ.clone())) {
return *id;
}

assert!(matches!(typ, Type::Array(..) | Type::Slice(_)));
self.make_value(Value::Array { array, typ })
let id = self.make_value(Value::Array { array: array.clone(), typ: typ.clone() });
self.arrays.insert((array, typ), id);
id
}

/// Gets or creates a ValueId for the given FunctionId.
Expand Down
8 changes: 8 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ impl<'f> FunctionInserter<'f> {

let new_array_clone = new_array.clone();
let new_id = self.function.dfg.make_array(new_array, typ.clone());

// If we got the same value (can happen because a function's dfg will cache
// identical arrays) we don't cache the new value as it would end to an infinite loop
// when trying to resolve it.
if new_id == value {
return value;
}

self.values.insert(value, new_id);
self.const_arrays.insert((new_array_clone, typ), new_id);
new_id
Expand Down
2 changes: 0 additions & 2 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -844,9 +844,7 @@ mod test {
assert_eq!(instructions.len(), 10);
}

// This test currently fails. It being fixed will address the issue https://github.com/noir-lang/noir/issues/5756
#[test]
#[should_panic]
fn constant_array_deduplication() {
// fn main f0 {
// b0(v0: u64):
Expand Down
Loading