Skip to content

Commit

Permalink
fix: Implement slices of structs (#2150)
Browse files Browse the repository at this point in the history
* Implement slices of structs

* Add missed function call

* Use element_size in element_size
  • Loading branch information
jfecher authored Aug 3, 2023
1 parent 159d048 commit 6abcb79
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 36 deletions.
34 changes: 34 additions & 0 deletions crates/nargo_cli/tests/test_data/slices/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,39 @@ fn main(x : Field, y : pub Field) {
assert(removed_elem == 2);
assert(remove_slice[3] == 3);
assert(remove_slice.len() == 4);

regression_2083();
}

// Ensure that slices of struct/tuple values work.
fn regression_2083() {
let y = [(1, 2)];
let y = y.push_back((3, 4)); // [(1, 2), (3, 4)]
let y = y.push_back((5, 6)); // [(1, 2), (3, 4), (5, 6)]
assert(y[2].1 == 6);

let y = y.push_front((10, 11)); // [(10, 11), (1, 2), (3, 4), (5, 6)]
let y = y.push_front((12, 13)); // [(12, 13), (10, 11), (1, 2), (3, 4), (5, 6)]

assert(y[1].0 == 10);

let y = y.insert(1, (55, 56)); // [(12, 13), (55, 56), (10, 11), (1, 2), (3, 4), (5, 6)]
assert(y[0].1 == 13);
assert(y[1].1 == 56);
assert(y[2].0 == 10);

let (y, x) = y.remove(2); // [(12, 13), (55, 56), (1, 2), (3, 4), (5, 6)]
assert(y[2].0 == 1);
assert(x.0 == 10);
assert(x.1 == 11);

let (x, y) = y.pop_front(); // [(55, 56), (1, 2), (3, 4), (5, 6)]
assert(y[0].0 == 55);
assert(x.0 == 12);
assert(x.1 == 13);

let (y, x) = y.pop_back(); // [(55, 56), (1, 2), (3, 4)]
assert(y.len() == 3);
assert(x.0 == 5);
assert(x.1 == 6);
}
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ impl DataFlowGraph {
/// Otherwise, this returns None.
pub(crate) fn get_array_constant(&self, value: ValueId) -> Option<(im::Vector<ValueId>, Type)> {
match &self.values[self.resolve(value)] {
// Vectors are shared, so cloning them is cheap
// Arrays are shared, so cloning them is cheap
Value::Array { array, typ } => Some((array.clone(), typ.clone())),
_ => None,
}
Expand Down
96 changes: 65 additions & 31 deletions crates/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::rc::Rc;
use std::{collections::VecDeque, rc::Rc};

use acvm::{acir::BlackBoxFunc, BlackBoxResolutionError, FieldElement};
use iter_extended::vecmap;
Expand Down Expand Up @@ -53,22 +53,22 @@ pub(super) fn simplify_call(
}
Intrinsic::ArrayLen => {
let slice = dfg.get_array_constant(arguments[0]);
if let Some((slice, _)) = slice {
SimplifyResult::SimplifiedTo(
dfg.make_constant((slice.len() as u128).into(), Type::field()),
)
if let Some((slice, typ)) = slice {
let length = FieldElement::from((slice.len() / typ.element_size()) as u128);
SimplifyResult::SimplifiedTo(dfg.make_constant(length, Type::field()))
} else if let Some(length) = dfg.try_get_array_length(arguments[0]) {
SimplifyResult::SimplifiedTo(
dfg.make_constant((length as u128).into(), Type::field()),
)
let length = FieldElement::from(length as u128);
SimplifyResult::SimplifiedTo(dfg.make_constant(length, Type::field()))
} else {
SimplifyResult::None
}
}
Intrinsic::SlicePushBack => {
let slice = dfg.get_array_constant(arguments[0]);
if let (Some((mut slice, element_type)), elem) = (slice, arguments[1]) {
slice.push_back(elem);
if let Some((mut slice, element_type)) = slice {
for elem in &arguments[1..] {
slice.push_back(*elem);
}
let new_slice = dfg.make_array(slice, element_type);
SimplifyResult::SimplifiedTo(new_slice)
} else {
Expand All @@ -77,8 +77,10 @@ pub(super) fn simplify_call(
}
Intrinsic::SlicePushFront => {
let slice = dfg.get_array_constant(arguments[0]);
if let (Some((mut slice, element_type)), elem) = (slice, arguments[1]) {
slice.push_front(elem);
if let Some((mut slice, element_type)) = slice {
for elem in arguments[1..].iter().rev() {
slice.push_front(*elem);
}
let new_slice = dfg.make_array(slice, element_type);
SimplifyResult::SimplifiedTo(new_slice)
} else {
Expand All @@ -87,34 +89,58 @@ pub(super) fn simplify_call(
}
Intrinsic::SlicePopBack => {
let slice = dfg.get_array_constant(arguments[0]);
if let Some((mut slice, element_type)) = slice {
let elem =
slice.pop_back().expect("There are no elements in this slice to be removed");
let new_slice = dfg.make_array(slice, element_type);
SimplifyResult::SimplifiedToMultiple(vec![new_slice, elem])
if let Some((mut slice, typ)) = slice {
let element_count = typ.element_size();
let mut results = VecDeque::with_capacity(element_count + 1);

// We must pop multiple elements in the case of a slice of tuples
for _ in 0..element_count {
let elem = slice
.pop_back()
.expect("There are no elements in this slice to be removed");
results.push_front(elem);
}

let new_slice = dfg.make_array(slice, typ);
results.push_front(new_slice);

SimplifyResult::SimplifiedToMultiple(results.into())
} else {
SimplifyResult::None
}
}
Intrinsic::SlicePopFront => {
let slice = dfg.get_array_constant(arguments[0]);
if let Some((mut slice, element_type)) = slice {
let elem =
slice.pop_front().expect("There are no elements in this slice to be removed");
let new_slice = dfg.make_array(slice, element_type);
SimplifyResult::SimplifiedToMultiple(vec![elem, new_slice])
if let Some((mut slice, typ)) = slice {
let element_count = typ.element_size();

// We must pop multiple elements in the case of a slice of tuples
let mut results = vecmap(0..element_count, |_| {
slice.pop_front().expect("There are no elements in this slice to be removed")
});

let new_slice = dfg.make_array(slice, typ);

// The slice is the last item returned for pop_front
results.push(new_slice);
SimplifyResult::SimplifiedToMultiple(results)
} else {
SimplifyResult::None
}
}
Intrinsic::SliceInsert => {
let slice = dfg.get_array_constant(arguments[0]);
let index = dfg.get_numeric_constant(arguments[1]);
if let (Some((mut slice, element_type)), Some(index), value) =
(slice, index, arguments[2])
{
slice.insert(index.to_u128() as usize, value);
let new_slice = dfg.make_array(slice, element_type);
if let (Some((mut slice, typ)), Some(index)) = (slice, index) {
let elements = &arguments[2..];
let mut index = index.to_u128() as usize * elements.len();

for elem in &arguments[2..] {
slice.insert(index, *elem);
index += 1;
}

let new_slice = dfg.make_array(slice, typ);
SimplifyResult::SimplifiedTo(new_slice)
} else {
SimplifyResult::None
Expand All @@ -123,10 +149,18 @@ pub(super) fn simplify_call(
Intrinsic::SliceRemove => {
let slice = dfg.get_array_constant(arguments[0]);
let index = dfg.get_numeric_constant(arguments[1]);
if let (Some((mut slice, element_type)), Some(index)) = (slice, index) {
let removed_elem = slice.remove(index.to_u128() as usize);
let new_slice = dfg.make_array(slice, element_type);
SimplifyResult::SimplifiedToMultiple(vec![new_slice, removed_elem])
if let (Some((mut slice, typ)), Some(index)) = (slice, index) {
let element_count = typ.element_size();
let mut results = Vec::with_capacity(element_count + 1);
let index = index.to_u128() as usize * element_count;

for _ in 0..element_count {
results.push(slice.remove(index));
}

let new_slice = dfg.make_array(slice, typ);
results.insert(0, new_slice);
SimplifyResult::SimplifiedToMultiple(results)
} else {
SimplifyResult::None
}
Expand Down
11 changes: 11 additions & 0 deletions crates/noirc_evaluator/src/ssa/ir/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ impl Type {
pub(crate) fn field() -> Type {
Type::Numeric(NumericType::NativeField)
}

/// Returns the size of the element type for this array/slice.
/// The size of a type is defined as representing how many Fields are needed
/// to represent the type. This is 1 for every primitive type, and is the number of fields
/// for any flattened tuple type.
pub(crate) fn element_size(&self) -> usize {
match self {
Type::Array(elements, _) | Type::Slice(elements) => elements.len(),
other => panic!("element_size: Expected array or slice, found {other}"),
}
}
}

/// Composite Types are essentially flattened struct or tuple types.
Expand Down
6 changes: 2 additions & 4 deletions crates/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,10 +641,8 @@ impl<'a> FunctionContext<'a> {
}

fn element_size(&self, array: ValueId) -> FieldElement {
match self.builder.type_of_value(array) {
Type::Array(elements, _) | Type::Slice(elements) => (elements.len() as u128).into(),
t => panic!("Uncaught type error: tried to take element size of non-array type {t}"),
}
let size = self.builder.type_of_value(array).element_size();
FieldElement::from(size as u128)
}

/// Given an lhs containing only references, create a store instruction to store each value of
Expand Down

0 comments on commit 6abcb79

Please sign in to comment.