From dd89b900dfabbea889f6f181aa562e73a87215b7 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 18 Jul 2024 10:06:27 -0400 Subject: [PATCH] fix(ssa): More robust array deduplication check (#5547) # Description ## Problem\* Resolves https://github.com/noir-lang/noir/issues/5529 ## Summary\* In https://github.com/noir-lang/noir/pull/5287 we added a check to avoid duplication of arrays. However, we can make this check more complete by allowing us to check constant arrays based off of their contents when we are inside of ACIR and also distinguishing between arrays and slices. The program in the issue, for any size input array, is now the same number of ACIR gates for both versions (using a local array variable and an array fetched from a function). ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../src/ssa/ir/function_inserter.rs | 40 ++++++++++++++----- .../trait_method_mut_self/Nargo.toml | 0 .../trait_method_mut_self/Prover.toml | 0 .../trait_method_mut_self/src/main.nr | 0 .../turbofish_call_func_diff_types/Nargo.toml | 0 .../Prover.toml | 0 .../src/main.nr | 0 7 files changed, 29 insertions(+), 11 deletions(-) rename test_programs/{execution_success => compile_success_empty}/trait_method_mut_self/Nargo.toml (100%) rename test_programs/{execution_success => compile_success_empty}/trait_method_mut_self/Prover.toml (100%) rename test_programs/{execution_success => compile_success_empty}/trait_method_mut_self/src/main.nr (100%) rename test_programs/{execution_success => compile_success_empty}/turbofish_call_func_diff_types/Nargo.toml (100%) rename test_programs/{execution_success => compile_success_empty}/turbofish_call_func_diff_types/Prover.toml (100%) rename test_programs/{execution_success => compile_success_empty}/turbofish_call_func_diff_types/src/main.nr (100%) diff --git a/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs b/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs index a063a7ff268..9baeb19206f 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs @@ -1,9 +1,11 @@ use iter_extended::vecmap; +use crate::ssa::ir::types::Type; + use super::{ basic_block::BasicBlockId, dfg::{CallStack, InsertInstructionResult}, - function::Function, + function::{Function, RuntimeType}, instruction::{Instruction, InstructionId}, value::ValueId, }; @@ -16,7 +18,11 @@ pub(crate) struct FunctionInserter<'f> { pub(crate) function: &'f mut Function, values: HashMap, - const_arrays: HashMap, ValueId>, + /// Map containing repeat array constant so that we do not initialize a new + /// array unnecessarily. An extra bool is included as part of the key to + /// distinguish between Type::Array and Type::Slice, as both are valid + /// types for a Value::Array + const_arrays: HashMap<(im::Vector, bool), ValueId>, } impl<'f> FunctionInserter<'f> { @@ -37,15 +43,27 @@ impl<'f> FunctionInserter<'f> { let typ = typ.clone(); let new_array: im::Vector = array.iter().map(|id| self.resolve(*id)).collect(); - if self.const_arrays.get(&new_array) == Some(&value) { - value - } else { - let new_array_clone = new_array.clone(); - let new_id = self.function.dfg.make_array(new_array, typ); - self.values.insert(value, new_id); - self.const_arrays.insert(new_array_clone, new_id); - new_id - } + + // Flag to determine the type of the value's array list + let is_array = matches!(typ, Type::Array { .. }); + if let Some(fetched_value) = + self.const_arrays.get(&(new_array.clone(), is_array)) + { + // Arrays in ACIR are immutable, but in Brillig arrays are copy-on-write + // so for function's with a Brillig runtime we make sure to check that value + // in our constants array map matches the resolved array value id. + if matches!(self.function.runtime(), RuntimeType::Acir(_)) { + return *fetched_value; + } else if *fetched_value == value { + return value; + } + }; + + let new_array_clone = new_array.clone(); + let new_id = self.function.dfg.make_array(new_array, typ); + self.values.insert(value, new_id); + self.const_arrays.insert((new_array_clone, is_array), new_id); + new_id } _ => value, }, diff --git a/test_programs/execution_success/trait_method_mut_self/Nargo.toml b/test_programs/compile_success_empty/trait_method_mut_self/Nargo.toml similarity index 100% rename from test_programs/execution_success/trait_method_mut_self/Nargo.toml rename to test_programs/compile_success_empty/trait_method_mut_self/Nargo.toml diff --git a/test_programs/execution_success/trait_method_mut_self/Prover.toml b/test_programs/compile_success_empty/trait_method_mut_self/Prover.toml similarity index 100% rename from test_programs/execution_success/trait_method_mut_self/Prover.toml rename to test_programs/compile_success_empty/trait_method_mut_self/Prover.toml diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/compile_success_empty/trait_method_mut_self/src/main.nr similarity index 100% rename from test_programs/execution_success/trait_method_mut_self/src/main.nr rename to test_programs/compile_success_empty/trait_method_mut_self/src/main.nr diff --git a/test_programs/execution_success/turbofish_call_func_diff_types/Nargo.toml b/test_programs/compile_success_empty/turbofish_call_func_diff_types/Nargo.toml similarity index 100% rename from test_programs/execution_success/turbofish_call_func_diff_types/Nargo.toml rename to test_programs/compile_success_empty/turbofish_call_func_diff_types/Nargo.toml diff --git a/test_programs/execution_success/turbofish_call_func_diff_types/Prover.toml b/test_programs/compile_success_empty/turbofish_call_func_diff_types/Prover.toml similarity index 100% rename from test_programs/execution_success/turbofish_call_func_diff_types/Prover.toml rename to test_programs/compile_success_empty/turbofish_call_func_diff_types/Prover.toml diff --git a/test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr b/test_programs/compile_success_empty/turbofish_call_func_diff_types/src/main.nr similarity index 100% rename from test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr rename to test_programs/compile_success_empty/turbofish_call_func_diff_types/src/main.nr