From f9f48adaaf4a1da89c7dee1c503479cedd435936 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 4 Apr 2024 12:54:59 +0000 Subject: [PATCH 1/4] feat: Allow slices to brillig entry points --- .../src/brillig/brillig_gen/brillig_fn.rs | 22 +++------ .../src/brillig/brillig_ir/artifact.rs | 3 +- .../src/brillig/brillig_ir/entry_point.rs | 43 +++++++++++++++-- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 46 +++++++++++++++++-- .../brillig_slice_input/Nargo.toml | 6 +++ .../brillig_slice_input/src/main.nr | 26 +++++++++++ 6 files changed, 120 insertions(+), 26 deletions(-) create mode 100644 test_programs/execution_success/brillig_slice_input/Nargo.toml create mode 100644 test_programs/execution_success/brillig_slice_input/src/main.nr diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs index 92027026ce8..9bbc2f83e67 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs @@ -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)) @@ -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 { - 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 { func.returns() diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 90a5d54ae59..3d310b29ba7 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -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, usize), /// A slice parameter or return value. Holds the type of a slice item. - Slice(Vec), + /// It can hold the slice size if known at compile time. + Slice(Vec, Option), } /// The result of compiling and linking brillig artifacts. diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index 1d823ded718..4a900ac6e81 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -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, @@ -83,7 +83,23 @@ impl BrilligContext { 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(); @@ -115,7 +131,16 @@ impl BrilligContext { 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") + } + } } } @@ -138,8 +163,16 @@ impl BrilligContext { 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" + ) + } } } } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index e3dd4c33986..b826ad63ca9 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -21,7 +21,7 @@ use super::{ }, 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}; @@ -297,12 +297,14 @@ impl Context { 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 = 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. @@ -594,8 +596,9 @@ impl Context { } 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 = vecmap(result_ids, |result_id| { dfg.type_of_value(*result_id).into() @@ -673,14 +676,49 @@ impl Context { Ok(()) } + fn gen_brillig_parameters( + &self, + values: &[ValueId], + dfg: &DataFlowGraph, + ) -> Vec { + 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"), + }; + + 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, brillig: &Brillig, ) -> Result { // 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()), ); diff --git a/test_programs/execution_success/brillig_slice_input/Nargo.toml b/test_programs/execution_success/brillig_slice_input/Nargo.toml new file mode 100644 index 00000000000..a1c8cc3242b --- /dev/null +++ b/test_programs/execution_success/brillig_slice_input/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "brillig_slice_input" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/brillig_slice_input/src/main.nr b/test_programs/execution_success/brillig_slice_input/src/main.nr new file mode 100644 index 00000000000..a1391e0932f --- /dev/null +++ b/test_programs/execution_success/brillig_slice_input/src/main.nr @@ -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); +} From ca017ea873ff2720999279705e469b2de5b176dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Thu, 4 Apr 2024 15:02:00 +0200 Subject: [PATCH 2/4] Update compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index b826ad63ca9..ba96e03c9b9 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -693,7 +693,7 @@ impl Context { { AcirValue::DynamicArray(AcirDynamicArray { len, .. }) => *len, AcirValue::Array(array) => array.len(), - _ => unreachable!("ICE: Slice value is not a dynamic array"), + _ => unreachable!("ICE: Slice value is not an array"), }; BrilligParameter::Slice( From 5c6ee98bd9eaa6f556c776aad1d3e3bed39111d0 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 4 Apr 2024 13:53:37 +0000 Subject: [PATCH 3/4] refactor: make slice length non-optional --- .../src/brillig/brillig_gen/brillig_fn.rs | 9 ++---- .../src/brillig/brillig_ir/artifact.rs | 6 ++-- .../src/brillig/brillig_ir/entry_point.rs | 29 ++++--------------- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 2 +- 4 files changed, 12 insertions(+), 34 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs index 9bbc2f83e67..617e400b92f 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs @@ -81,12 +81,9 @@ impl FunctionContext { }), *size, ), - Type::Slice(item_type) => BrilligParameter::Slice( - vecmap(item_type.iter(), |item_typ| { - FunctionContext::ssa_type_to_parameter(item_typ) - }), - None, - ), + Type::Slice(_) => { + panic!("ICE: Slice parameters cannot be derived from type information") + } _ => unimplemented!("Unsupported function parameter/return type {typ:?}"), } } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 3d310b29ba7..8ce15ba4e73 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -3,7 +3,7 @@ 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. @@ -11,8 +11,8 @@ pub(crate) enum BrilligParameter { /// An array parameter or return value. Holds the type of an array item and its size. Array(Vec, usize), /// A slice parameter or return value. Holds the type of a slice item. - /// It can hold the slice size if known at compile time. - Slice(Vec, Option), + /// Only known-length slices can be passed to brillig entry points, so the size is available as well. + Slice(Vec, usize), } /// The result of compiling and linking brillig artifacts. diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index 4a900ac6e81..df694cc35fb 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -128,19 +128,10 @@ impl BrilligContext { fn flat_bit_sizes(param: &BrilligParameter) -> Box + '_> { 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(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") - } - } } } @@ -159,21 +150,11 @@ impl BrilligContext { 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(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" - ) - } - } } } @@ -490,8 +471,8 @@ mod tests { 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}, }; diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index ba96e03c9b9..d4760c29eda 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -701,7 +701,7 @@ impl Context { .iter() .map(BrilligFunctionContext::ssa_type_to_parameter) .collect(), - Some(len / item_types.len()), + len / item_types.len(), ) } else { BrilligFunctionContext::ssa_type_to_parameter(&typ) From 56311b393238863780b3bf3be443dca2d7968d44 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 4 Apr 2024 14:09:57 +0000 Subject: [PATCH 4/4] feat: add slice of arrays support --- .../src/brillig/brillig_ir/entry_point.rs | 28 +++++++++++---- .../brillig_slice_input/src/main.nr | 36 +++++++++++++------ 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index df694cc35fb..db872487fcc 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -105,18 +105,34 @@ impl BrilligContext { // Deflatten arrays for (argument_variable, argument) in argument_variables.iter_mut().zip(arguments) { - if let ( - BrilligVariable::BrilligArray(array), - BrilligParameter::Array(item_type, item_count), - ) = (argument_variable, argument) - { - if BrilligContext::has_nested_arrays(item_type) { + match (argument_variable, argument) { + ( + BrilligVariable::BrilligArray(array), + BrilligParameter::Array(item_type, item_count), + ) => { let deflattened_address = self.deflatten_array(item_type, array.size, array.pointer); self.mov_instruction(array.pointer, deflattened_address); array.size = item_type.len() * item_count; self.deallocate_register(deflattened_address); } + ( + BrilligVariable::BrilligVector(vector), + BrilligParameter::Slice(item_type, item_count), + ) => { + let flattened_size = BrilligContext::flattened_size(argument); + + let deflattened_address = + self.deflatten_array(item_type, flattened_size, vector.pointer); + self.mov_instruction(vector.pointer, deflattened_address); + self.usize_const_instruction( + vector.size, + (item_type.len() * item_count).into(), + ); + + self.deallocate_register(deflattened_address); + } + _ => {} } } } diff --git a/test_programs/execution_success/brillig_slice_input/src/main.nr b/test_programs/execution_success/brillig_slice_input/src/main.nr index a1391e0932f..09a9d9aef9d 100644 --- a/test_programs/execution_success/brillig_slice_input/src/main.nr +++ b/test_programs/execution_success/brillig_slice_input/src/main.nr @@ -3,24 +3,38 @@ struct Point { y: Field, } -unconstrained fn sum_slice(slice: [Point]) -> Field { +unconstrained fn sum_slice(slice: [[Point; 2]]) -> Field { let mut sum = 0; for i in 0..slice.len() { - sum += slice[i].x + slice[i].y; + for j in 0..slice[i].len() { + sum += slice[i][j].x + slice[i][j].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, - }); + slice = slice.push_back([ + Point { + x: 13, + y: 14, + }, + Point { + x: 20, + y: 8, + } + ]); + slice = slice.push_back([ + Point { + x: 15, + y: 5, + }, + Point { + x: 12, + y: 13, + } + ]); let brillig_sum = sum_slice(slice); - assert_eq(brillig_sum, 55); + assert_eq(brillig_sum, 100); }