Skip to content

Commit

Permalink
feat(acir): Handle dynamic array operations for nested slices (#3187)
Browse files Browse the repository at this point in the history
Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
  • Loading branch information
4 people committed Nov 14, 2023
1 parent 84d7dde commit f944ef1
Show file tree
Hide file tree
Showing 8 changed files with 333 additions and 66 deletions.
233 changes: 174 additions & 59 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

Large diffs are not rendered by default.

12 changes: 12 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,18 @@ 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,
Type::Reference => false,
Type::Function => false,
}
}

/// 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);

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]);
}

0 comments on commit f944ef1

Please sign in to comment.