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: Allow slices to brillig entry points #4713

Merged
merged 4 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
19 changes: 3 additions & 16 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FunctionContext {
function_id.to_string()
}

fn ssa_type_to_parameter(typ: &Type) -> BrilligParameter {
pub(crate) fn ssa_type_to_parameter(typ: &Type) -> BrilligParameter {
match typ {
Type::Numeric(_) | Type::Reference(_) => {
BrilligParameter::SingleAddr(get_bit_size_from_ssa_type(typ))
Expand All @@ -81,26 +81,13 @@ impl FunctionContext {
}),
*size,
),
Type::Slice(item_type) => {
BrilligParameter::Slice(vecmap(item_type.iter(), |item_typ| {
FunctionContext::ssa_type_to_parameter(item_typ)
}))
Type::Slice(_) => {
panic!("ICE: Slice parameters cannot be derived from type information")
}
_ => unimplemented!("Unsupported function parameter/return type {typ:?}"),
}
}

/// Collects the parameters of a given function
pub(crate) fn parameters(func: &Function) -> Vec<BrilligParameter> {
func.parameters()
.iter()
.map(|&value_id| {
let typ = func.dfg.type_of_value(value_id);
FunctionContext::ssa_type_to_parameter(&typ)
})
.collect()
}

/// Collects the return values of a given function
pub(crate) fn return_values(func: &Function) -> Vec<BrilligParameter> {
func.returns()
Expand Down
5 changes: 3 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ use std::collections::{BTreeMap, HashMap};

use crate::ssa::ir::dfg::CallStack;

/// Represents a parameter or a return value of a function.
/// Represents a parameter or a return value of an entry point function.
#[derive(Debug, Clone)]
pub(crate) enum BrilligParameter {
/// A single address parameter or return value. Holds the bit size of the parameter.
SingleAddr(u32),
/// An array parameter or return value. Holds the type of an array item and its size.
Array(Vec<BrilligParameter>, usize),
/// A slice parameter or return value. Holds the type of a slice item.
Slice(Vec<BrilligParameter>),
/// Only known-length slices can be passed to brillig entry points, so the size is available as well.
Slice(Vec<BrilligParameter>, usize),
}

/// The result of compiling and linking brillig artifacts.
Expand Down
32 changes: 23 additions & 9 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{
artifact::{BrilligArtifact, BrilligParameter},
brillig_variable::{BrilligArray, BrilligVariable, SingleAddrVariable},
brillig_variable::{BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable},
debug_show::DebugShow,
registers::BrilligRegistersContext,
BrilligBinaryOp, BrilligContext, ReservedRegisters,
Expand Down Expand Up @@ -83,7 +83,23 @@
current_calldata_pointer += flattened_size;
var
}
BrilligParameter::Slice(_) => unimplemented!("Unsupported slices as parameter"),
BrilligParameter::Slice(_, _) => {
let pointer_to_the_array_in_calldata =
self.make_usize_constant_instruction(current_calldata_pointer.into());

let flattened_size = BrilligContext::flattened_size(argument);
let size_register = self.make_usize_constant_instruction(flattened_size.into());
let rc_register = self.make_usize_constant_instruction(1_usize.into());

let var = BrilligVariable::BrilligVector(BrilligVector {
pointer: pointer_to_the_array_in_calldata.address,
size: size_register.address,
rc: rc_register.address,
});

current_calldata_pointer += flattened_size;
var
}
})
.collect();

Expand Down Expand Up @@ -112,15 +128,15 @@
fn flat_bit_sizes(param: &BrilligParameter) -> Box<dyn Iterator<Item = u32> + '_> {
match param {
BrilligParameter::SingleAddr(bit_size) => Box::new(std::iter::once(*bit_size)),
BrilligParameter::Array(item_types, item_count) => Box::new(
BrilligParameter::Array(item_types, item_count)
| BrilligParameter::Slice(item_types, item_count) => Box::new(
(0..*item_count).flat_map(move |_| item_types.iter().flat_map(flat_bit_sizes)),
),
BrilligParameter::Slice(..) => unimplemented!("Unsupported slices as parameter"),
}
}

for (i, bit_size) in arguments.iter().flat_map(flat_bit_sizes).enumerate() {
// Calldatacopy tags everything with field type, so when downcast when necessary

Check warning on line 139 in compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Calldatacopy)
if bit_size < FieldElement::max_num_bits() {
self.cast_instruction(
SingleAddrVariable::new(MemoryAddress(MAX_STACK_SIZE + i), bit_size),
Expand All @@ -134,13 +150,11 @@
fn flattened_size(param: &BrilligParameter) -> usize {
match param {
BrilligParameter::SingleAddr(_) => 1,
BrilligParameter::Array(item_types, item_count) => {
BrilligParameter::Array(item_types, item_count)
| BrilligParameter::Slice(item_types, item_count) => {
let item_size: usize = item_types.iter().map(BrilligContext::flattened_size).sum();
item_count * item_size
}
BrilligParameter::Slice(_) => {
unreachable!("ICE: Slices cannot be passed as entry point arguments")
}
}
}

Expand Down Expand Up @@ -457,8 +471,8 @@
use acvm::FieldElement;

use crate::brillig::brillig_ir::{
artifact::BrilligParameter,
brillig_variable::BrilligArray,
entry_point::BrilligParameter,
tests::{create_and_run_vm, create_context, create_entry_point_bytecode},
};

Expand Down
46 changes: 42 additions & 4 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
},
ssa_gen::Ssa,
};
use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
use crate::brillig::brillig_ir::artifact::{BrilligParameter, GeneratedBrillig};
use crate::brillig::brillig_ir::BrilligContext;
use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
Expand Down Expand Up @@ -183,7 +183,7 @@
abi_distinctness: Distinctness,
) -> Result<Vec<GeneratedAcir>, RuntimeError> {
let mut acirs = Vec::new();
// TODO: can we parallelise this?

Check warning on line 186 in compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (parallelise)
for function in self.functions.values() {
let context = Context::new();
if let Some(generated_acir) = context.convert_ssa_function(&self, function, brillig)? {
Expand Down Expand Up @@ -297,12 +297,14 @@
let typ = dfg.type_of_value(*param_id);
self.create_value_from_type(&typ, &mut |this, _| Ok(this.acir_context.add_variable()))
})?;
let arguments = self.gen_brillig_parameters(dfg[main_func.entry_block()].parameters(), dfg);

let witness_inputs = self.acir_context.extract_witness(&inputs);

let outputs: Vec<AcirType> =
vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into());

let code = self.gen_brillig_for(main_func, brillig)?;
let code = self.gen_brillig_for(main_func, arguments, brillig)?;

// We specifically do not attempt execution of the brillig code being generated as this can result in it being
// replaced with constraints on witnesses to the program outputs.
Expand Down Expand Up @@ -594,8 +596,9 @@
}

let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg));
let arguments = self.gen_brillig_parameters(arguments, dfg);

let code = self.gen_brillig_for(func, brillig)?;
let code = self.gen_brillig_for(func, arguments, brillig)?;

let outputs: Vec<AcirType> = vecmap(result_ids, |result_id| {
dfg.type_of_value(*result_id).into()
Expand Down Expand Up @@ -673,14 +676,49 @@
Ok(())
}

fn gen_brillig_parameters(
&self,
values: &[ValueId],
dfg: &DataFlowGraph,
) -> Vec<BrilligParameter> {
values
.iter()
.map(|&value_id| {
let typ = dfg.type_of_value(value_id);
if let Type::Slice(item_types) = typ {
let len = match self
.ssa_values
.get(&value_id)
.expect("ICE: Unknown slice input to brillig")
{
AcirValue::DynamicArray(AcirDynamicArray { len, .. }) => *len,
AcirValue::Array(array) => array.len(),
_ => unreachable!("ICE: Slice value is not an array"),
};

BrilligParameter::Slice(
item_types
.iter()
.map(BrilligFunctionContext::ssa_type_to_parameter)
.collect(),
len / item_types.len(),
)
} else {
BrilligFunctionContext::ssa_type_to_parameter(&typ)
}
})
.collect()
}

fn gen_brillig_for(
&self,
func: &Function,
arguments: Vec<BrilligParameter>,
brillig: &Brillig,
) -> Result<GeneratedBrillig, InternalError> {
// Create the entry point artifact
let mut entry_point = BrilligContext::new_entry_point_artifact(
BrilligFunctionContext::parameters(func),
arguments,
BrilligFunctionContext::return_values(func),
BrilligFunctionContext::function_id_to_function_label(func.id()),
);
Expand Down Expand Up @@ -2549,7 +2587,7 @@
let acir_functions = ssa
.into_acir(&Brillig::default(), noirc_frontend::Distinctness::Distinct)
.expect("Should compile manually written SSA into ACIR");
// The expected result should look very similar to the abvoe test expect that the input witnesses of the `Call`

Check warning on line 2590 in compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (abvoe)
// opcodes will be different. The changes can discerned from the checks below.

let main_acir = &acir_functions[0];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "brillig_slice_input"
type = "bin"
authors = [""]

[dependencies]
26 changes: 26 additions & 0 deletions test_programs/execution_success/brillig_slice_input/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
struct Point {
x: Field,
y: Field,
}

unconstrained fn sum_slice(slice: [Point]) -> Field {
let mut sum = 0;
for i in 0..slice.len() {
sum += slice[i].x + slice[i].y;
}
sum
}

fn main() {
let mut slice = &[];
slice = slice.push_back(Point {
x: 13,
y: 14,
});
slice = slice.push_front(Point {
x: 20,
y: 8,
});
let brillig_sum = sum_slice(slice);
assert_eq(brillig_sum, 55);
}
Loading