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: Complex slice inputs for dynamic slice builtins #3617

Merged
merged 118 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
118 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
ce13374
fill internal slices pass
vezenovm Oct 23, 2023
554b8a9
cleanup
vezenovm Oct 23, 2023
c2d7ce7
cleanup
vezenovm Oct 23, 2023
27e0835
fix up acir gen to not use value ids with slice values
vezenovm Oct 23, 2023
1456af9
Merge branch 'master' into mv/slice-struct-fields
vezenovm Oct 23, 2023
77b394e
fix for terminator in fill internal slices path
vezenovm Oct 23, 2023
3850cf1
Update compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs
TomAFrench Oct 23, 2023
db60a52
working nested array get/set w/ no regressions
vezenovm Oct 25, 2023
8cc5993
Merge branch 'mv/fill-slices-pass' into mv/fill-slices-pass-dbg
vezenovm Oct 25, 2023
a8d0431
working basic slice intrinsics on nested slices
vezenovm Oct 26, 2023
f9b8544
debugging changes for no regressions and changes to ACIR gen
vezenovm Oct 26, 2023
00a2f47
remove gate regression to get slice push back working for nested slices
vezenovm Oct 26, 2023
43f11e5
PR comment on removing from slice
vezenovm Oct 26, 2023
0d39a6b
improve fill internal slices pass
vezenovm Oct 27, 2023
6beabc2
cargo fmt
vezenovm Oct 27, 2023
42c0547
included simple unit test to fill internal slices pass
vezenovm Oct 30, 2023
5236691
cleanup
vezenovm Oct 30, 2023
c31906d
missed cargo fmt
vezenovm Oct 30, 2023
72a8a2a
spell check
vezenovm Oct 30, 2023
f231731
remove broken push back
vezenovm Oct 30, 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
e3c9327
starting work on update acir gen for nested inputs to slice intrinsic…
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
52d6ea4
merge conflcits after mv/slice-struct-fields updates
vezenovm Nov 1, 2023
776e593
initial testing and debugging to get nested slice inputs
vezenovm Nov 3, 2023
328b830
merge conflicts after 3410 merged to master
vezenovm Nov 3, 2023
b49341a
working basic push back with resolving value parents, code a mess nee…
vezenovm Nov 6, 2023
d3c3bf1
working SlicePushFront and SlicePopBack w/ nested dynamic struct inpu…
vezenovm Nov 7, 2023
ae3231f
add push front back to complex slice intrinsics test
vezenovm Nov 7, 2023
a9af033
working pop back with push back
vezenovm Nov 7, 2023
b2d171e
worknig pop front
vezenovm Nov 20, 2023
882c6be
working insert and remove
vezenovm Nov 27, 2023
4a61c18
all tests passing
vezenovm Nov 27, 2023
5784719
cleanup internal slice pass debugging a bit
vezenovm Nov 27, 2023
f200a3d
cargo fmt
vezenovm Nov 27, 2023
b21a101
merge w/ master
vezenovm Nov 27, 2023
802e425
remove unused new_with_generated_ssa method
vezenovm Nov 27, 2023
30826c7
cleanup fill_internal_slices more
vezenovm Nov 27, 2023
5cd0677
cargo fmt
vezenovm Nov 27, 2023
62ebdb9
cleanup the way we replace slice array values
vezenovm Nov 28, 2023
0e6115a
cargo fmt
vezenovm Nov 28, 2023
56aac2d
merge conflicts after test_programs move
vezenovm Nov 28, 2023
9c11d73
comments and formatting
vezenovm Nov 28, 2023
71a83d9
remove slice_args
vezenovm Nov 28, 2023
8144882
clippy
vezenovm Nov 28, 2023
ab0964a
merge conflicts w/ master
vezenovm Nov 28, 2023
7baaa2e
cargo fmt
vezenovm Nov 29, 2023
3d379b7
merge conflicts w/ master
vezenovm Nov 29, 2023
cbf5351
Update compiler/noirc_evaluator/src/ssa/ir/types.rs
vezenovm Nov 30, 2023
b286cf2
unpack arguments in SlicePushBack
vezenovm Nov 30, 2023
b47aa2f
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 30, 2023
64c9e69
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 30, 2023
f5397a7
comments and more unpacking in the intrinsics
vezenovm Nov 30, 2023
10488af
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 30, 2023
e87d771
rename predicates and clippy
vezenovm Nov 30, 2023
da4c191
cleanup and more renaming
vezenovm Nov 30, 2023
02fbaa0
Merge branch 'master' into mv/dyn-nested-slice-builtin
vezenovm Nov 30, 2023
23253e9
remove unnecessary reads from insert
vezenovm Nov 30, 2023
c3d7c8c
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 30, 2023
9947f6c
Update compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs
vezenovm Nov 30, 2023
2a69c91
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 30, 2023
ac02e52
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
vezenovm Nov 30, 2023
7a4fc8d
add comments to intrinsics arguments
vezenovm Nov 30, 2023
f41e354
Merge branch 'master' into mv/dyn-nested-slice-builtin
vezenovm Nov 30, 2023
3ab0504
feat: Dynamic index for slice insert and remove (#3624)
vezenovm Nov 30, 2023
32fe5bb
merge conflicts w/ master
vezenovm Nov 30, 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 @@ -937,7 +937,7 @@ impl AcirContext {

/// Returns an `AcirVar` which will be `1` if lhs >= rhs
/// and `0` otherwise.
fn more_than_eq_var(
pub(crate) fn more_than_eq_var(
&mut self,
lhs: AcirVar,
rhs: AcirVar,
Expand Down
629 changes: 511 additions & 118 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

Large diffs are not rendered by default.

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

pub(crate) fn is_nested_slice(&self) -> bool {
if let Type::Slice(element_types) = self {
element_types.as_ref().iter().any(|typ| typ.contains_slice_element())
} else {
false
}
}

/// True if this type is an array (or slice) or internally contains an array (or slice)
pub(crate) fn contains_an_array(&self) -> bool {
match self {
Expand Down
94 changes: 78 additions & 16 deletions compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ impl<'f> Context<'f> {
panic!("ICE: should have inner slice set for {slice_value}")
});
slice_sizes.insert(results[0], inner_slice.clone());
if slice_value != results[0] {
self.mapped_slice_values.insert(slice_value, results[0]);
}
}
}
}
Expand All @@ -198,17 +201,11 @@ impl<'f> Context<'f> {

let inner_sizes = slice_sizes.get_mut(array).expect("ICE expected slice sizes");
inner_sizes.1.push(*value);

let value_parent = self.resolve_slice_parent(*value);
if slice_values.contains(&value_parent) {
// Map the value parent to the current array in case nested slices
// from the current array are set to larger values later in the program
self.mapped_slice_values.insert(value_parent, *array);
}
}

if let Some(inner_sizes) = slice_sizes.get_mut(array) {
let inner_sizes = inner_sizes.clone();

slice_sizes.insert(results[0], inner_sizes);

self.mapped_slice_values.insert(*array, results[0]);
Expand All @@ -224,14 +221,27 @@ impl<'f> Context<'f> {
| Intrinsic::SlicePopBack
| Intrinsic::SliceInsert
| Intrinsic::SliceRemove => (1, 1),
Intrinsic::SlicePopFront => (1, 2),
// `pop_front` returns the popped element, and then the respective slice.
// This means in the case of a slice with structs, the result index of the popped slice
// will change depending on the number of elements in the struct.
// For example, a slice with four elements will look as such in SSA:
// v3, v4, v5, v6, v7, v8 = call slice_pop_front(v1, v2)
// where v7 is the slice length and v8 is the popped slice itself.
Intrinsic::SlicePopFront => (1, results.len() - 1),
_ => return,
};
let slice_contents = arguments[argument_index];
match intrinsic {
Intrinsic::SlicePushBack
| Intrinsic::SlicePushFront
| Intrinsic::SliceInsert => {
let slice_contents = arguments[argument_index];
for arg in &arguments[(argument_index + 1)..] {
let element_typ = self.inserter.function.dfg.type_of_value(*arg);
if element_typ.contains_slice_element() {
slice_values.push(*arg);
self.compute_slice_sizes(*arg, slice_sizes);
}
}
if let Some(inner_sizes) = slice_sizes.get_mut(&slice_contents) {
inner_sizes.0 += 1;

Expand All @@ -240,12 +250,12 @@ impl<'f> Context<'f> {

self.mapped_slice_values
.insert(slice_contents, results[result_index]);
self.slice_parents.insert(results[result_index], slice_contents);
}
}
Intrinsic::SlicePopBack
| Intrinsic::SlicePopFront
| Intrinsic::SliceRemove => {
let slice_contents = arguments[argument_index];
| Intrinsic::SliceRemove
| Intrinsic::SlicePopFront => {
// We do not decrement the size on intrinsics that could remove values from a slice.
// This is because we could potentially go back to the smaller slice and not fill in dummies.
// This pass should be tracking the potential max that a slice ***could be***
Expand All @@ -255,6 +265,7 @@ impl<'f> Context<'f> {

self.mapped_slice_values
.insert(slice_contents, results[result_index]);
self.slice_parents.insert(results[result_index], slice_contents);
}
}
_ => {}
Expand All @@ -277,7 +288,6 @@ impl<'f> Context<'f> {
if slice_values.contains(array) {
let (new_array_op_instr, call_stack) =
self.get_updated_array_op_instr(*array, slice_sizes, instruction);

self.inserter.push_instruction_value(
new_array_op_instr,
instruction,
Expand All @@ -288,6 +298,55 @@ impl<'f> Context<'f> {
self.inserter.push_instruction(instruction, block);
}
}
Instruction::Call { func: _, arguments } => {
let mut args_to_replace = Vec::new();
for (i, arg) in arguments.iter().enumerate() {
let element_typ = self.inserter.function.dfg.type_of_value(*arg);
if slice_values.contains(arg) && element_typ.contains_slice_element() {
args_to_replace.push((i, *arg));
}
}
if args_to_replace.is_empty() {
self.inserter.push_instruction(instruction, block);
} else {
// Using the original slice is ok to do as during collection of slice information
// we guarantee that only the arguments to slice intrinsic calls can be replaced.
let slice_contents = arguments[1];

let element_typ = self.inserter.function.dfg.type_of_value(arguments[1]);
let elem_depth = Self::compute_nested_slice_depth(&element_typ);

let mut max_sizes = Vec::new();
max_sizes.resize(elem_depth, 0);
// We want the max for the parent of the argument
let parent = self.resolve_slice_parent(slice_contents);
self.compute_slice_max_sizes(parent, slice_sizes, &mut max_sizes, 0);

for (index, arg) in args_to_replace {
let element_typ = self.inserter.function.dfg.type_of_value(arg);
max_sizes.remove(0);
let new_array =
self.attach_slice_dummies(&element_typ, Some(arg), false, &max_sizes);

let instruction_id = instruction;
let (instruction, call_stack) =
self.inserter.map_instruction(instruction_id);
let new_call_instr = match instruction {
Instruction::Call { func, mut arguments } => {
arguments[index] = new_array;
Instruction::Call { func, arguments }
}
_ => panic!("Expected call instruction"),
};
self.inserter.push_instruction_value(
new_call_instr,
instruction_id,
block,
call_stack,
);
}
}
}
_ => {
self.inserter.push_instruction(instruction, block);
}
Expand All @@ -314,6 +373,7 @@ impl<'f> Context<'f> {
let typ = self.inserter.function.dfg.type_of_value(array_id);
let depth = Self::compute_nested_slice_depth(&typ);
max_sizes.resize(depth, 0);

max_sizes[0] = *current_size;
self.compute_slice_max_sizes(array_id, slice_sizes, &mut max_sizes, 1);

Expand Down Expand Up @@ -370,9 +430,12 @@ impl<'f> Context<'f> {
if let Some(value) = value {
let mut slice = im::Vector::new();

let array = match self.inserter.function.dfg[value].clone() {
let value = self.inserter.function.dfg[value].clone();
let array = match value {
Value::Array { array, .. } => array,
_ => panic!("Expected an array value"),
_ => {
panic!("Expected an array value");
}
};

if is_parent_slice {
Expand Down Expand Up @@ -487,7 +550,6 @@ impl<'f> Context<'f> {
self.compute_slice_max_sizes(*inner_slice, slice_sizes, max_sizes, depth + 1);
}

max_sizes[depth] = max;
if max > max_sizes[depth] {
max_sizes[depth] = max;
}
Expand Down
16 changes: 2 additions & 14 deletions noir_stdlib/src/slice.nr
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,16 @@ impl<T> [T] {
#[builtin(slice_pop_front)]
pub fn pop_front(_self: Self) -> (T, Self) { }

pub fn insert(self, _index: Field, _elem: T) -> Self {
// TODO(#2462): Slice insert with a dynamic index
crate::assert_constant(_index);
self.__slice_insert(_index, _elem)
}

/// Insert an element at a specified index, shifting all elements
/// after it to the right
#[builtin(slice_insert)]
fn __slice_insert(_self: Self, _index: Field, _elem: T) -> Self { }

pub fn remove(self, _index: Field) -> (Self, T) {
// TODO(#2462): Slice remove with a dynamic index
crate::assert_constant(_index);
self.__slice_remove(_index)
}
pub fn insert(_self: Self, _index: Field, _elem: T) -> Self { }

/// Remove an element at a specified index, shifting all elements
/// after it to the left, returning the altered slice and
/// the removed element
#[builtin(slice_remove)]
fn __slice_remove(_self: Self, _index: Field) -> (Self, T) { }
pub fn remove(_self: Self, _index: Field) -> (Self, T) { }

// Append each element of the `other` slice to the end of `self`.
// This returns a new slice and leaves both input slices unchanged.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,13 @@ fn dynamic_slice_merge_if(mut slice: [Field], x: Field) {
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);

slice = rest_of_slice.insert(x - 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);

let (removed_slice, removed_elem) = slice.remove(x - 1);
// The deconstructed tuple assigns to the slice but is not seen outside of the if statement
// without a direct assignment
slice = removed_slice;
Expand Down
Loading
Loading