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 as_slice builtin function, add execution test #4523

Merged
merged 24 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d986bfc
add as_slice builtin function, add execution test
michaeljklein Mar 11, 2024
462af64
wip debugging test failures, add dynamic array case to test
michaeljklein Mar 11, 2024
2b749e1
wip debugging error on valid index of result of as_slice
michaeljklein Mar 11, 2024
3da8474
update can_omit_element_sizes_array
vezenovm Mar 11, 2024
197fd77
remove dbg calls and cleanup, add brillig implementation for AsSlice,…
michaeljklein Mar 11, 2024
a5f7d9e
wip debugging brillig as_slice implementation, rewrote implementation…
michaeljklein Mar 12, 2024
e8637a5
wip debugging brillig slice access error, fixed wrong initialization …
michaeljklein Mar 12, 2024
efa81db
wip debugging: cleanup for second pair of eyes
michaeljklein Mar 12, 2024
84cfbc3
got brillig tests passing using version of convert_ssa_array_set, cle…
michaeljklein Mar 12, 2024
c29add0
remove debugging return
michaeljklein Mar 12, 2024
5be64bd
Merge branch 'master' into michaeljklein/as-slice-builtin
michaeljklein Mar 12, 2024
fecda34
cargo clippy / fmt
michaeljklein Mar 12, 2024
b6feea0
fix destination length init in brillig as_slice implementation, split…
michaeljklein Mar 13, 2024
e940103
Merge branch 'master' into michaeljklein/as-slice-builtin
michaeljklein Mar 13, 2024
269bdda
combine convert_ssa_as_slice with convert_ssa_array_set to pull in re…
michaeljklein Mar 13, 2024
e5da158
Merge branch 'master' into michaeljklein/as-slice-builtin
michaeljklein Mar 13, 2024
f66772b
move as_slice logic out of convert_ssa_array_set
michaeljklein Mar 13, 2024
214229e
Merge branch 'master' into michaeljklein/as-slice-builtin
michaeljklein Mar 14, 2024
10d97e6
Merge branch 'master' into michaeljklein/as-slice-builtin
michaeljklein Mar 15, 2024
e938872
Merge branch 'master' into michaeljklein/as-slice-builtin
michaeljklein Mar 15, 2024
27ff4f0
Merge branch 'master' into michaeljklein/as-slice-builtin
michaeljklein Mar 15, 2024
0f33218
remove method string from ssa generation function (panic will unwind …
michaeljklein Mar 15, 2024
fe6156d
Merge branch 'master' into michaeljklein/as-slice-builtin
michaeljklein Mar 15, 2024
c1ec91f
Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
michaeljklein Mar 15, 2024
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
69 changes: 51 additions & 18 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,33 @@ impl<'block> BrilligBlock<'block> {
self.convert_ssa_array_len(arguments[0], result_variable.address, dfg);
}
}
Value::Intrinsic(Intrinsic::AsSlice) => {
let source_variable = self.convert_ssa_value(arguments[0], dfg);
let result_ids = dfg.instruction_results(instruction_id);
let destination_len_variable = self.variables.define_single_addr_variable(
self.function_context,
self.brillig_context,
result_ids[0],
dfg,
);
let destination_variable = self.variables.define_variable(
self.function_context,
self.brillig_context,
result_ids[1],
dfg,
);
let source_size_as_register = self.convert_ssa_array_set(
source_variable,
destination_variable,
None,
"as_slice".to_string(),
);

// we need to explicitly set the destination_len_variable
self.brillig_context
.mov_instruction(destination_len_variable.address, source_size_as_register);
self.brillig_context.deallocate_register(source_size_as_register);
}
Value::Intrinsic(
Intrinsic::SlicePushBack
| Intrinsic::SlicePopBack
Expand Down Expand Up @@ -614,13 +641,13 @@ impl<'block> BrilligBlock<'block> {
dfg,
);
self.validate_array_index(source_variable, index_register);

self.convert_ssa_array_set(
let source_size_as_register = self.convert_ssa_array_set(
source_variable,
destination_variable,
index_register.address,
value_variable,
Some((index_register.address, value_variable)),
"array set".to_string(),
);
self.brillig_context.deallocate_register(source_size_as_register);
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
let value = self.convert_ssa_single_addr_value(*value, dfg);
Expand Down Expand Up @@ -798,23 +825,26 @@ impl<'block> BrilligBlock<'block> {

/// Array set operation in SSA returns a new array or slice that is a copy of the parameter array or slice
/// With a specific value changed.
///
/// Returns `source_size_as_register`, which is expected to be deallocated with:
/// `self.brillig_context.deallocate_register(source_size_as_register)`
fn convert_ssa_array_set(
&mut self,
source_variable: BrilligVariable,
destination_variable: BrilligVariable,
index_register: MemoryAddress,
value_variable: BrilligVariable,
) {
opt_index_and_value: Option<(MemoryAddress, BrilligVariable)>,
method_str: String,
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
) -> MemoryAddress {
let destination_pointer = match destination_variable {
BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer,
BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer,
_ => unreachable!("ICE: array set returns non-array"),
_ => unreachable!("ICE: {method_str} returns non-array"),
};

let reference_count = match source_variable {
BrilligVariable::BrilligArray(BrilligArray { rc, .. })
| BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc,
_ => unreachable!("ICE: array set on non-array"),
_ => unreachable!("ICE: {method_str} on non-array"),
};

let (source_pointer, source_size_as_register) = match source_variable {
Expand All @@ -828,7 +858,7 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.mov_instruction(source_size_register, size);
(pointer, source_size_register)
}
_ => unreachable!("ICE: array set on non-array"),
_ => unreachable!("ICE: {method_str} on non-array"),
};

// Here we want to compare the reference count against 1.
Expand Down Expand Up @@ -871,18 +901,20 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.mov_instruction(target_size, source_size_as_register);
self.brillig_context.usize_const(target_rc, 1_usize.into());
}
_ => unreachable!("ICE: array set on non-array"),
_ => unreachable!("ICE: {method_str} on non-array"),
}

// Then set the value in the newly created array
self.store_variable_in_array(
destination_pointer,
SingleAddrVariable::new_usize(index_register),
value_variable,
);
if let Some((index_register, value_variable)) = opt_index_and_value {
// Then set the value in the newly created array
self.store_variable_in_array(
destination_pointer,
SingleAddrVariable::new_usize(index_register),
value_variable,
);
}

self.brillig_context.deallocate_register(source_size_as_register);
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
self.brillig_context.deallocate_register(condition);
source_size_as_register
}

pub(crate) fn store_variable_in_array_with_ctx(
Expand Down Expand Up @@ -1311,6 +1343,7 @@ impl<'block> BrilligBlock<'block> {
Value::Param { .. } | Value::Instruction { .. } => {
// All block parameters and instruction results should have already been
// converted to registers so we fetch from the cache.

self.variables.get_allocation(self.function_context, value_id, dfg)
}
Value::NumericConstant { constant, .. } => {
Expand Down
54 changes: 52 additions & 2 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,13 @@ impl Context {
self.array_set_value(&store_value, result_block_id, &mut var_index)?;

let element_type_sizes = if !can_omit_element_sizes_array(&array_typ) {
Some(self.init_element_type_sizes_array(&array_typ, array_id, None, dfg)?)
let acir_value = self.convert_value(array_id, dfg);
Some(self.init_element_type_sizes_array(
&array_typ,
array_id,
Some(&acir_value),
dfg,
)?)
} else {
None
};
Expand Down Expand Up @@ -1246,7 +1252,8 @@ impl Context {
let read = self.acir_context.read_from_memory(source, &index_var)?;
Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
})?;
self.initialize_array(destination, array_len, Some(AcirValue::Array(init_values.into())))?;
let array: im::Vector<AcirValue> = init_values.into();
self.initialize_array(destination, array_len, Some(AcirValue::Array(array.clone())))?;
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
}

Expand Down Expand Up @@ -1663,6 +1670,49 @@ impl Context {
};
Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())])
}
Intrinsic::AsSlice => {
let (slice_contents, slice_typ, block_id) =
self.check_array_is_initialized(arguments[0], dfg)?;
assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation");

let result_block_id = self.block_id(&result_ids[1]);
let acir_value = self.convert_value(slice_contents, dfg);

let array_len = if !slice_typ.contains_slice_element() {
slice_typ.flattened_size()
} else {
self.flattened_slice_size(slice_contents, dfg)
};
let slice_length = self.acir_context.add_constant(array_len);
self.copy_dynamic_array(block_id, result_block_id, array_len)?;

let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&acir_value),
dfg,
)?)
} else {
None
};

let value_types = self.convert_value(slice_contents, dfg).flat_numeric_types();
assert!(
array_len == value_types.len(),
"AsSlice: unexpected length difference: {:?} != {:?}",
array_len,
value_types.len()
);

let result = AcirValue::DynamicArray(AcirDynamicArray {
block_id: result_block_id,
len: value_types.len(),
value_types,
element_type_sizes,
});
Ok(vec![AcirValue::Var(slice_length, AcirType::field()), result])
}
Intrinsic::SlicePushBack => {
// arguments = [slice_length, slice_contents, ...elements_to_push]
let slice_length = self.convert_value(arguments[0], dfg).into_var()?;
Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub(crate) type InstructionId = Id<Instruction>;
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub(crate) enum Intrinsic {
ArrayLen,
AsSlice,
AssertConstant,
SlicePushBack,
SlicePushFront,
Expand All @@ -57,6 +58,7 @@ impl std::fmt::Display for Intrinsic {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Intrinsic::ArrayLen => write!(f, "array_len"),
Intrinsic::AsSlice => write!(f, "as_slice"),
Intrinsic::AssertConstant => write!(f, "assert_constant"),
Intrinsic::SlicePushBack => write!(f, "slice_push_back"),
Intrinsic::SlicePushFront => write!(f, "slice_push_front"),
Expand Down Expand Up @@ -89,6 +91,7 @@ impl Intrinsic {
Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) => true,

Intrinsic::ArrayLen
| Intrinsic::AsSlice
| Intrinsic::SlicePushBack
| Intrinsic::SlicePushFront
| Intrinsic::SlicePopBack
Expand All @@ -109,6 +112,7 @@ impl Intrinsic {
pub(crate) fn lookup(name: &str) -> Option<Intrinsic> {
match name {
"array_len" => Some(Intrinsic::ArrayLen),
"as_slice" => Some(Intrinsic::AsSlice),
"assert_constant" => Some(Intrinsic::AssertConstant),
"apply_range_constraint" => Some(Intrinsic::ApplyRangeConstraint),
"slice_push_back" => Some(Intrinsic::SlicePushBack),
Expand Down
10 changes: 10 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ pub(super) fn simplify_call(
SimplifyResult::None
}
}
Intrinsic::AsSlice => {
let slice = dfg.get_array_constant(arguments[0]);
if let Some((slice, element_type)) = slice {
let slice_length = dfg.make_constant(slice.len().into(), Type::length_type());
let new_slice = dfg.make_array(slice, element_type);
SimplifyResult::SimplifiedToMultiple(vec![slice_length, new_slice])
} else {
SimplifyResult::None
}
}
Intrinsic::SlicePushBack => {
let slice = dfg.get_array_constant(arguments[1]);
if let Some((mut slice, element_type)) = slice {
Expand Down
10 changes: 2 additions & 8 deletions noir_stdlib/src/array.nr
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,8 @@ impl<T, N> [T; N] {
result
}

// Converts an array into a slice.
pub fn as_slice(self) -> [T] {
let mut slice = [];
for elem in self {
slice = slice.push_back(elem);
}
slice
}
#[builtin(as_slice)]
pub fn as_slice(self) -> [T] {}

// Apply a function to each element of an array, returning a new array
// containing the mapped elements.
Expand Down
7 changes: 7 additions & 0 deletions test_programs/execution_success/array_to_slice/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "array_to_slice"
type = "bin"
authors = [""]
compiler_version = ">=0.24.0"

[dependencies]
2 changes: 2 additions & 0 deletions test_programs/execution_success/array_to_slice/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = "0"
y = "1"
33 changes: 33 additions & 0 deletions test_programs/execution_success/array_to_slice/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Converts an array into a slice.
fn as_slice_push<T, N>(xs: [T; N]) -> [T] {
let mut slice = [];
for elem in xs {
slice = slice.push_back(elem);
}
slice
}

fn main(x: Field, y: pub Field) {
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
let xs: [Field; 0] = [];
let ys: [Field; 1] = [1];
let zs: [Field; 2] = [1, 2];
let ws: [Field; 3] = [1; 3];
let qs: [Field; 4] = [3, 2, 1, 0];

let mut dynamic: [Field; 4] = [3, 2, 1, 0];
let dynamic_expected: [Field; 4] = [1000, 2, 1, 0];
dynamic[x] = 1000;

assert(x != y);
assert(xs.as_slice() == as_slice_push(xs));
assert(ys.as_slice() == as_slice_push(ys));
assert(zs.as_slice() == as_slice_push(zs));
assert(ws.as_slice() == as_slice_push(ws));
assert(qs.as_slice() == as_slice_push(qs));

assert(dynamic.as_slice()[0] == dynamic_expected[0]);
assert(dynamic.as_slice()[1] == dynamic_expected[1]);
assert(dynamic.as_slice()[2] == dynamic_expected[2]);
assert(dynamic.as_slice()[3] == dynamic_expected[3]);
assert(dynamic.as_slice().len() == 4);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "brillig_array_to_slice"
type = "bin"
authors = [""]
compiler_version = ">=0.25.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x = "0"
18 changes: 18 additions & 0 deletions test_programs/execution_success/brillig_array_to_slice/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
unconstrained fn brillig_as_slice(x: Field) -> (u64, Field, Field) {
let mut dynamic: [Field; 1] = [1];
dynamic[x] = 2;
assert(dynamic[0] == 2);

let brillig_slice = dynamic.as_slice();
assert(brillig_slice.len() == 1);

(brillig_slice.len(), dynamic[0], brillig_slice[0])
}

fn main(x: Field) {
let (slice_len, dynamic_0, slice_0) = brillig_as_slice(x);
assert(slice_len == 1);
assert(dynamic_0 == 2);
assert(slice_0 == 2);
}

Loading