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 1 commit
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
22 changes: 6 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,16 @@ impl FunctionContext {
}),
*size,
),
Type::Slice(item_type) => {
BrilligParameter::Slice(vecmap(item_type.iter(), |item_typ| {
Type::Slice(item_type) => BrilligParameter::Slice(
vecmap(item_type.iter(), |item_typ| {
FunctionContext::ssa_type_to_parameter(item_typ)
}))
}
}),
None,
),
_ => 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
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ pub(crate) enum BrilligParameter {
/// 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>),
/// It can hold the slice size if known at compile time.
Slice(Vec<BrilligParameter>, Option<usize>),
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
}

/// The result of compiling and linking brillig artifacts.
Expand Down
43 changes: 38 additions & 5 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 @@ -115,12 +131,21 @@
BrilligParameter::Array(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"),
BrilligParameter::Slice(item_types, item_count) => {
if let Some(item_count) = item_count {
Box::new(
(0..*item_count)
.flat_map(move |_| item_types.iter().flat_map(flat_bit_sizes)),
)
} else {
unreachable!("ICE: Slices with unknown length cannot be passed as entry point arguments")
}
}
}
}

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 148 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 @@ -138,8 +163,16 @@
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")
BrilligParameter::Slice(item_types, item_count) => {
if let Some(item_count) = item_count {
let item_size: usize =
item_types.iter().map(BrilligContext::flattened_size).sum();
item_count * item_size
} else {
unreachable!(
"ICE: Slices with unknown length cannot be passed as entry point arguments"
)
}
}
}
}
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 a dynamic array"),
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
};

BrilligParameter::Slice(
item_types
.iter()
.map(BrilligFunctionContext::ssa_type_to_parameter)
.collect(),
Some(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