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: Dynamic indexing of non-homogenous slices #2883

Merged
merged 27 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 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
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
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ impl AcirContext {
self.brillig_array_input(var_expressions, var)?;
}
}
AcirValue::DynamicArray(AcirDynamicArray { block_id, len }) => {
AcirValue::DynamicArray(AcirDynamicArray { block_id, len, .. }) => {
for i in 0..len {
// We generate witnesses corresponding to the array values
let index_var = self.add_constant(FieldElement::from(i as u128));
Expand Down
227 changes: 170 additions & 57 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,8 @@ impl FunctionBuilder {
array: ValueId,
index: ValueId,
value: ValueId,
length: Option<ValueId>,
) -> ValueId {
self.insert_instruction(Instruction::ArraySet { array, index, value, length }, None).first()
self.insert_instruction(Instruction::ArraySet { array, index, value }, None).first()
}

/// Terminates the current block with the given terminator instruction
Expand Down
16 changes: 5 additions & 11 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,7 @@ pub(crate) enum Instruction {

/// Creates a new array with the new value at the given index. All other elements are identical
/// to those in the given array. This will not modify the original array.
///
/// An optional length can be provided to enable handling of dynamic slice indices.
ArraySet { array: ValueId, index: ValueId, value: ValueId, length: Option<ValueId> },
ArraySet { array: ValueId, index: ValueId, value: ValueId },
}

impl Instruction {
Expand Down Expand Up @@ -306,12 +304,9 @@ impl Instruction {
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array: f(*array), index: f(*index) }
}
Instruction::ArraySet { array, index, value, length } => Instruction::ArraySet {
array: f(*array),
index: f(*index),
value: f(*value),
length: length.map(f),
},
Instruction::ArraySet { array, index, value } => {
Instruction::ArraySet { array: f(*array), index: f(*index), value: f(*value) }
}
}
}

Expand Down Expand Up @@ -348,11 +343,10 @@ impl Instruction {
f(*array);
f(*index);
}
Instruction::ArraySet { array, index, value, length } => {
Instruction::ArraySet { array, index, value } => {
f(*array);
f(*index);
f(*value);
length.map(&mut f);
}
Instruction::EnableSideEffects { condition } => {
f(*condition);
Expand Down
11 changes: 3 additions & 8 deletions compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,19 +163,14 @@ pub(crate) fn display_instruction(
Instruction::ArrayGet { array, index } => {
writeln!(f, "array_get {}, index {}", show(*array), show(*index))
}
Instruction::ArraySet { array, index, value, length } => {
write!(
Instruction::ArraySet { array, index, value } => {
writeln!(
f,
"array_set {}, index {}, value {}",
show(*array),
show(*index),
show(*value)
)?;
if let Some(length) = length {
writeln!(f, ", length {}", show(*length))
} else {
writeln!(f)
}
)
}
}
}
34 changes: 32 additions & 2 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,7 @@ impl<'f> Context<'f> {
// 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 {
let zero = FieldElement::zero();
self.inserter.function.dfg.make_constant(zero, Type::field())
self.make_slice_dummy_data(element_type)
} else {
let get = Instruction::ArrayGet { array, index };
self.insert_instruction_with_typevars(get, typevars).first()
Expand All @@ -484,6 +483,37 @@ impl<'f> Context<'f> {
self.inserter.function.dfg.make_array(merged, typ)
}

/// Construct a dummy value to be attached to the smaller of two slices being merged.
/// We need to make sure we follow the internal element type structure of the slice type
/// even for dummy data to ensure that we do not have errors later in the compiler,
/// such as with dynamic indexing of non-homogenous slices.
fn make_slice_dummy_data(&mut self, typ: &Type) -> ValueId {
match typ {
Type::Numeric(_) => {
let zero = FieldElement::zero();
self.inserter.function.dfg.make_constant(zero, Type::field())
}
Type::Array(element_types, len) => {
let mut array = im::Vector::new();
for _ in 0..*len {
for typ in element_types.iter() {
array.push_back(self.make_slice_dummy_data(typ));
}
}
self.inserter.function.dfg.make_array(array, typ.clone())
}
Type::Slice(_) => {
unreachable!("ICE: Slices of slice is unsupported")
}
Type::Reference => {
unreachable!("ICE: Merging references is unsupported")
}
Type::Function => {
unreachable!("ICE: Merging functions is unsupported")
}
}
}

fn get_slice_length(&mut self, value_id: ValueId) -> usize {
let value = &self.inserter.function.dfg[value_id];
match value {
Expand Down
14 changes: 4 additions & 10 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,19 +661,14 @@ impl<'a> FunctionContext<'a> {
match lvalue {
LValue::Ident => unreachable!("Cannot assign to a variable without a reference"),
LValue::Index { old_array: mut array, index, array_lvalue, location } => {
array = self.assign_lvalue_index(new_value, array, index, None, location);
array = self.assign_lvalue_index(new_value, array, index, location);
self.assign_new_value(*array_lvalue, array.into());
}
LValue::SliceIndex { old_slice: slice, index, slice_lvalue, location } => {
let mut slice_values = slice.into_value_list(self);

slice_values[1] = self.assign_lvalue_index(
new_value,
slice_values[1],
index,
Some(slice_values[0]),
location,
);
slice_values[1] =
self.assign_lvalue_index(new_value, slice_values[1], index, location);

// The size of the slice does not change in a slice index assignment so we can reuse the same length value
let new_slice = Tree::Branch(vec![slice_values[0].into(), slice_values[1].into()]);
Expand All @@ -694,7 +689,6 @@ impl<'a> FunctionContext<'a> {
new_value: Values,
mut array: ValueId,
index: ValueId,
length: Option<ValueId>,
location: Location,
) -> ValueId {
let element_size = self.builder.field_constant(self.element_size(array));
Expand All @@ -706,7 +700,7 @@ impl<'a> FunctionContext<'a> {

new_value.for_each(|value| {
let value = value.eval(self);
array = self.builder.insert_array_set(array, index, value, length);
array = self.builder.insert_array_set(array, index, value);
index = self.builder.insert_binary(index, BinaryOp::Add, one);
});
array
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "nested_slice_dynamic"
type = "bin"
authors = [""]
compiler_version = "0.13.0"

[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,49 @@
struct Bar {
inner: [Field; 3],
}

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

fn main(y : Field) {
let foo_one = Foo { a: 1, b: [2, 3, 20], bar: Bar { inner: [100, 101, 102] } };
let foo_two = Foo { a: 4, b: [5, 6, 21], 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];
x = x.push_back(foo_two);
x = x.push_back(foo_three);
x = x.push_back(foo_four);

assert(x[y - 3].a == 1);
assert(x[y - 3].b == [2, 3, 20]);
assert(x[y - 2].a == 4);
assert(x[y - 2].b == [5, 6, 21]);
assert(x[y - 1].a == 7);
assert(x[y - 1].b == [8, 9, 22]);
assert(x[y].a == 10);
assert(x[y].b == [11, 12, 23]);
assert(x[y].bar.inner == [109, 110, 111]);

if y != 2 {
x[y - 2].a = 50;
} else {
x[y - 2].a = 100;
}
assert(x[y - 2].a == 50);

if y == 2 {
x[y - 1].b = [50, 51, 52];
} else {
x[y - 1].b = [100, 101, 102];
}
assert(x[2].b == [100, 101, 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]);
}
Loading