From 9cc03a4d6f714a1b2d31c6982eb8e791ba5c869c Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 18 Apr 2024 20:09:13 +0100 Subject: [PATCH] feat: Allow numeric generics to non inlined ACIR functions (#4834) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description ## Problem\* Resolves https://github.com/noir-lang/noir/issues/4826. ## Summary\* When running `nargo info` with the following code: ```rust use dep::std::hash::{pedersen_hash_with_separator, poseidon2::Poseidon2}; global NUM_HASHES = 2; global HASH_LENGTH = 10; #[fold] pub fn poseidon_hash(inputs: [Field; N]) -> Field { Poseidon2::hash(inputs, inputs.len()) } fn main( to_hash: [[Field; HASH_LENGTH]; NUM_HASHES], enable: [bool; NUM_HASHES] ) -> pub [Field; NUM_HASHES] { let mut result = [0; NUM_HASHES]; for i in 0..NUM_HASHES { let enable = enable[i]; let to_hash = to_hash[i]; if enable { result[i] = poseidon_hash(to_hash); } } result } ``` Screenshot 2024-04-17 at 3 46 54 PM When using `pedersen_hash` we also get 10 opcodes for `main`. Once we get more granularity in our inline types (fold vs. inline for proving) we can most likely fully resolve this issue (https://github.com/noir-lang/noir/issues/4688). ## Additional Context ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [ ] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../src/hir/resolution/resolver.rs | 5 +- .../noirc_frontend/src/hir/type_check/mod.rs | 17 ++++++- compiler/noirc_frontend/src/hir_def/types.rs | 48 +++++++++++++++++++ .../src/monomorphization/mod.rs | 8 +++- .../fold_numeric_generic_poseidon/Nargo.toml | 7 +++ .../fold_numeric_generic_poseidon/Prover.toml | 2 + .../fold_numeric_generic_poseidon/src/main.nr | 33 +++++++++++++ tooling/debugger/ignored-tests.txt | 1 + 8 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 test_programs/execution_success/fold_numeric_generic_poseidon/Nargo.toml create mode 100644 test_programs/execution_success/fold_numeric_generic_poseidon/Prover.toml create mode 100644 test_programs/execution_success/fold_numeric_generic_poseidon/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 479f357126a..01d477e9d4c 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -929,6 +929,7 @@ impl<'a> Resolver<'a> { let name_ident = HirIdent::non_trait_method(id, location); let attributes = func.attributes().clone(); + let should_fold = attributes.is_foldable(); let mut generics = vecmap(&self.generics, |(_, typevar, _)| typevar.clone()); let mut parameters = vec![]; @@ -1009,8 +1010,6 @@ impl<'a> Resolver<'a> { .map(|(name, typevar, _span)| (name.clone(), typevar.clone())) .collect(); - let should_fold = attributes.is_foldable(); - FuncMeta { name: name_ident, kind: func.kind, @@ -1039,7 +1038,7 @@ impl<'a> Resolver<'a> { /// True if the 'pub' keyword is allowed on parameters in this function /// 'pub' on function parameters is only allowed for entry point functions fn pub_allowed(&self, func: &NoirFunction) -> bool { - self.is_entry_point_function(func) + self.is_entry_point_function(func) || func.attributes().is_foldable() } fn is_entry_point_function(&self, func: &NoirFunction) -> bool { diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index f5323cd07de..b8931ce56b9 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -172,7 +172,9 @@ fn check_if_type_is_valid_for_program_input( errors: &mut Vec, ) { let meta = type_checker.interner.function_meta(&func_id); - if (meta.is_entry_point || meta.should_fold) && !param.1.is_valid_for_program_input() { + if (meta.is_entry_point && !param.1.is_valid_for_program_input()) + || (meta.should_fold && !param.1.is_valid_non_inlined_function_input()) + { let span = param.0.span(); errors.push(TypeCheckError::InvalidTypeForEntryPoint { span }); } @@ -632,12 +634,23 @@ pub mod test { #[fold] fn fold(x: &mut Field) -> Field { *x - } + } "#; type_check_src_code_errors_expected(src, vec![String::from("fold")], 1); } + #[test] + fn fold_numeric_generic() { + let src = r#" + #[fold] + fn fold(x: T) -> T { + x + } + "#; + + type_check_src_code(src, vec![String::from("fold")]); + } // This is the same Stub that is in the resolver, maybe we can pull this out into a test module and re-use? struct TestPathResolver(HashMap); diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index ec8b54c33b8..6aced6cced4 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -726,6 +726,54 @@ impl Type { } } + /// True if this type can be used as a parameter to an ACIR function that is not `main` or a contract function. + /// This encapsulates functions for which we may not want to inline during compilation. + /// + /// The inputs allowed for a function entry point differ from those allowed as input to a program as there are + /// certain types which through compilation we know what their size should be. + /// This includes types such as numeric generics. + pub(crate) fn is_valid_non_inlined_function_input(&self) -> bool { + match self { + // Type::Error is allowed as usual since it indicates an error was already issued and + // we don't need to issue further errors about this likely unresolved type + Type::FieldElement + | Type::Integer(_, _) + | Type::Bool + | Type::Unit + | Type::Constant(_) + | Type::TypeVariable(_, _) + | Type::NamedGeneric(_, _) + | Type::Error => true, + + Type::FmtString(_, _) + // To enable this we would need to determine the size of the closure outputs at compile-time. + // This is possible as long as the output size is not dependent upon a witness condition. + | Type::Function(_, _, _) + | Type::Slice(_) + | Type::MutableReference(_) + | Type::Forall(_, _) + // TODO: probably can allow code as it is all compile time + | Type::Code + | Type::TraitAsType(..) => false, + + Type::Alias(alias, generics) => { + let alias = alias.borrow(); + alias.get_type(generics).is_valid_non_inlined_function_input() + } + + Type::Array(length, element) => { + length.is_valid_non_inlined_function_input() && element.is_valid_non_inlined_function_input() + } + Type::String(length) => length.is_valid_non_inlined_function_input(), + Type::Tuple(elements) => elements.iter().all(|elem| elem.is_valid_non_inlined_function_input()), + Type::Struct(definition, generics) => definition + .borrow() + .get_fields(generics) + .into_iter() + .all(|(_, field)| field.is_valid_non_inlined_function_input()), + } + } + /// Returns the number of `Forall`-quantified type variables on this type. /// Returns 0 if this is not a Type::Forall pub fn generic_count(&self) -> usize { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 2cccc18fb09..20b9c0885bf 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -283,7 +283,13 @@ impl<'interner> Monomorphizer<'interner> { } let meta = self.interner.function_meta(&f).clone(); - let func_sig = meta.function_signature(); + let mut func_sig = meta.function_signature(); + // Follow the bindings of the function signature for entry points + // which are not `main` such as foldable functions. + for param in func_sig.0.iter_mut() { + param.1 = param.1.follow_bindings(); + } + func_sig.1 = func_sig.1.map(|return_type| return_type.follow_bindings()); let modifiers = self.interner.function_modifiers(&f); let name = self.interner.function_name(&f).to_owned(); diff --git a/test_programs/execution_success/fold_numeric_generic_poseidon/Nargo.toml b/test_programs/execution_success/fold_numeric_generic_poseidon/Nargo.toml new file mode 100644 index 00000000000..8c2bc79ea8d --- /dev/null +++ b/test_programs/execution_success/fold_numeric_generic_poseidon/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "fold_numeric_generic_poseidon" +type = "bin" +authors = [""] +compiler_version = ">=0.27.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/fold_numeric_generic_poseidon/Prover.toml b/test_programs/execution_success/fold_numeric_generic_poseidon/Prover.toml new file mode 100644 index 00000000000..00e821cf89d --- /dev/null +++ b/test_programs/execution_success/fold_numeric_generic_poseidon/Prover.toml @@ -0,0 +1,2 @@ +enable = [true, false] +to_hash = [[0, 0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0]] diff --git a/test_programs/execution_success/fold_numeric_generic_poseidon/src/main.nr b/test_programs/execution_success/fold_numeric_generic_poseidon/src/main.nr new file mode 100644 index 00000000000..f9f3e75789b --- /dev/null +++ b/test_programs/execution_success/fold_numeric_generic_poseidon/src/main.nr @@ -0,0 +1,33 @@ +use dep::std::hash::{pedersen_hash_with_separator, poseidon2::Poseidon2}; + +global NUM_HASHES = 2; +global HASH_LENGTH = 10; + +#[fold] +pub fn poseidon_hash(inputs: [Field; N]) -> Field { + Poseidon2::hash(inputs, inputs.len()) +} + +fn main( + to_hash: [[Field; HASH_LENGTH]; NUM_HASHES], + enable: [bool; NUM_HASHES] +) -> pub [Field; NUM_HASHES + 1] { + let mut result = [0; NUM_HASHES + 1]; + for i in 0..NUM_HASHES { + let enable = enable[i]; + let to_hash = to_hash[i]; + if enable { + result[i] = poseidon_hash(to_hash); + } + } + + // We want to make sure that the foldable function with a numeric generic + // is monomorphized correctly. + let mut double_preimage = [0; 20]; + for i in 0..HASH_LENGTH * 2 { + double_preimage[i] = to_hash[0][i % HASH_LENGTH]; + } + result[NUM_HASHES] = poseidon_hash(double_preimage); + + result +} diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index 40d32072843..3b63f8d5542 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -16,4 +16,5 @@ fold_basic fold_basic_nested_call fold_call_witness_condition fold_after_inlined_calls +fold_numeric_generic_poseidon