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

Enable numeric generic inputs for non-inlined ACIR #4826

Closed
Tracked by #4426
vezenovm opened this issue Apr 16, 2024 · 0 comments · Fixed by #4834
Closed
Tracked by #4426

Enable numeric generic inputs for non-inlined ACIR #4826

vezenovm opened this issue Apr 16, 2024 · 0 comments · Fixed by #4834
Labels
compiler frontend `noirc_frontend` crate compiler enhancement New feature or request

Comments

@vezenovm
Copy link
Contributor

vezenovm commented Apr 16, 2024

Problem

As part of the initial work to add non-inlined ACIR #4428 we added type checking that treated all inputs to entry point ACIR the same. This meant non-inlined ACIR functions which were not main would have the same restriction of only support what is supported in Noir's ABI. This is an unnecessary restriction.

Happy Case

We should support more compile-time inputs for non-inlined ACIR entry points. For example, numeric generics should be known at compile-time for non-inlined ACIR.

References will still have to be restricted. We would only be able to pass references as inputs if we started to support them in ACIR gen. Slice sizes are technically known at compile time, but not during monomorphization when we have slices whose size is dependent upon witness conditions. For now, we just want to upgrade numeric generics.

Project Impact

Blocker

Impact Context

This prevents being able to not inline functions which utilize numeric generics, such as our poseidon2 hash method:

pub fn hash<N>(input: [Field; N], message_size: u64) -> Field {
        if message_size == N {
            Poseidon2::hash_internal(input, N, false)
        } else {
            Poseidon2::hash_internal(input, message_size, true)
        }
    }

Workaround

None

Workaround Description

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@vezenovm vezenovm added enhancement New feature or request compiler compiler frontend `noirc_frontend` crate labels Apr 16, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Apr 16, 2024
@vezenovm vezenovm changed the title Enable more compile-time inputs for non-inlined ACIR Enable numeric generic inputs for non-inlined ACIR Apr 17, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 18, 2024
# 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.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler frontend `noirc_frontend` crate compiler enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant