ACIR gen: Enable complex outputs to ACIR function calls #4608
Labels
Comments
Closed
vezenovm
changed the title
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
May 1, 2024
…tnesses # Description ## Problem\* Found the issue while working on #4608. ## Summary\* For this program: ```rust fn main(x: u32, y: pub u32) { let new_field = new_field_in_array([x, y, 3]); assert(new_field[0] == 25); } #[fold] fn new_field_in_array(mut input: [u32; 3]) -> [u32 ; 3] { input[0] = input[0] + 20; input } ``` We get the following ACIR: Old ACIR: ``` func 0 current witness index : 5 private parameters indices : [0] public parameters indices : [1] return value indices : [] BLACKBOX::RANGE [(_0, num_bits: 32)] [ ] BLACKBOX::RANGE [(_1, num_bits: 32)] [ ] EXPR [ (-1, _2) 3 ] CALL func 1: PREDICATE = %EXPR [ 1 ]% inputs: [Witness(0), Witness(1), Witness(2)], outputs: [Witness(3), Witness(4), Witness(5)] EXPR [ (1, _3) -25 ] func 1 current witness index : 3 private parameters indices : [0, 1, 2] public parameters indices : [] return value indices : [1, 2, 3] BLACKBOX::RANGE [(_0, num_bits: 32)] [ ] BLACKBOX::RANGE [(_1, num_bits: 32)] [ ] BLACKBOX::RANGE [(_2, num_bits: 32)] [ ] EXPR [ (1, _0) (-1, _3) 20 ] BLACKBOX::RANGE [(_3, num_bits: 32)] [ ] ``` One can see how simply the next witness (`_3` in this case) is being assigned to 20 directly and returned rather than returning it correctly according to the type. The constraint in the `main` function above will fail as the returned array will be `[5, 10, 20]` when we want `[20, 5, 10]`. This re-ordering can be mitigated by requiring all returns from foldable functions to be distinct for which we get the following ACIR: New ACIR: The main func is the same. ``` func 0 (same as above) func 1 current witness index : 6 private parameters indices : [0, 1, 2] public parameters indices : [] return value indices : [4, 5, 6] BLACKBOX::RANGE [(_0, num_bits: 32)] [ ] BLACKBOX::RANGE [(_1, num_bits: 32)] [ ] BLACKBOX::RANGE [(_2, num_bits: 32)] [ ] EXPR [ (1, _0) (-1, _3) 20 ] BLACKBOX::RANGE [(_3, num_bits: 32)] [ ] EXPR [ (1, _3) (-1, _4) 0 ] EXPR [ (1, _1) (-1, _5) 0 ] EXPR [ (1, _2) (-1, _6) 0 ] ``` I think just defaulting all return values from foldable functions to be distinct is going to be the least confusing for users (and we have discussed making it the default on `main` as well). If we want to add the ability for a user to not use `distinct` on foldable function return values that can come in a follow up. ## 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. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
github-merge-queue bot
pushed a commit
that referenced
this issue
May 1, 2024
# Description ## Problem\* Resolves #4608 ## Summary\* The change ended up being quite simple where we just have to call the flattened size on the type rather than `try_get_array_length`. We type check that any other types such as slices or references are not returned from the foldable function being called so this call is safe to do. A test was also included in `fold_complex_outputs` that checks whether we are able to successfully modify a complex struct in a foldable function correctly. ## Additional Context ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [x] **[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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
The initial codegen work for having multiple ACIR function calls was done here (AztecProtocol/aztec-packages#5341). As a call accepts witnesses as inputs and returns witnesses for outputs (similar to a black box function) the same logic was reused that we have for black box functions to create these inputs/outputs.
However, black box functions only support single variables or primitive arrays as outputs.
Happy Case
We expand the supported outputs for ACIR function calls to non-homogenous arrays and add tests for this codegen.
This would also enable complex outputs to black box functions in the future. It looks like the codegen does not bother with this right now as our existing black box functions only return single variables or arrays right now. However, with ACIR function calls users will be surprised when they attempt to use a non-homogenous array and it does not work.
Project Impact
None
Impact Context
No response
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
The text was updated successfully, but these errors were encountered: