Skip to content

Commit

Permalink
feat: Allow numeric generics to non inlined ACIR functions (#4834)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #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<N>(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
}
```
<img width="739" alt="Screenshot 2024-04-17 at 3 46 54 PM"
src="https://github.com/noir-lang/noir/assets/43554004/fa31977d-96e9-45f7-8001-43865bc4a83c">

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
(#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.
  • Loading branch information
vezenovm authored Apr 18, 2024
1 parent 8e39706 commit 9cc03a4
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 6 deletions.
5 changes: 2 additions & 3 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![];
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 15 additions & 2 deletions compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,9 @@ fn check_if_type_is_valid_for_program_input(
errors: &mut Vec<TypeCheckError>,
) {
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 });
}
Expand Down Expand Up @@ -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<T>(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<String, ModuleDefId>);

Expand Down
48 changes: 48 additions & 0 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "fold_numeric_generic_poseidon"
type = "bin"
authors = [""]
compiler_version = ">=0.27.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -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]]
Original file line number Diff line number Diff line change
@@ -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<N>(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
}
1 change: 1 addition & 0 deletions tooling/debugger/ignored-tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ fold_basic
fold_basic_nested_call
fold_call_witness_condition
fold_after_inlined_calls
fold_numeric_generic_poseidon

0 comments on commit 9cc03a4

Please sign in to comment.