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

feat(acir): Handle dynamic array operations for nested slices #3187

Merged
merged 66 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 64 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
c2f2f62
enabled dynamic indices on nested arrays
vezenovm Sep 14, 2023
45b3787
remove debug
vezenovm Sep 14, 2023
7ccf8e9
working non-homogenous arr accesses for block params
vezenovm Sep 15, 2023
d5d16d7
add array_set, broken, need to maintain structure from SSA for accura…
vezenovm Sep 19, 2023
ec54e01
merge conflcits w/ master
vezenovm Sep 19, 2023
e2eff9b
working non-homogenous arrays using internal type element size array,…
vezenovm Sep 19, 2023
219c8f2
cleanup lots of debug and move const index access into its own method
vezenovm Sep 19, 2023
3195325
updating array_get and array_set to reduce repeated code
vezenovm Sep 19, 2023
8fb3bb1
restrict dynamic indices for non-homogenous slices
vezenovm Sep 19, 2023
469c333
cleanup array_get_value
vezenovm Sep 19, 2023
0648142
add internal_memory_blocks map
vezenovm Sep 19, 2023
b99f0e2
do not make separate bool for handle_constant_index
vezenovm Sep 20, 2023
5784170
fetch dummy value with predicate index
vezenovm Sep 20, 2023
05f35c6
flatten the index once for both array_set and array_get, do not modif…
vezenovm Sep 20, 2023
f2dd702
remove first_elem param
vezenovm Sep 21, 2023
490f5af
initial slice changes with fetch flat size from SSA values, need to g…
vezenovm Sep 25, 2023
0037867
merge master
vezenovm Sep 25, 2023
3636bc6
initial working nested dynamic slices
vezenovm Sep 28, 2023
31e7bc2
cleanup and fmt
vezenovm Sep 28, 2023
4354b76
cargo clippy and fmt
vezenovm Sep 28, 2023
87e8314
merge conflicts w/ master
vezenovm Sep 28, 2023
658d234
remove boolean AcirType
vezenovm Sep 28, 2023
408ac2c
remove deaad code not in master
vezenovm Sep 28, 2023
22ce20c
better solution for fetching nested slice with multiple slice mergesr
vezenovm Sep 28, 2023
686640e
bring back assert in nested_array_dynamic
vezenovm Sep 28, 2023
626616b
add assert to nested_slice_dynamic test
vezenovm Sep 28, 2023
f60183c
cleanup w/ copy_dynamic_array method
vezenovm Sep 28, 2023
be5dc79
starting test for slices in struct fields
vezenovm Sep 29, 2023
a7fde3e
more initial debugging for adding slices to struct fields
vezenovm Oct 2, 2023
d1c9192
basic slice struct fields working, but broken for struct fields of di…
vezenovm Oct 3, 2023
6ab1212
got old nested array and slice working with new acir, basic array get…
vezenovm Oct 4, 2023
d5911b8
merged w/ master, and working old nested arr/slice tests, array_set f…
vezenovm Oct 4, 2023
b9c6a0a
heavy debugging work to get flattening with slice of slices working
vezenovm Oct 5, 2023
11f7dc8
working slice field in a struct
vezenovm Oct 5, 2023
7f4c822
little test func for flattened slice size of res
vezenovm Oct 5, 2023
d97b965
merge conflicts w/ master
vezenovm Oct 9, 2023
20f61a5
Merge branch 'master' into mv/slice-struct-fields
vezenovm Oct 11, 2023
37a0f0f
merge conflicts w/ master
vezenovm Oct 16, 2023
1e0bd3f
update slice struct fields to use map of slice sizes in ACIR gen
vezenovm Oct 17, 2023
3b08844
clean up dbgs and old debug in acvm
vezenovm Oct 17, 2023
9086383
cleanup
vezenovm Oct 17, 2023
f1c707c
comments update
vezenovm Oct 17, 2023
9108769
check if we have side effects enabeld when flattening index
vezenovm Oct 17, 2023
589185c
Merge branch 'master' into mv/slice-struct-fields
vezenovm Oct 17, 2023
4209388
cleanup
vezenovm Oct 17, 2023
2949302
add TODOs to value merger
vezenovm Oct 17, 2023
1456af9
Merge branch 'master' into mv/slice-struct-fields
vezenovm Oct 23, 2023
e58dbba
Merge branch 'master' into mv/slice-struct-fields
TomAFrench Oct 31, 2023
f30062e
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Oct 31, 2023
dd3baec
feat(slices): Fill slice internal dummy data initial pass (#3258)
vezenovm Oct 31, 2023
3b30290
simplify predicate logic when fetching flattened index
vezenovm Nov 1, 2023
4926713
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 1, 2023
24bfae9
use ok_or_else on store value slice sizes
vezenovm Nov 1, 2023
a8dae7a
reduce nesting from compute_slice_sizes in acir gen
vezenovm Nov 1, 2023
d558647
Merge branch 'master' into mv/slice-struct-fields
vezenovm Nov 1, 2023
ed372cc
Merge branch 'master' into mv/slice-struct-fields
jfecher Nov 1, 2023
ff280cb
Merge branch 'master' into mv/slice-struct-fields
vezenovm Nov 2, 2023
402f421
revert merge of 3258
vezenovm Nov 2, 2023
5286fd1
use slice not mutable vec for array_get_value
vezenovm Nov 2, 2023
96805de
comment out failing constraints fixed by fill internal slices pass
vezenovm Nov 2, 2023
c7649d3
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 3, 2023
bf0aa94
wrap parent_array in Some
vezenovm Nov 3, 2023
2528f3d
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 3, 2023
38669e9
add comments for slize sizes
vezenovm Nov 3, 2023
2b446a1
remove catch all for contains_slice_element
vezenovm Nov 3, 2023
33beadf
Merge branch 'master' into mv/slice-struct-fields
vezenovm Nov 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
233 changes: 174 additions & 59 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,20 @@ impl Type {
}
}

pub(crate) fn contains_slice_element(&self) -> bool {
match self {
Type::Array(elements, _) => {
elements.iter().any(|element| element.contains_slice_element())
}
Type::Slice(_) => true,
Type::Numeric(_) => false,
// TODO: Look at if we need special handling for references
_ => {
unreachable!("ICE: expected array, slice, or numeric type but got {self:?}");
}
jfecher marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// Returns the flattened size of a Type
pub(crate) fn flattened_size(&self) -> usize {
let mut size = 0;
Expand Down
14 changes: 11 additions & 3 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,15 +194,16 @@ impl<'a> ValueMerger<'a> {

for i in 0..len {
for (element_index, element_type) in element_types.iter().enumerate() {
let index_value = ((i * element_types.len() + element_index) as u128).into();
let index_usize = i * element_types.len() + element_index;
let index_value = (index_usize as u128).into();
let index = self.dfg.make_constant(index_value, Type::field());

let typevars = Some(vec![element_type.clone()]);

let mut get_element = |array, typevars, len| {
// The smaller slice is filled with placeholder data. Codegen for slice accesses must
// include checks against the dynamic slice length so that this placeholder data is not incorrectly accessed.
if len <= index_value.to_u128() as usize {
if len <= index_usize {
self.make_slice_dummy_data(element_type)
} else {
let get = Instruction::ArrayGet { array, index };
Expand Down Expand Up @@ -239,6 +240,11 @@ impl<'a> ValueMerger<'a> {
Value::Instruction { instruction: instruction_id, .. } => {
let instruction = &self.dfg[*instruction_id];
match instruction {
// TODO(#3188): A slice can be the result of an ArrayGet when it is the
// fetched from a slice of slices or as a struct field.
// However, we need to incorporate nested slice support in flattening
// in order for this to be valid
// Instruction::ArrayGet { array, .. } => {}
Instruction::ArraySet { array, .. } => {
let array = *array;
let len = self.get_slice_length(array);
Expand Down Expand Up @@ -323,7 +329,9 @@ impl<'a> ValueMerger<'a> {
self.dfg.make_array(array, typ.clone())
}
Type::Slice(_) => {
unreachable!("ICE: Slices of slice is unsupported")
// TODO(#3188): Need to update flattening to use true user facing length of slices
// to accurately construct dummy data
unreachable!("ICE: Cannot return a slice of slices from an if expression")
}
Type::Reference => {
unreachable!("ICE: Merging references is unsupported")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn main(mut x : [Foo; 4], y : pub Field) {
} else {
x[y].a = 100;
}
assert(x[y].a == 50);
assert(x[3].a == 50);
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved

if y == 2 {
x[y - 1].b = [50, 51, 52];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
fn main(mut x: [u32; 5], idx: Field) {
// We should not hit out of bounds here as we have a predicate
// that should not be hit
if idx as u32 < 3 {
if idx as u32 < 3 {
x[idx] = 10;
}
assert(x[4] == 111);
}
assert(x[4] == 111);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "slice_struct_field"
type = "bin"
authors = [""]

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
y = "3"
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
struct FooParent {
parent_arr: [Field; 3],
foos: [Foo],
}

struct Bar {
inner: [Field; 3],
}

struct Foo {
a: Field,
b: [Field],
bar: Bar,
}

fn main(y : pub Field) {
let mut b_one = [2, 3, 20];
b_one = b_one.push_back(20);
// let b_one = Vec::from_slice([2, 3, 20]);
let foo_one = Foo { a: 1, b: b_one, bar: Bar { inner: [100, 101, 102] } };
let mut b_two = [5, 6, 21];
b_two = b_two.push_back(21);
// let b_two = Vec::from_slice([5, 6, 21]);
let foo_two = Foo { a: 4, b: b_two, bar: Bar { inner: [103, 104, 105] } };
let foo_three = Foo { a: 7, b: [8, 9, 22], bar: Bar { inner: [106, 107, 108] } };
let foo_four = Foo { a: 10, b: [11, 12, 23], bar: Bar { inner: [109, 110, 111] } };

let mut x = [foo_one, foo_two];
x = x.push_back(foo_three);
x = x.push_back(foo_four);

assert(x[y - 3].a == 1);
let struct_slice = x[y - 3].b;
for i in 0..4 {
assert(struct_slice[i] == b_one[i]);
}

assert(x[y - 2].a == 4);
let struct_slice = x[y - 2].b;
for i in 0..4 {
assert(struct_slice[i] == b_two[i]);
}

assert(x[y - 1].a == 7);
let struct_slice = x[y - 1].b;
assert(struct_slice[0] == 8);
assert(struct_slice[1] == 9);
assert(struct_slice[2] == 22);

assert(x[y].a == 10);
let struct_slice = x[y].b;
assert(struct_slice[0] == 11);
assert(struct_slice[1] == 12);
assert(struct_slice[2] == 23);
assert(x[y].bar.inner == [109, 110, 111]);

// TODO: Enable merging nested slices
// if y != 2 {
// x[y].a = 50;
// } else {
// x[y].a = 100;
// }
// assert(x[3].a == 50);

// if y == 2 {
// x[y - 1].b = [50, 51, 52];
// } else {
// x[y - 1].b = [100, 101, 102];
// }
// assert(x[2].b[0] == 100);
// assert(x[2].b[1] == 101);
// assert(x[2].b[2] == 102);

assert(x[y - 3].bar.inner == [100, 101, 102]);
assert(x[y - 2].bar.inner == [103, 104, 105]);
assert(x[y - 1].bar.inner == [106, 107, 108]);
assert(x[y].bar.inner == [109, 110, 111]);

let q = x.push_back(foo_four);
let foo_parent_one = FooParent { parent_arr: [0, 1, 2], foos: x };
let foo_parent_two = FooParent { parent_arr: [3, 4, 5], foos: q };
let mut foo_parents = [foo_parent_one];
foo_parents = foo_parents.push_back(foo_parent_two);

// TODO: Merging nested slices is broken
// if y == 2 {
// foo_parents[y - 2].foos[y - 1].b[y - 1] = 5000;
// } else {
// foo_parents[y - 2].foos[y - 1].b[y - 1] = 1000;
// }
assert(foo_parents[y - 2].foos[y - 1].a == 7);
foo_parents[y - 2].foos[y - 1].a = 50;
assert(foo_parents[y - 2].foos[y - 1].a == 50);

assert(foo_parents[y - 2].foos[y - 2].b[y - 1] == 21);
foo_parents[y - 2].foos[y - 2].b[y - 1] = 5000;
assert(foo_parents[y - 2].foos[y - 2].b[y - 1] == 5000);

let b_array = foo_parents[y - 2].foos[y - 3].b;
assert(b_array[0] == 2);
assert(b_array[1] == 3);
assert(b_array[2] == 20);
assert(b_array[3] == 20);

let b_array = foo_parents[y - 2].foos[y - 2].b;
assert(b_array[0] == 5);
assert(b_array[1] == 6);
assert(b_array[2] == 5000);
assert(b_array[3] == 21);

let b_array = foo_parents[y - 2].foos[y - 1].b;
assert(b_array[0] == 8);
assert(b_array[1] == 9);
assert(b_array[2] == 22);

// Fixed by #3410
// let b_array = foo_parents[y - 2].foos[y].b;
// assert(b_array[0] == 11);
// assert(b_array[1] == 12);
// assert(b_array[2] == 23);

// assert(foo_parents[y - 2].foos[y - 3].bar.inner == [100, 101, 102]);
// assert(foo_parents[y - 2].foos[y - 2].bar.inner == [103, 104, 105]);
// assert(foo_parents[y - 2].foos[y - 1].bar.inner == [106, 107, 108]);
}
Loading