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: Add support for slices of structs and nested slices in brillig #2084

Merged
merged 27 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
6884895
Start experiment to merge array and slice types
jfecher Jul 20, 2023
79440f2
Finish merger of slices and arrays
jfecher Jul 21, 2023
22f2ed3
Implement missing try_bind function
jfecher Jul 21, 2023
fca324d
Merge branch 'master' into jf/merge-array-and-slice-types
jfecher Jul 27, 2023
c550dbb
Add missed case for NotConstant
jfecher Jul 27, 2023
d06b69c
Fix some tests
jfecher Jul 27, 2023
a9089fc
Merge branch 'master' into jf/merge-array-and-slice-types
jfecher Jul 27, 2023
eb33561
Fix poseidon test
jfecher Jul 27, 2023
3e366d8
Fix evaluation of slice length
jfecher Jul 27, 2023
fe00354
Fix tests
jfecher Jul 27, 2023
704d9b8
fix: Initialize Value::Array of type Slice
sirasistant Jul 28, 2023
5c10ddb
test: improved brillig test after bug fix
sirasistant Jul 28, 2023
37aefbc
fix: Slice initialization (#2080)
sirasistant Jul 28, 2023
94cb291
feat: nested slices
sirasistant Jul 28, 2023
4b49b56
fix: insert and remove indexes
sirasistant Jul 28, 2023
fc915c3
Merge branch 'jf/merge-array-and-slice-types' into arv/nested_slices
sirasistant Jul 28, 2023
3a3f242
Merge branch 'master' into jf/merge-array-and-slice-types
jfecher Jul 28, 2023
742a55d
fix: add missing deallocation
sirasistant Jul 28, 2023
9f0bc88
Merge branch 'jf/merge-array-and-slice-types' into arv/nested_slices
sirasistant Jul 28, 2023
da0ae19
Merge branch 'master' into arv/nested_slices
sirasistant Jul 28, 2023
c9b37b7
Merge branch 'master' into arv/nested_slices
sirasistant Aug 7, 2023
be7674c
test: added more cases to nested slice test
sirasistant Aug 7, 2023
00b5438
refactor: simplify array length instructions
sirasistant Aug 7, 2023
0cbe9ce
test: fix stale import
sirasistant Aug 7, 2023
a731cba
Merge branch 'master' into arv/nested_slices
sirasistant Aug 7, 2023
2bdda12
refactor: reuse element_size and regen json
sirasistant Aug 7, 2023
d1c9f27
Update crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
sirasistant Aug 7, 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
@@ -0,0 +1,6 @@
[package]
name = "brillig_slices"
authors = [""]
compiler_version = "0.6.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
a = "5"
b = "10"
59 changes: 59 additions & 0 deletions crates/nargo_cli/tests/test_data/brillig_nested_slices/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
use dep::std::slice;
use dep::std;

// Tests nested slice passing to/from functions
unconstrained fn push_back_to_slice<T>(slice: [T], item: T) -> [T] {
slice.push_back(item)
}

struct NestedSliceStruct {
id: Field,
arr: [Field]
}

unconstrained fn create_foo(id: Field, value: Field) -> NestedSliceStruct {
let mut arr = [id];
arr = arr.push_back(value);
NestedSliceStruct { id, arr }
}

unconstrained fn main(a: Field, b: Field) {
let mut slice = [create_foo(a, b), create_foo(b, a)];
assert(slice.len() == 2);

assert(slice[0].id == a);
assert(slice[0].arr[0] == a);
assert(slice[1].id == b);
assert(slice[1].arr[1] == a);

slice = push_back_to_slice(slice, create_foo(0, 42));
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
assert(slice.len() == 3);

assert(slice[0].id == a);
assert(slice[0].arr[0] == a);
assert(slice[1].id == b);
assert(slice[1].arr[1] == a);

assert(slice[2].id == 0);
assert(slice[2].arr[0] == 0);
assert(slice[2].arr[1] == 42);

let mut remove_result = slice.remove(0);
slice = remove_result.0;
let mut removed_item = remove_result.1;
assert(removed_item.arr[0] == a);

remove_result = slice.remove(1);
slice = remove_result.0;
removed_item = remove_result.1;
assert(removed_item.arr[0] == 0);

let last_item = slice[0];

assert(last_item.id == b);
slice = slice.insert(1, removed_item);

assert(slice.len() == 2);
assert(slice[0].id == b);
assert(slice[1].id == 0);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"backend":"acvm-backend-barretenberg","abi":{"parameters":[{"name":"x","type":{"kind":"field"},"visibility":"private"},{"name":"y","type":{"kind":"field"},"visibility":"private"}],"param_witnesses":{"x":[1],"y":[2]},"return_type":null,"return_witnesses":[]},"bytecode":"H4sIAAAAAAAA/8WcTWwdVxXH53lemvhNkxRSmhA7jtOUhqZx8xybfiRpepsGN/1I46RJE5w4TYrjfOB8tBIrNkjsYYOQ2MAKCYklEhskFqxg0w1igYTEEokFC1ixQMAd5v/e7x1fv1bqPa9XiubMnDP3/z/nnnPPHdtKWRRFq/j/KP/3b6xYP6QPzbX72cZsK99cXU+eY04849jSBHq8iTv5jyfW4RHIIROnR/LHrLsJXLc18qa8GEdifNomPi1zHyBvQgw35+UyN14Mrs0ncdkMLlvycqlzbBxY4iWcCvoSNuN5edT1vcX4r3vy+7Rcxz5nrg64L8Q5Op/gfyfBozNC/4mvZ+3PmUtlODhxmasS2J3RxGEuzvlo5jkVO3EXZ+GU0D/dXLc2dlsTPm/Py6/eux4r+kOY2xH7bY1Mu8fAbRt80VX6GePTF/CefNqR16fZOMfj4CrMHfDpi41Mu8fBTfox+CT9vPHpS3hPPu3M69OROMcucBXmTvj0RCPvAg+P/XMX/BUm+5v0x02cvoz3xC93Psc5J/LOWZ81dheDY9hZYwL+7cnsX8SdwvwBGMTdmxd3lrit5p8w9LyEfKboj72QlQPiHPNiMmFHebd5p4J+0tnnPeARcC+smMsnYTOZ4L0dvKXXvhJzawJzyZdJB18mikFfJgxn9vjdflzqHm+xOxvEIXdvcKjJen/guscxbH/YA/8y52y9P0xj/gAM4u7LHFfian8Qhp6XkFeL/tgHWTkgzjEvphJ2lO3+UUE/5ewz96mAe2HFXL4Cm6kE7x3gzb00jphbezCXfJly8GWjvW4K/NqGuwOXen+w2J0N4pD7nOW152Su83rPYS7FMWzP2YuYZa6Des95EvMHYBB3f+a4Eld7jjD0vIT83aI/9kNWXolzzLXphB1luydV0E87+8y9L+BeWLE+vg2b6QTvneAtvfI+5hbPN/Jl2sGXjfbPafBrG+4OXOaqBHZngzh4fINzTYXJb/DpRk71wDb0/F6V/gfNVd9hT+I9+eTxDf4UuAqT3+DKV9o9BW774Yuu0v/I+PQVvCefPL7BD4CrMPkNrp/hHAAPj9gegL9Pm9iW0P/ExOmreE/8nsnLb86rLx/KHMe4/zxb9EdALIQV98KDxeAY1mefRVxnHPgeMrjiNgPc5/Li1v39MOYPwHgGz7sO/h5GzFvA0PMS8i+L/uAv/pTz4txBvLobvHPIvFNBP+Ps83PgEXAvrFi7PweHzHXRpY9xHDQxqaBXnY9DzxrInIvz5NYu1q9TCf2vm+vWwq9vdxNxYt9WbnYRE4++xNw4bNamhP63JiYevelIsT4m7PuzjUy7I+AvPc8y0v/O8PeI5XyCP3v8XCPTbh7858BbV+k/Nvw9ft/0fIL/o+D/tUaWnVcePI+YCJNnFOn/aGJyNDOXuKe+1Mylv4HRHiWsuH+9WAyOYX3+JcTumAPfowZX63gMuMfz4tZ9/mXMH4BBf084+PsyYt4Chp6XkP9a9McJyIqZOHcQrxMbvPOieaeC/pizz8fBI+BeWLFu/gwOmeuiSx/jOGjiGOP3QiIWHnvuUfgsTPYv6f/eXONeMQ5ezE+PPfU4+AmTv8OX/p/g53XmSOUvzxyqk1TOt6Fnf5X+X4a/R18ICf48H7xiMCm3oWd/lf4/hr9Hrp5M8Of54NVGlp1XHE8iJq+aOJbEbw3GZCEzlzj9qWYu9VfVo7Birb5WDI5h/fUUYvd6Xr51nzuN+QMwiPuGQ5xOw/cWMPS8hPwEAvRGX+zlnDjHnFtI2FF+zbxTQb/g7PPr4BFwL6yYvxV8XSjW+3oKvn49wdujvhbAT5isL+knUV/jRTqXPPah0+AnTPZM6feZ+vfoSW8W/aFcY0/SWtPuTfCXnj1J+gOGv8dav53gz570ViPT7m3wfwu8dZV+xpm/189bz2bmGcPwDuIX51WNCCvWD/92KI5hfULzRb6LefnWfeIc5g/A2IHn5x3idA6+t4Ch5yXkVxCg832xl8fiHPP4bMKO8hnzTgX9WWefF8Ej4F5Ysc7mG19b4CN7W7epOc8Ym+jLOei9feG6Uo42pxLc3s3Lrc7pC5g/AIO5ftEhJhfgbwsYel5CXkROX+yLvTUX55jT5xN2lBfNOxX05519fhc8Au6FVZ8t4Os52NtcraDnz931jHuhxzfwIjgLk9/A0l8awXkjtWfxvDFsn2tDz/OG9MsjOG9cSPDneUN5wlr1OENeQEyEyTOk9CsmJh5c3ivWx4Tf1aoZ2r0H/hfBW1fp747gDLaUec5I+XIzl3qEalxYsf4vFYNj2HlJ80W+V/LyrXvLVcwfgMGetuwQp6vwvQUMPS8hfwcBWu6LvZwT55hzSwk7ypfMOxX0S84+8+9KA+6FFWviIc5LS8Y+dRbKnBOzKdwlE7OUL5cMN7vG0eadBH/v3Aq4Xwbutby4dS29j/kDMFhj1x38fR/+toCh5yXk76OWrvfF3hqLcwfxoh3lK+adCvplZ5+vgUfAvbBiLX0Pvl6Ffap/f8PYefXMq+AnTPZv6X9ofkalWF925rcEflcS/KT/8QjOjFxj7S88MyrHaHcN/KXnmVH6n47gzHg9wZ85p5qk3XXwZ/3qKv3PRnA+Wsk8Z6T8QTOX9mbls7Birt8oBsew85Hmi3xv5uVb7+mrmD8Agz3slkOcVuF7Cxh6XkL+FQJ0qy/2ck6cY86tJOwo3zDvVNCvOPt8EzwC7oUVa+IXOB+tGHtbYw45MZvCXTExS/lyw3CzaxxtLif4e+dWwP0t4N7Oi1vX0h3MH4DBGrvr4O8d+NsChp6XkH+PWrrbF3trLM4dxIt2lG+adyrobzn7fBs8Au6FFWvpN/B1FfapXvXNhN0q5pKevUr6j81ZRnHh3u3Rf1fAz+7bJfR/GMFZ5nYivjzLKB9odxv8pedZRvo/OfOPc67lnXOWdah9SPkgrJgrqssCthwBsuaLfO9ljkHEvY/5AzC24/kDhzjdh+8tYOh5CflvCNCDvtjrP+Icc24tYUf5jnmngn7N2ed74BFwL6xYE3/BWWDN2Mtfrk3mnJhN4a6ZmKV8uWO42TWONh8k+HvnVsD9A+A+zItb19KHmD8AgzX2kYO/H8LfFjD0vIT8b9TSR32xt8bi3EG8aEf5nnmngv6Bs88PwSPgXlixlv4BX+/D3tZR9PVbCbv7mEt69irpx5qHOgsoLty7PfrvGvjZfbuEfjP4xf+DzvabbAuj4LSK9H8m+V/w7MsncVIAAA==","proving_key":null,"verification_key":null}
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,4 @@ unconstrained fn main(x: Field, y: Field) {
// Tests slice passing to/from functions
unconstrained fn push_front_to_slice<T>(slice: [T], item: T) -> [T] {
slice.push_front(item)
}
}
194 changes: 120 additions & 74 deletions crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
use crate::brillig::brillig_gen::brillig_slice_ops::{
convert_array_or_vector_to_vector, slice_push_back_operation,
};
use crate::brillig::brillig_ir::{
BrilligBinaryOp, BrilligContext, BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE,
};
Expand All @@ -20,18 +17,14 @@ use iter_extended::vecmap;

use super::brillig_black_box::convert_black_box_call;
use super::brillig_fn::FunctionContext;
use super::brillig_slice_ops::{
slice_insert_operation, slice_pop_back_operation, slice_pop_front_operation,
slice_push_front_operation, slice_remove_operation,
};

/// Generate the compilation artifacts for compiling a function into brillig bytecode.
pub(crate) struct BrilligBlock<'block> {
function_context: &'block mut FunctionContext,
pub(crate) function_context: &'block mut FunctionContext,
/// The basic block that is being converted
block_id: BasicBlockId,
pub(crate) block_id: BasicBlockId,
/// Context for creating brillig opcodes
brillig_context: &'block mut BrilligContext,
pub(crate) brillig_context: &'block mut BrilligContext,
}

impl<'block> BrilligBlock<'block> {
Expand Down Expand Up @@ -309,8 +302,19 @@ impl<'block> BrilligBlock<'block> {
dfg.instruction_results(instruction_id)[0],
dfg,
);
let param = self.convert_ssa_value(arguments[0], dfg);
self.brillig_context.length_of_variable_instruction(param, result_register);
let param_id = arguments[0];
let param_variable = self.convert_ssa_value(param_id, dfg);

self.brillig_context
.length_of_variable_instruction(param_variable, result_register);

let item_size = self.get_ssa_item_size(param_id, dfg);

self.brillig_context.usize_op_in_place(
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
result_register,
BinaryIntOp::UnsignedDiv,
item_size,
);
}
Value::Intrinsic(
Intrinsic::SlicePushBack
Expand Down Expand Up @@ -408,7 +412,11 @@ impl<'block> BrilligBlock<'block> {
};

let index_register = self.convert_ssa_register_value(*index, dfg);
self.convert_ssa_array_get(array_pointer, index_register, destination_variable);
self.retrieve_variable_from_array(
array_pointer,
index_register,
destination_variable,
);
}
Instruction::ArraySet { array, index, value } => {
let source_variable = self.convert_ssa_value(*array, dfg);
Expand Down Expand Up @@ -483,7 +491,7 @@ impl<'block> BrilligBlock<'block> {
.post_call_prep_returns_load_registers(&returned_registers, &saved_registers);
}

fn convert_ssa_array_get(
pub(crate) fn retrieve_variable_from_array(
&mut self,
array_pointer: RegisterIndex,
index_register: RegisterIndex,
Expand All @@ -496,7 +504,13 @@ impl<'block> BrilligBlock<'block> {
RegisterOrMemory::HeapArray(HeapArray { pointer, .. }) => {
self.brillig_context.array_get(array_pointer, index_register, pointer);
}
RegisterOrMemory::HeapVector(_) => unimplemented!("ICE: Array get for vector"),
RegisterOrMemory::HeapVector(..) => {
// Vectors are stored as references inside arrays to be able to match SSA indexes
let reference = self.brillig_context.allocate_register();
self.brillig_context.array_get(array_pointer, index_register, reference);
self.brillig_context.load_variable_instruction(destination_variable, reference);
self.brillig_context.deallocate_register(reference);
}
}
}

Expand Down Expand Up @@ -551,7 +565,7 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.deallocate_register(source_size_as_register);
}

fn store_variable_in_array(
pub(crate) fn store_variable_in_array(
&mut self,
destination_pointer: RegisterIndex,
index_register: RegisterIndex,
Expand All @@ -565,7 +579,12 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.array_set(destination_pointer, index_register, pointer);
}
RegisterOrMemory::HeapVector(_) => {
unimplemented!("ICE: cannot store a vector in array")
// Vectors are stored as references inside arrays to be able to match SSA indexes
let reference = self.brillig_context.allocate_register();
self.brillig_context.allocate_variable_instruction(reference);
self.brillig_context.store_variable_instruction(reference, value_variable);
self.brillig_context.array_set(destination_pointer, index_register, reference);
self.brillig_context.deallocate_register(reference);
}
}
}
Expand All @@ -578,9 +597,10 @@ impl<'block> BrilligBlock<'block> {
instruction_id: InstructionId,
arguments: &[ValueId],
) {
let source_variable = self.convert_ssa_value(arguments[0], dfg);
let source_vector =
convert_array_or_vector_to_vector(self.brillig_context, source_variable);
let slice_id = arguments[0];
let subitem_count = self.get_ssa_item_size(slice_id, dfg);
let source_variable = self.convert_ssa_value(slice_id, dfg);
let source_vector = self.convert_array_or_vector_to_vector(source_variable);

match intrinsic {
Value::Intrinsic(Intrinsic::SlicePushBack) => {
Expand All @@ -590,13 +610,10 @@ impl<'block> BrilligBlock<'block> {
dfg,
);
let target_vector = self.function_context.extract_heap_vector(target_variable);
let item_value = self.convert_ssa_register_value(arguments[1], dfg);
slice_push_back_operation(
self.brillig_context,
target_vector,
source_vector,
item_value,
);
let item_values = vecmap(&arguments[1..subitem_count + 1], |arg| {
self.convert_ssa_value(*arg, dfg)
});
self.slice_push_back_operation(target_vector, source_vector, &item_values);
}
Value::Intrinsic(Intrinsic::SlicePushFront) => {
let target_variable = self.function_context.create_variable(
Expand All @@ -605,90 +622,103 @@ impl<'block> BrilligBlock<'block> {
dfg,
);
let target_vector = self.function_context.extract_heap_vector(target_variable);
let item_value = self.convert_ssa_register_value(arguments[1], dfg);
slice_push_front_operation(
self.brillig_context,
target_vector,
source_vector,
item_value,
);
let item_values = vecmap(&arguments[1..subitem_count + 1], |arg| {
self.convert_ssa_value(*arg, dfg)
});
self.slice_push_front_operation(target_vector, source_vector, &item_values);
}
Value::Intrinsic(Intrinsic::SlicePopBack) => {
let results = dfg.instruction_results(instruction_id);

let target_variable =
self.function_context.create_variable(self.brillig_context, results[0], dfg);

let target_vector = self.function_context.extract_heap_vector(target_variable);

let pop_item = self.function_context.create_register_variable(
self.brillig_context,
results[1],
dfg,
);
let pop_variables = vecmap(&results[1..subitem_count + 1], |result| {
self.function_context.create_variable(self.brillig_context, *result, dfg)
});

slice_pop_back_operation(
self.brillig_context,
target_vector,
source_vector,
pop_item,
);
self.slice_pop_back_operation(target_vector, source_vector, &pop_variables);
}
Value::Intrinsic(Intrinsic::SlicePopFront) => {
let results = dfg.instruction_results(instruction_id);

let pop_item = self.function_context.create_register_variable(
let pop_variables = vecmap(&results[0..subitem_count], |result| {
self.function_context.create_variable(self.brillig_context, *result, dfg)
});

let target_variable = self.function_context.create_variable(
self.brillig_context,
results[0],
results[subitem_count],
dfg,
);
let target_variable =
self.function_context.create_variable(self.brillig_context, results[1], dfg);
let target_vector = self.function_context.extract_heap_vector(target_variable);

slice_pop_front_operation(
self.brillig_context,
target_vector,
source_vector,
pop_item,
);
self.slice_pop_front_operation(target_vector, source_vector, &pop_variables);
}
Value::Intrinsic(Intrinsic::SliceInsert) => {
let results = dfg.instruction_results(instruction_id);
let index = self.convert_ssa_register_value(arguments[1], dfg);
let item = self.convert_ssa_register_value(arguments[2], dfg);
let target_id = results[0];
let target_variable =
self.function_context.create_variable(self.brillig_context, results[0], dfg);
self.function_context.create_variable(self.brillig_context, target_id, dfg);

let target_vector = self.function_context.extract_heap_vector(target_variable);
slice_insert_operation(
self.brillig_context,
target_vector,
source_vector,
index,
item,

// TODO: Remove after https://github.com/noir-lang/noir/issues/2083 is fixed
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
let user_index = self.convert_ssa_register_value(arguments[1], dfg);

let converted_index = self
.brillig_context
.make_constant(self.get_ssa_item_size(target_id, dfg).into());

self.brillig_context.memory_op(
converted_index,
user_index,
converted_index,
BinaryIntOp::Mul,
);

let items = vecmap(&arguments[2..subitem_count + 2], |arg| {
self.convert_ssa_value(*arg, dfg)
});

self.slice_insert_operation(target_vector, source_vector, converted_index, &items);
self.brillig_context.deallocate_register(converted_index);
}
Value::Intrinsic(Intrinsic::SliceRemove) => {
let results = dfg.instruction_results(instruction_id);
let index = self.convert_ssa_register_value(arguments[1], dfg);
let target_id = results[0];

let target_variable =
self.function_context.create_variable(self.brillig_context, results[0], dfg);
self.function_context.create_variable(self.brillig_context, target_id, dfg);
let target_vector = self.function_context.extract_heap_vector(target_variable);

let removed_item_register = self.function_context.create_register_variable(
self.brillig_context,
results[1],
dfg,
// TODO: Remove after https://github.com/noir-lang/noir/issues/2083 is fixed
let user_index = self.convert_ssa_register_value(arguments[1], dfg);

let converted_index = self
.brillig_context
.make_constant(self.get_ssa_item_size(target_id, dfg).into());
self.brillig_context.memory_op(
converted_index,
user_index,
converted_index,
BinaryIntOp::Mul,
);

slice_remove_operation(
self.brillig_context,
let removed_items = vecmap(&results[1..subitem_count + 1], |result| {
self.function_context.create_variable(self.brillig_context, *result, dfg)
});

self.slice_remove_operation(
target_vector,
source_vector,
index,
removed_item_register,
converted_index,
&removed_items,
);

self.brillig_context.deallocate_register(converted_index);
}
_ => unreachable!("ICE: Slice operation not supported"),
}
Expand Down Expand Up @@ -890,6 +920,22 @@ impl<'block> BrilligBlock<'block> {
}
}
}

/// Gets the number of subitems an array item has.
/// This allows operations like returning the user-defined length of an array where every item has multiple fields.
/// Such as an array/slice of structs.
fn get_ssa_item_size(&self, array_id: ValueId, dfg: &DataFlowGraph) -> usize {
let array_type = dfg[array_id].get_type();
match array_type {
Type::Array(item_types, _) | Type::Slice(item_types) => {
// To match SSA indexes, all subitems are stored as one single value
item_types.len()
}
_ => {
unreachable!("ICE: Cannot get item size of {array_type:?}")
}
}
}
}

/// Returns the type of the operation considering the types of the operands
Expand Down
Loading