-
Notifications
You must be signed in to change notification settings - Fork 388
feat: Enable dynamic indices on slices #2446
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
Merged
Merged
Changes from all commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
1f9c258
initial work for get dynamic indexing for slices working
vezenovm 0c79421
Merge branch 'master' into mv/slice-dyn-index
vezenovm 78c1921
Add everything except alias handling
49cc0c8
Fix all tests
88913c1
clippy
079ea08
initial work to get dyn slices working
vezenovm 0619ad7
Merge branch 'jf/new-mem2reg' into mv/slice-dyn-index
vezenovm 4f22d7b
merged in jf/new-mem2reg, have partial dynamic indices support but ca…
vezenovm fb77dfa
debug comment
vezenovm 8804a3f
Mild cleanup
cd29ec8
Undo some unneeded changes
f3f34aa
Partially revert cleanup commit which broke some tests
0887db5
Add large doc comment
a8a3f36
Merge branch 'master' into jf/new-mem2reg
33feb07
Add test and remove printlns
2574c33
Remove todos
96b9469
working dynamic slice indices w/ slice push back, need to still clean…
vezenovm 240eedb
Merge branch 'jf/new-mem2reg' into mv/slice-dyn-index
vezenovm cd8cf41
adding slice insert to get_slice_length in flattening, need to rework…
vezenovm 876bde8
remove comment from slice push back
vezenovm d3dbffa
working dynamic slices except for insert and remove
vezenovm c44321b
remove old get_slice_length method
vezenovm d7bf169
some cleanup
vezenovm 7c17c62
conflicts w/ master
vezenovm b60e23c
cargo clippy and fmt
vezenovm c672088
remove commented out patch from cargo toml
vezenovm 25d21b4
fix how we resolve length for slice intrinsics in flattening
vezenovm a1adca3
bring back full slice test
vezenovm 3749b90
uncomment slices test
vezenovm 3b7ef52
remove unnecessary assert that was for testing
vezenovm 71b01a2
Merge branch 'master' into mv/slice-dyn-index
vezenovm 5c30a44
fix outer block stores in flattening
vezenovm 7fb1b11
cargo fmt
vezenovm cdfa36b
move regression_dynamic_slice_index to new project
vezenovm d0dded9
switch to var_to_expression and remove get_constant method
vezenovm 347e9a6
slcie insert and remove with constant index
vezenovm 681d3e9
add comments for issues
vezenovm 9b95a3a
Empty-Commit
vezenovm c0094df
Merge branch 'master' into mv/slice-dyn-index
vezenovm e067884
fix constant folding optimization and cargo fmt
vezenovm d093f19
refactor handle array operation
vezenovm 6e4f75a
cargo clipy
vezenovm 15c79e0
remove assert in slice_dynamic_index_test
vezenovm 14849e1
master merge
vezenovm 1ad7de0
remove old debug derive
vezenovm 39fee20
add assert constant in slice insert and remove
vezenovm 8afe885
fix brillig slices
vezenovm 1d0c822
master conflcits
vezenovm e6d7aa8
pass correct inputs for recursive aggregation but we need to most lik…
vezenovm c38e873
Merge branch 'master' into mv/slice-dyn-index
vezenovm 395e79c
merge conflicts
vezenovm dc2defb
use default after fxHashMap
vezenovm 6446e86
Merge branch 'master' into mv/slice-dyn-index
vezenovm cbf6f36
fix call_black_box for recursive agg
vezenovm a14f9f3
cleanup printer and cargo fmt
vezenovm 2c695e3
remove assert_constant case from brillig_block
vezenovm 1e346be
remove arguments.len() check in simplify_call ArrayLen
vezenovm 576b15c
add test for using push back on dyn slice outside of if statement
vezenovm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 changes: 7 additions & 0 deletions
7
crates/nargo_cli/tests/execution_success/slice_dynamic_index/Nargo.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| [package] | ||
| name = "slice_dynamic_index" | ||
| type = "bin" | ||
| authors = [""] | ||
| compiler_version = "0.10.3" | ||
|
|
||
| [dependencies] |
1 change: 1 addition & 0 deletions
1
crates/nargo_cli/tests/execution_success/slice_dynamic_index/Prover.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| x = "5" |
222 changes: 222 additions & 0 deletions
222
crates/nargo_cli/tests/execution_success/slice_dynamic_index/src/main.nr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,222 @@ | ||
| fn main(x : Field) { | ||
| // The parameters to this function must come directly from witness values (inputs to main). | ||
| regression_dynamic_slice_index(x - 1, x - 4); | ||
| } | ||
|
|
||
| fn regression_dynamic_slice_index(x: Field, y: Field) { | ||
| let mut slice = []; | ||
| for i in 0..5 { | ||
| slice = slice.push_back(i); | ||
| } | ||
| assert(slice.len() == 5); | ||
|
|
||
| dynamic_slice_index_set_if(slice, x, y); | ||
| dynamic_slice_index_set_else(slice, x, y); | ||
| dynamic_slice_index_set_nested_if_else_else(slice, x, y); | ||
| dynamic_slice_index_set_nested_if_else_if(slice, x, y + 1); | ||
| dynamic_slice_index_if(slice, x); | ||
| dynamic_array_index_if([0, 1, 2, 3, 4], x); | ||
| dynamic_slice_index_else(slice, x); | ||
|
|
||
| dynamic_slice_merge_if(slice, x); | ||
| dynamic_slice_merge_else(slice, x); | ||
| } | ||
|
|
||
| fn dynamic_slice_index_set_if(mut slice: [Field], x: Field, y: Field) { | ||
| assert(slice[x] == 4); | ||
| assert(slice[y] == 1); | ||
| slice[y] = 0; | ||
| assert(slice[x] == 4); | ||
| assert(slice[1] == 0); | ||
| if x as u32 < 10 { | ||
| assert(slice[x] == 4); | ||
| slice[x] = slice[x] - 2; | ||
| slice[x - 1] = slice[x]; | ||
| } else { | ||
| slice[x] = 0; | ||
| } | ||
| assert(slice[3] == 2); | ||
| assert(slice[4] == 2); | ||
| } | ||
|
|
||
| fn dynamic_slice_index_set_else(mut slice: [Field], x: Field, y: Field) { | ||
| assert(slice[x] == 4); | ||
| assert(slice[y] == 1); | ||
| slice[y] = 0; | ||
| assert(slice[x] == 4); | ||
| assert(slice[1] == 0); | ||
| if x as u32 > 10 { | ||
| assert(slice[x] == 4); | ||
| slice[x] = slice[x] - 2; | ||
| slice[x - 1] = slice[x]; | ||
| } else { | ||
| slice[x] = 0; | ||
| } | ||
| assert(slice[4] == 0); | ||
| } | ||
|
|
||
| // This tests the case of missing a store instruction in the else branch | ||
| // of merging slices | ||
| fn dynamic_slice_index_if(mut slice: [Field], x: Field) { | ||
| if x as u32 < 10 { | ||
| assert(slice[x] == 4); | ||
| slice[x] = slice[x] - 2; | ||
| } else { | ||
| assert(slice[x] == 0); | ||
| } | ||
| assert(slice[4] == 2); | ||
| } | ||
|
|
||
| fn dynamic_array_index_if(mut array: [Field; 5], x: Field) { | ||
| if x as u32 < 10 { | ||
| assert(array[x] == 4); | ||
| array[x] = array[x] - 2; | ||
| } else { | ||
| assert(array[x] == 0); | ||
| } | ||
| assert(array[4] == 2); | ||
| } | ||
|
|
||
| // This tests the case of missing a store instruction in the then branch | ||
| // of merging slices | ||
| fn dynamic_slice_index_else(mut slice: [Field], x: Field) { | ||
| if x as u32 > 10 { | ||
| assert(slice[x] == 0); | ||
| } else { | ||
| assert(slice[x] == 4); | ||
| slice[x] = slice[x] - 2; | ||
| } | ||
| assert(slice[4] == 2); | ||
| } | ||
|
|
||
|
|
||
| fn dynamic_slice_merge_if(mut slice: [Field], x: Field) { | ||
| if x as u32 < 10 { | ||
| assert(slice[x] == 4); | ||
| slice[x] = slice[x] - 2; | ||
|
|
||
| slice = slice.push_back(10); | ||
| // Having an array set here checks whether we appropriately | ||
| // handle a slice length that is not yet resolving to a constant | ||
| // during flattening | ||
| slice[x] = 10; | ||
| assert(slice[slice.len() - 1] == 10); | ||
| assert(slice.len() == 6); | ||
|
|
||
| slice[x] = 20; | ||
| slice[x] = slice[x] + 10; | ||
|
|
||
| slice = slice.push_front(11); | ||
| assert(slice[0] == 11); | ||
| assert(slice.len() == 7); | ||
| assert(slice[5] == 30); | ||
|
|
||
| slice = slice.push_front(12); | ||
| assert(slice[0] == 12); | ||
| assert(slice.len() == 8); | ||
| assert(slice[6] == 30); | ||
|
|
||
| let (popped_slice, last_elem) = slice.pop_back(); | ||
| assert(last_elem == 10); | ||
| assert(popped_slice.len() == 7); | ||
|
|
||
| let (first_elem, rest_of_slice) = popped_slice.pop_front(); | ||
| assert(first_elem == 12); | ||
| assert(rest_of_slice.len() == 6); | ||
|
|
||
| // TODO(#2462): SliceInsert and SliceRemove with a dynamic index are not yet implemented in ACIR gen | ||
| slice = rest_of_slice.insert(2, 20); | ||
| assert(slice[2] == 20); | ||
| assert(slice[6] == 30); | ||
| assert(slice.len() == 7); | ||
|
|
||
| // TODO(#2462): SliceInsert and SliceRemove with a dynamic index are not yet implemented in ACIR gen | ||
| let (removed_slice, removed_elem) = slice.remove(3); | ||
| // The deconstructed tuple assigns to the slice but is not seen outside of the if statement | ||
| // without a direct assignment | ||
| slice = removed_slice; | ||
|
|
||
| assert(removed_elem == 1); | ||
| assert(slice.len() == 6); | ||
| } else { | ||
| assert(slice[x] == 0); | ||
| slice = slice.push_back(20); | ||
| } | ||
|
|
||
| assert(slice.len() == 6); | ||
| assert(slice[slice.len() - 1] == 30); | ||
| } | ||
|
|
||
| fn dynamic_slice_merge_else(mut slice: [Field], x: Field) { | ||
| if x as u32 > 10 { | ||
| assert(slice[x] == 0); | ||
| slice[x] = 2; | ||
| } else { | ||
| assert(slice[x] == 4); | ||
| slice[x] = slice[x] - 2; | ||
| slice = slice.push_back(10); | ||
| } | ||
| assert(slice.len() == 6); | ||
| assert(slice[slice.len() - 1] == 10); | ||
|
|
||
| slice = slice.push_back(20); | ||
| assert(slice.len() == 7); | ||
| assert(slice[slice.len() - 1] == 20); | ||
| } | ||
|
|
||
| fn dynamic_slice_index_set_nested_if_else_else(mut slice: [Field], x: Field, y: Field) { | ||
| assert(slice[x] == 4); | ||
| assert(slice[y] == 1); | ||
| slice[y] = 0; | ||
| assert(slice[x] == 4); | ||
| assert(slice[1] == 0); | ||
| if x as u32 < 10 { | ||
| slice[x] = slice[x] - 2; | ||
| if y != 1 { | ||
| slice[x] = slice[x] + 20; | ||
| } else { | ||
| if x == 5 { | ||
| // We should not hit this case | ||
| assert(slice[x] == 22); | ||
| } else { | ||
| slice[x] = 10; | ||
| slice = slice.push_back(15); | ||
| assert(slice.len() == 6); | ||
| } | ||
| assert(slice[4] == 10); | ||
| } | ||
| } else { | ||
| slice[x] = 0; | ||
| } | ||
| assert(slice[4] == 10); | ||
| assert(slice.len() == 6); | ||
| assert(slice[slice.len() - 1] == 15); | ||
|
|
||
| slice = slice.push_back(20); | ||
| assert(slice.len() == 7); | ||
| assert(slice[slice.len() - 1] == 20); | ||
| } | ||
|
|
||
| fn dynamic_slice_index_set_nested_if_else_if(mut slice: [Field], x: Field, y: Field) { | ||
| assert(slice[x] == 4); | ||
| assert(slice[y] == 2); | ||
| slice[y] = 0; | ||
| assert(slice[x] == 4); | ||
| assert(slice[2] == 0); | ||
| if x as u32 < 10 { | ||
| slice[x] = slice[x] - 2; | ||
| // TODO: this panics as we have a load for the slice in flattening | ||
| if y == 1 { | ||
| slice[x] = slice[x] + 20; | ||
| } else { | ||
| if x == 4 { | ||
| slice[x] = 5; | ||
| } | ||
| assert(slice[4] == 5); | ||
| } | ||
| } else { | ||
| slice[x] = 0; | ||
| } | ||
| assert(slice[4] == 5); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.