Skip to content

Commit

Permalink
feat: add as_slice builtin function, add execution test (#4523)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Expected to resolve slowdown in
#4504

## Summary\*

Adds builtin function for `as_slice`, allowing it to be much more
efficient (`O(1)` instead of `O(n)`)

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: vezenovm <mvezenov@gmail.com>
  • Loading branch information
michaeljklein and vezenovm authored Mar 15, 2024
1 parent 414194c commit 6a9ea35
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 28 deletions.
63 changes: 45 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 @@ -453,6 +453,29 @@ 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);

// 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 @@ -612,13 +635,12 @@ 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)),
);
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 @@ -806,23 +828,25 @@ 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)>,
) -> 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: array_set SSA 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: array_set SSA on non-array"),
};

let (source_pointer, source_size_as_register) = match source_variable {
Expand All @@ -836,7 +860,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: array_set SSA on non-array"),
};

// Here we want to compare the reference count against 1.
Expand Down Expand Up @@ -879,18 +903,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: array_set SSA 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);
self.brillig_context.deallocate_register(condition);
source_size_as_register
}

pub(crate) fn store_variable_in_array_with_ctx(
Expand Down Expand Up @@ -1319,6 +1345,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)))?;
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) {
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);
}

0 comments on commit 6a9ea35

Please sign in to comment.