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: Make arrays and slices polymorphic over each other #2070

Merged
merged 16 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
6 changes: 3 additions & 3 deletions crates/nargo_cli/tests/test_data/array_len/src/main.nr
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use dep::std;

fn len_plus_1<T>(array: [T]) -> Field {
fn len_plus_1<T, N>(array: [T; N]) -> Field {
array.len() + 1
}

fn add_lens<T>(a: [T], b: [Field]) -> Field {
fn add_lens<T, N, M>(a: [T; N], b: [Field; M]) -> Field {
a.len() + b.len()
}

fn nested_call(b: [Field]) -> Field {
fn nested_call<N>(b: [Field; N]) -> Field {
len_plus_1(b)
}

Expand Down
98 changes: 51 additions & 47 deletions crates/nargo_cli/tests/test_data/brillig_slices/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -2,71 +2,75 @@ use dep::std::slice;
use dep::std;

unconstrained fn main(x: Field, y: Field) {
// Mark it as mut so the compiler doesn't simplify the following operations
// But don't reuse the mut slice variable until this is fixed https://github.com/noir-lang/noir/issues/1931
let slice: [Field] = [y, x];
let mut slice: [Field] = [y, x];
assert(slice.len() == 2);

let mut pushed_back_slice = slice.push_back(7);
assert(pushed_back_slice.len() == 3);
assert(pushed_back_slice[0] == y);
assert(pushed_back_slice[1] == x);
assert(pushed_back_slice[2] == 7);
slice = slice.push_back(7);
assert(slice.len() == 3);
assert(slice[0] == y);
assert(slice[1] == x);
assert(slice[2] == 7);

// Array set on slice target
pushed_back_slice[0] = x;
pushed_back_slice[1] = y;
pushed_back_slice[2] = 1;

assert(pushed_back_slice[0] == x);
assert(pushed_back_slice[1] == y);
assert(pushed_back_slice[2] == 1);

assert(slice.len() == 2);

let pushed_front_slice = pushed_back_slice.push_front(2);
assert(pushed_front_slice.len() == 4);
assert(pushed_front_slice[0] == 2);
assert(pushed_front_slice[1] == x);
assert(pushed_front_slice[2] == y);
assert(pushed_front_slice[3] == 1);

let (item, popped_front_slice) = pushed_front_slice.pop_front();
slice[0] = x;
slice[1] = y;
slice[2] = 1;

assert(slice[0] == x);
assert(slice[1] == y);
assert(slice[2] == 1);

slice = push_front_to_slice(slice, 2);
assert(slice.len() == 4);
assert(slice[0] == 2);
assert(slice[1] == x);
assert(slice[2] == y);
assert(slice[3] == 1);

let (item, popped_front_slice) = slice.pop_front();
slice = popped_front_slice;
assert(item == 2);

assert(popped_front_slice.len() == 3);
assert(popped_front_slice[0] == x);
assert(popped_front_slice[1] == y);
assert(popped_front_slice[2] == 1);
assert(slice.len() == 3);
assert(slice[0] == x);
assert(slice[1] == y);
assert(slice[2] == 1);

let (popped_back_slice, another_item) = popped_front_slice.pop_back();
let (popped_back_slice, another_item) = slice.pop_back();
slice = popped_back_slice;
assert(another_item == 1);

assert(popped_back_slice.len() == 2);
assert(popped_back_slice[0] == x);
assert(popped_back_slice[1] == y);
assert(slice.len() == 2);
assert(slice[0] == x);
assert(slice[1] == y);

let inserted_slice = popped_back_slice.insert(1, 2);
assert(inserted_slice.len() == 3);
assert(inserted_slice[0] == x);
assert(inserted_slice[1] == 2);
assert(inserted_slice[2] == y);
slice = slice.insert(1, 2);
assert(slice.len() == 3);
assert(slice[0] == x);
assert(slice[1] == 2);
assert(slice[2] == y);

let (removed_slice, should_be_2) = inserted_slice.remove(1);
let (removed_slice, should_be_2) = slice.remove(1);
slice = removed_slice;
assert(should_be_2 == 2);

assert(removed_slice.len() == 2);
assert(removed_slice[0] == x);
assert(removed_slice[1] == y);
assert(slice.len() == 2);
assert(slice[0] == x);
assert(slice[1] == y);

let (slice_with_only_x, should_be_y) = removed_slice.remove(1);
let (slice_with_only_x, should_be_y) = slice.remove(1);
slice = slice_with_only_x;
assert(should_be_y == y);

assert(slice_with_only_x.len() == 1);
assert(removed_slice[0] == x);
assert(slice.len() == 1);
assert(slice[0] == x);

let (empty_slice, should_be_x) = slice_with_only_x.remove(0);
let (empty_slice, should_be_x) = slice.remove(0);
assert(should_be_x == x);
assert(empty_slice.len() == 0);
}

// Tests slice passing to/from functions
unconstrained fn push_front_to_slice<T>(slice: [T], item: T) -> [T] {
slice.push_front(item)
}
4 changes: 2 additions & 2 deletions crates/nargo_cli/tests/test_data/slices/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ fn main(x : Field, y : pub Field) {
/// TODO(#1889): Using slices in if statements where the condition is a witness
/// is not yet supported

let mut slice: [Field] = [0; 2];
let mut slice = [0; 2];
assert(slice[0] == 0);
assert(slice[0] != 1);
slice[0] = x;
Expand All @@ -15,7 +15,7 @@ fn main(x : Field, y : pub Field) {
assert(slice_plus_10[2] != 8);
assert(slice_plus_10.len() == 3);

let mut new_slice: [Field] = [];
let mut new_slice = [];
for i in 0..5 {
new_slice = new_slice.push_back(i);
}
Expand Down
31 changes: 23 additions & 8 deletions crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -800,22 +800,37 @@ impl<'block> BrilligBlock<'block> {
value_id,
dfg,
);
let heap_array = self.function_context.extract_heap_array(new_variable);

self.brillig_context
.allocate_fixed_length_array(heap_array.pointer, heap_array.size);
// Initialize the variable
let pointer = match new_variable {
RegisterOrMemory::HeapArray(heap_array) => {
self.brillig_context
.allocate_fixed_length_array(heap_array.pointer, array.len());

heap_array.pointer
}
RegisterOrMemory::HeapVector(heap_vector) => {
self.brillig_context
.const_instruction(heap_vector.size, array.len().into());
self.brillig_context
.allocate_array_instruction(heap_vector.pointer, heap_vector.size);

heap_vector.pointer
}
_ => unreachable!(
"ICE: Cannot initialize array value created as {new_variable:?}"
),
};

// Write the items

// Allocate a register for the iterator
let iterator_register = self.brillig_context.make_constant(0_usize.into());

for element_id in array.iter() {
let element_variable = self.convert_ssa_value(*element_id, dfg);
// Store the item in memory
self.store_variable_in_array(
heap_array.pointer,
iterator_register,
element_variable,
);
self.store_variable_in_array(pointer, iterator_register, element_variable);
// Increment the iterator
self.brillig_context.usize_op_in_place(iterator_register, BinaryIntOp::Add, 1);
}
Expand Down
3 changes: 2 additions & 1 deletion crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,8 @@ mod tests {
let one = builder.field_constant(FieldElement::one());

let element_type = Rc::new(vec![Type::field()]);
let array = builder.array_constant(im::Vector::unit(one), element_type);
let array_type = Type::Array(element_type, 1);
let array = builder.array_constant(im::Vector::unit(one), array_type);

builder.terminate_with_return(vec![array]);

Expand Down
31 changes: 10 additions & 21 deletions crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow::Cow, collections::HashMap, rc::Rc};
use std::{borrow::Cow, collections::HashMap};

use crate::ssa_refactor::ir::instruction::SimplifyResult;

Expand All @@ -9,7 +9,7 @@ use super::{
Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction,
},
map::DenseMap,
types::{CompositeType, Type},
types::Type,
value::{Value, ValueId},
};

Expand Down Expand Up @@ -226,12 +226,9 @@ impl DataFlowGraph {
}

/// Create a new constant array value from the given elements
pub(crate) fn make_array(
&mut self,
array: im::Vector<ValueId>,
element_type: Rc<CompositeType>,
) -> ValueId {
self.make_value(Value::Array { array, element_type })
pub(crate) fn make_array(&mut self, array: im::Vector<ValueId>, typ: Type) -> ValueId {
assert!(matches!(typ, Type::Array(..) | Type::Slice(_)));
self.make_value(Value::Array { array, typ })
}

/// Gets or creates a ValueId for the given FunctionId.
Expand Down Expand Up @@ -369,27 +366,19 @@ impl DataFlowGraph {

/// Returns the Value::Array associated with this ValueId if it refers to an array constant.
/// Otherwise, this returns None.
pub(crate) fn get_array_constant(
&self,
value: ValueId,
) -> Option<(im::Vector<ValueId>, Rc<CompositeType>)> {
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
Value::Array { array, element_type } => Some((array.clone(), element_type.clone())),
Value::Array { array, typ } => Some((array.clone(), typ.clone())),
_ => None,
}
}

/// Returns the Type::Array associated with this ValueId if it refers to an array parameter.
/// Otherwise, this returns None.
jfecher marked this conversation as resolved.
Show resolved Hide resolved
pub(crate) fn get_array_parameter_type(
&self,
value: ValueId,
) -> Option<(Rc<CompositeType>, usize)> {
match &self.values[self.resolve(value)] {
Value::Param { typ: Type::Array(element_type, size), .. } => {
Some((element_type.clone(), *size))
}
pub(crate) fn try_get_array_length(&self, value: ValueId) -> Option<usize> {
match self.type_of_value(value) {
Type::Array(_, length) => Some(length),
_ => None,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ impl<'f> FunctionInserter<'f> {
match self.values.get(&value) {
Some(value) => *value,
None => match &self.function.dfg[value] {
super::value::Value::Array { array, element_type } => {
super::value::Value::Array { array, typ } => {
let array = array.clone();
let element_type = element_type.clone();
let typ = typ.clone();
let new_array = array.iter().map(|id| self.resolve(*id)).collect();
let new_id = self.function.dfg.make_array(new_array, element_type);
let new_id = self.function.dfg.make_array(new_array, typ);
self.values.insert(value, new_id);
new_id
}
Expand Down
18 changes: 7 additions & 11 deletions crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,15 +420,9 @@ fn simplify_call(func: ValueId, arguments: &[ValueId], dfg: &mut DataFlowGraph)
Intrinsic::ArrayLen => {
let slice = dfg.get_array_constant(arguments[0]);
if let Some((slice, _)) = slice {
let slice_len =
dfg.make_constant(FieldElement::from(slice.len() as u128), Type::field());
SimplifiedTo(slice_len)
} else if let Some((_, slice_len)) = dfg.get_array_parameter_type(arguments[0]) {
let slice_len = dfg.make_constant(
FieldElement::from(slice_len as u128),
Type::Numeric(NumericType::NativeField),
);
SimplifiedTo(slice_len)
SimplifiedTo(dfg.make_constant((slice.len() as u128).into(), Type::field()))
} else if let Some(length) = dfg.try_get_array_length(arguments[0]) {
SimplifiedTo(dfg.make_constant((length as u128).into(), Type::field()))
} else {
None
}
Expand Down Expand Up @@ -534,9 +528,11 @@ fn constant_to_radix(
while limbs.len() < limb_count_with_padding as usize {
limbs.push(FieldElement::zero());
}
let result_constants =
let result_constants: im::Vector<ValueId> =
limbs.into_iter().map(|limb| dfg.make_constant(limb, Type::unsigned(bit_size))).collect();
dfg.make_array(result_constants, Rc::new(vec![Type::unsigned(bit_size)]))

let typ = Type::Array(Rc::new(vec![Type::unsigned(bit_size)]), result_constants.len());
dfg.make_array(result_constants, typ)
}

/// The possible return values for Instruction::return_types
Expand Down
10 changes: 3 additions & 7 deletions crates/noirc_evaluator/src/ssa_refactor/ir/value.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::rc::Rc;

use acvm::FieldElement;

use crate::ssa_refactor::ir::basic_block::BasicBlockId;
Expand All @@ -8,7 +6,7 @@ use super::{
function::FunctionId,
instruction::{InstructionId, Intrinsic},
map::Id,
types::{CompositeType, Type},
types::Type,
};

pub(crate) type ValueId = Id<Value>;
Expand Down Expand Up @@ -38,7 +36,7 @@ pub(crate) enum Value {
NumericConstant { constant: FieldElement, typ: Type },

/// Represents a constant array value
Array { array: im::Vector<ValueId>, element_type: Rc<CompositeType> },
Array { array: im::Vector<ValueId>, typ: Type },

/// This Value refers to a function in the IR.
/// Functions always have the type Type::Function.
Expand All @@ -64,9 +62,7 @@ impl Value {
Value::Instruction { typ, .. } => typ.clone(),
Value::Param { typ, .. } => typ.clone(),
Value::NumericConstant { typ, .. } => typ.clone(),
Value::Array { element_type, array } => {
Type::Array(element_type.clone(), array.len() / element_type.len())
}
Value::Array { typ, .. } => typ.clone(),
Value::Function { .. } => Type::Function,
Value::Intrinsic { .. } => Type::Function,
Value::ForeignFunction { .. } => Type::Function,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ impl Context {

#[cfg(test)]
mod test {
use std::rc::Rc;

use crate::ssa_refactor::{
ir::{
function::RuntimeType,
Expand Down Expand Up @@ -176,8 +178,9 @@ mod test {
let v0 = builder.add_parameter(Type::field());
let one = builder.field_constant(1u128);
let v1 = builder.insert_binary(v0, BinaryOp::Add, one);
let arr =
builder.current_function.dfg.make_array(vec![v1].into(), vec![Type::field()].into());

let array_type = Type::Array(Rc::new(vec![Type::field()]), 1);
let arr = builder.current_function.dfg.make_array(vec![v1].into(), array_type);
builder.terminate_with_return(vec![arr]);

let ssa = builder.finish().fold_constants();
Expand Down
Loading