Skip to content

Commit

Permalink
feat: Brillig array inputs and outputs (#1630)
Browse files Browse the repository at this point in the history
* first attempt at brillig multiple array inptus

* working array identity function for brillig

* cleanup dbgs

* remove unused imports

* remove dbg

* a little cleanup

* fix up foreign calls for array inputs and outputs

* fix outputs clippy err

* move conversion to RegisterValueOrArray to its own method

* missing &mut and TODO link

* PR comment for brillig output array types

* cleanup comment

* enable struct inputs/outputs

* cargo clippy
  • Loading branch information
vezenovm authored Jun 13, 2023
1 parent e66fb0c commit f7f2647
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 53 deletions.
29 changes: 21 additions & 8 deletions crates/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ pub fn execute_circuit(
let mut blocks = Blocks::default();
let solver_status = solve(backend, &mut initial_witness, &mut blocks, circuit.opcodes)?;

// TODO(#1615): Nargo only supports "oracle_print_impl" functions that print a singular value and nothing else
// expand this in a general logging refactor
// TODO(#1615): Nargo only supports "oracle_print_**_impl" functions that print a singular value or an array and nothing else
// This should be expanded in a general logging refactor
if let PartialWitnessGeneratorStatus::RequiresOracleData {
unresolved_brillig_calls,
required_oracle_data,
Expand All @@ -28,13 +28,26 @@ pub fn execute_circuit(
for unresolved_brillig_call in unresolved_brillig_calls {
let UnresolvedBrilligCall { foreign_call_wait_info, mut brillig } =
unresolved_brillig_call;
let value = foreign_call_wait_info.inputs[0];

// Execute foreign call "oracle_print_impl"
println!("{:?}", value.to_field().to_hex());

// TODO(#1615): "oracle_print_impl" is just an identity func
brillig.foreign_call_results.push(ForeignCallResult { values: vec![value] });
// Execute foreign calls
// TODO(#1615): "oracle_print_impl" and "oracle_print_array_impl" are just identity funcs
if foreign_call_wait_info.function == "oracle_print_impl" {
let value = foreign_call_wait_info.inputs[0];
println!("{:?}", value.to_field().to_hex());
brillig.foreign_call_results.push(ForeignCallResult { values: vec![value] });
} else if foreign_call_wait_info.function == "oracle_print_array_impl" {
let mut outputs_hex = Vec::new();
for value in foreign_call_wait_info.inputs.clone() {
outputs_hex.push(value.to_field().to_hex());
}
// Join all of the hex strings using a comma
let comma_separated_elements = outputs_hex.join(", ");
let output_witnesses_string = "[".to_owned() + &comma_separated_elements + "]";
println!("{output_witnesses_string}");
brillig
.foreign_call_results
.push(ForeignCallResult { values: vec![foreign_call_wait_info.inputs[0]] });
}

let mut next_opcodes_for_solving = vec![Opcode::Brillig(brillig)];
next_opcodes_for_solving.extend_from_slice(&unsolved_opcodes[..]);
Expand Down
10 changes: 9 additions & 1 deletion crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use acvm::Backend;
use iter_extended::try_vecmap;
use nargo::artifacts::contract::PreprocessedContract;
use noirc_driver::{CompileOptions, CompiledProgram, Driver};
use noirc_errors::reporter;
use std::path::Path;

use clap::Args;
Expand Down Expand Up @@ -118,5 +119,12 @@ pub(crate) fn compile_circuit<B: Backend>(
compile_options: &CompileOptions,
) -> Result<CompiledProgram, CliError<B>> {
let mut driver = setup_driver(backend, program_dir)?;
driver.compile_main(compile_options).map_err(|_| CliError::CompilationError)
driver.compile_main(compile_options).map_err(|errs| {
let file_manager = driver.file_manager();
let error_count = reporter::report_all(file_manager, &errs, compile_options.deny_warnings);
if error_count != 0 {
reporter::finish_report(error_count);
}
CliError::CompilationError
})
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,35 @@
use dep::std;

struct myStruct {
foo: Field,
foo_arr: [Field; 2],
}

// Tests a very simple program.
//
// The features being tested is the identity function in Brillig
fn main(x : Field) {
assert(x == identity(x));
// TODO: add support for array comparison
let arr = identity_array([x, x]);
assert(x == arr[0]);
assert(x == arr[1]);

let s = myStruct { foo: x, foo_arr: [x, x] };
let identity_struct = identity_struct(s);
assert(x == identity_struct.foo);
assert(x == identity_struct.foo_arr[0]);
assert(x == identity_struct.foo_arr[1]);
}

unconstrained fn identity(x : Field) -> Field {
x
}

unconstrained fn identity_array(arr : [Field; 2]) -> [Field; 2] {
arr
}

unconstrained fn identity_struct(s : myStruct) -> myStruct {
s
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// Tests oracle usage in brillig/unconstrained functions
fn main(x: Field) {
// call through a brillig wrapper
oracle_print_wrapper(x);
}
oracle_print_array_wrapper([x, x]);

// TODO(#1615) Nargo currently only supports resolving one foreign call
// oracle_print_wrapper(x);
}

#[oracle(oracle_print_impl)]
unconstrained fn oracle_print(_x : Field) {}
Expand All @@ -12,4 +14,11 @@ unconstrained fn oracle_print_wrapper(x: Field) {
oracle_print(x);
}

#[oracle(oracle_print_array_impl)]
unconstrained fn oracle_print_array(_arr : [Field; 2]) {}

unconstrained fn oracle_print_array_wrapper(arr: [Field; 2]) {
oracle_print_array(arr);
}


35 changes: 29 additions & 6 deletions crates/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::ssa_refactor::ir::{
types::{NumericType, Type},
value::{Value, ValueId},
};
use acvm::acir::brillig_vm::{BinaryFieldOp, BinaryIntOp, RegisterIndex};
use acvm::acir::brillig_vm::{BinaryFieldOp, BinaryIntOp, RegisterIndex, RegisterValueOrArray};
use iter_extended::vecmap;
use std::collections::HashMap;

Expand Down Expand Up @@ -105,10 +105,15 @@ impl BrilligGen {
Value::Param { typ, .. } => typ,
_ => unreachable!("ICE: Only Param type values should appear in block parameters"),
};

match param_type {
Type::Numeric(_) => {
self.get_or_create_register(*param_id);
}
Type::Array(_, size) => {
let pointer_register = self.get_or_create_register(*param_id);
self.context.allocate_array(pointer_register, *size as u32);
}
_ => {
todo!("ICE: Param type not supported")
}
Expand Down Expand Up @@ -162,10 +167,12 @@ impl BrilligGen {
Value::ForeignFunction(func_name) => {
let result_ids = dfg.instruction_results(instruction_id);

let input_registers =
vecmap(arguments, |value_id| self.convert_ssa_value(*value_id, dfg));
let output_registers =
vecmap(result_ids, |value_id| self.convert_ssa_value(*value_id, dfg));
let input_registers = vecmap(arguments, |value_id| {
self.convert_ssa_value_to_register_value_or_array(*value_id, dfg)
});
let output_registers = vecmap(result_ids, |value_id| {
self.convert_ssa_value_to_register_value_or_array(*value_id, dfg)
});

self.context.foreign_call_instruction(
func_name.to_owned(),
Expand Down Expand Up @@ -209,7 +216,6 @@ impl BrilligGen {
/// Converts an SSA `ValueId` into a `RegisterIndex`.
fn convert_ssa_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> RegisterIndex {
let value = &dfg[value_id];

let register = match value {
Value::Param { .. } | Value::Instruction { .. } => {
// All block parameters and instruction results should have already been
Expand Down Expand Up @@ -253,6 +259,23 @@ impl BrilligGen {
self.convert_block(block, &func.dfg);
}
}

fn convert_ssa_value_to_register_value_or_array(
&mut self,
value_id: ValueId,
dfg: &DataFlowGraph,
) -> RegisterValueOrArray {
let register_index = self.convert_ssa_value(value_id, dfg);
let typ = dfg[value_id].get_type();
match typ {
Type::Numeric(_) => RegisterValueOrArray::RegisterIndex(register_index),
Type::Array(_, size) => RegisterValueOrArray::HeapArray(register_index, size),
Type::Unit => RegisterValueOrArray::RegisterIndex(register_index),
_ => {
unreachable!("type not supported for conversion into brillig register")
}
}
}
}

/// Returns the type of the operation considering the types of the operands
Expand Down
9 changes: 5 additions & 4 deletions crates/noirc_evaluator/src/brillig/brillig_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,14 @@ impl BrilligContext {
pub(crate) fn foreign_call_instruction(
&mut self,
func_name: String,
inputs: &[RegisterIndex],
outputs: &[RegisterIndex],
inputs: &[RegisterValueOrArray],
outputs: &[RegisterValueOrArray],
) {
// TODO(https://github.com/noir-lang/acvm/issues/366): Enable multiple inputs and outputs to a foreign call
let opcode = BrilligOpcode::ForeignCall {
function: func_name,
destination: RegisterValueOrArray::RegisterIndex(outputs[0]),
input: RegisterValueOrArray::RegisterIndex(inputs[0]),
destination: outputs[0],
input: inputs[0],
};
self.push_opcode(opcode);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use acvm::{
use iter_extended::vecmap;
use std::{borrow::Cow, hash::Hash};

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
/// High level Type descriptor for Variables.
///
/// One can think of Expression/Witness/Const
Expand All @@ -27,25 +27,31 @@ use std::{borrow::Cow, hash::Hash};
/// We could store this information when we do a range constraint
/// but this information is readily available by the caller so
/// we allow the user to pass it in.
pub(crate) struct AcirType(NumericType);
pub(crate) enum AcirType {
NumericType(NumericType),
Array(Vec<AcirType>, usize),
}

impl AcirType {
pub(crate) fn new(typ: NumericType) -> Self {
Self(typ)
Self::NumericType(typ)
}

/// Returns the bit size of the underlying type
pub(crate) fn bit_size(&self) -> u32 {
match self.0 {
NumericType::Signed { bit_size } => bit_size,
NumericType::Unsigned { bit_size } => bit_size,
NumericType::NativeField => FieldElement::max_num_bits(),
match self {
AcirType::NumericType(numeric_type) => match numeric_type {
NumericType::Signed { bit_size } => *bit_size,
NumericType::Unsigned { bit_size } => *bit_size,
NumericType::NativeField => FieldElement::max_num_bits(),
},
AcirType::Array(_, _) => unreachable!("cannot fetch bit size of array type"),
}
}

/// Returns a boolean type
fn boolean() -> Self {
AcirType(NumericType::Unsigned { bit_size: 1 })
AcirType::NumericType(NumericType::Unsigned { bit_size: 1 })
}
}

Expand All @@ -58,7 +64,11 @@ impl From<SsaType> for AcirType {
impl<'a> From<&'a SsaType> for AcirType {
fn from(value: &SsaType) -> Self {
match value {
SsaType::Numeric(numeric_type) => AcirType(*numeric_type),
SsaType::Numeric(numeric_type) => AcirType::NumericType(*numeric_type),
SsaType::Array(elements, size) => {
let elements = elements.iter().map(|e| e.into()).collect();
AcirType::Array(elements, *size)
}
_ => unreachable!("The type {value} cannot be represented in ACIR"),
}
}
Expand Down Expand Up @@ -170,7 +180,7 @@ impl AcirContext {
rhs: AcirVar,
typ: AcirType,
) -> Result<AcirVar, AcirGenError> {
let inputs = vec![AcirValue::Var(lhs, typ), AcirValue::Var(rhs, typ)];
let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)];
let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs)?;
Ok(outputs[0])
}
Expand All @@ -182,7 +192,7 @@ impl AcirContext {
rhs: AcirVar,
typ: AcirType,
) -> Result<AcirVar, AcirGenError> {
let inputs = vec![AcirValue::Var(lhs, typ), AcirValue::Var(rhs, typ)];
let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)];
let outputs = self.black_box_function(BlackBoxFunc::AND, inputs)?;
Ok(outputs[0])
}
Expand All @@ -209,7 +219,7 @@ impl AcirContext {
let max = self.add_constant(FieldElement::from((1_u128 << bit_size) - 1));
let a = self.sub_var(max, lhs)?;
let b = self.sub_var(max, rhs)?;
let inputs = vec![AcirValue::Var(a, typ), AcirValue::Var(b, typ)];
let inputs = vec![AcirValue::Var(a, typ.clone()), AcirValue::Var(b, typ)];
let outputs = self.black_box_function(BlackBoxFunc::AND, inputs)?;
self.sub_var(max, outputs[0])
}
Expand Down Expand Up @@ -635,7 +645,7 @@ impl AcirContext {

let mut limb_vars = vecmap(limbs, |witness| {
let witness = self.add_data(AcirVarData::Witness(witness));
AcirValue::Var(witness, result_element_type)
AcirValue::Var(witness, result_element_type.clone())
});

if endian == Endian::Big {
Expand Down Expand Up @@ -711,19 +721,63 @@ impl AcirContext {
pub(crate) fn brillig(
&mut self,
code: Vec<BrilligOpcode>,
inputs: Vec<AcirVar>,
output_len: usize,
) -> Vec<AcirVar> {
let b_inputs =
vecmap(inputs, |i| BrilligInputs::Single(self.vars[&i].to_expression().into_owned()));
let outputs = vecmap(0..output_len, |_| self.acir_ir.next_witness_index());
let outputs_var =
vecmap(&outputs, |witness_index| self.add_data(AcirVarData::Witness(*witness_index)));
let b_outputs = vecmap(outputs, BrilligOutputs::Simple);
inputs: Vec<AcirValue>,
outputs: Vec<AcirType>,
) -> Vec<AcirValue> {
let b_inputs = vecmap(inputs, |i| match i {
AcirValue::Var(var, _) => {
BrilligInputs::Single(self.vars[&var].to_expression().into_owned())
}
AcirValue::Array(vars) => {
let mut var_expressions: Vec<Expression> = Vec::new();
for var in vars {
self.brillig_array_input(&mut var_expressions, var);
}
BrilligInputs::Array(var_expressions)
}
});

let mut b_outputs = Vec::new();
let outputs_var = vecmap(outputs, |output| match output {
AcirType::NumericType(_) => {
let witness_index = self.acir_ir.next_witness_index();
b_outputs.push(BrilligOutputs::Simple(witness_index));
let var = self.add_data(AcirVarData::Witness(witness_index));
AcirValue::Var(var, output.clone())
}
AcirType::Array(element_types, size) => {
let mut witnesses = Vec::new();
let mut array_values = im::Vector::new();
for _ in 0..size {
for element_type in &element_types {
let witness_index = self.acir_ir.next_witness_index();
witnesses.push(witness_index);
let var = self.add_data(AcirVarData::Witness(witness_index));
array_values.push_back(AcirValue::Var(var, element_type.clone()));
}
}
b_outputs.push(BrilligOutputs::Array(witnesses));
AcirValue::Array(array_values)
}
});

self.acir_ir.brillig(code, b_inputs, b_outputs);

outputs_var
}

fn brillig_array_input(&self, var_expressions: &mut Vec<Expression>, input: AcirValue) {
match input {
AcirValue::Var(var, _) => {
var_expressions.push(self.vars[var].to_expression().into_owned());
}
AcirValue::Array(vars) => {
for var in vars {
self.brillig_array_input(var_expressions, var);
}
}
}
}
}

/// Enum representing the possible values that a
Expand Down
Loading

0 comments on commit f7f2647

Please sign in to comment.