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(brillig_gen): Return slices from foreign calls #1909

Merged
merged 10 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ rustc_version = "0.4.0"
acvm.workspace = true
noirc_abi.workspace = true
noirc_driver.workspace = true
iter-extended.workspace = true
toml.workspace = true
serde.workspace = true
thiserror.workspace = true
14 changes: 13 additions & 1 deletion crates/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use acvm::acir::brillig_vm::ForeignCallResult;
use acvm::acir::brillig_vm::{ForeignCallResult, Value};
use acvm::pwg::{ACVMStatus, ForeignCallWaitInfo, ACVM};
use acvm::BlackBoxFunctionSolver;
use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap};
use iter_extended::vecmap;

use crate::NargoError;

Expand Down Expand Up @@ -38,6 +39,7 @@ fn execute_foreign_call(foreign_call: &ForeignCallWaitInfo) -> ForeignCallResult
// 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
match foreign_call.function.as_str() {
// TODO(#1910): Move to an enum and don't match directly on these strings
"oracle_print_impl" => {
let values = &foreign_call.inputs[0];
println!("{:?}", values[0].to_field().to_hex());
Expand All @@ -57,6 +59,16 @@ fn execute_foreign_call(foreign_call: &ForeignCallWaitInfo) -> ForeignCallResult

foreign_call.inputs[0][0].into()
}
"get_number_sequence" => {
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
let sequence_length: u128 = foreign_call.inputs[0][0].to_field().to_u128();

vecmap(0..sequence_length, Value::from).into()
}
"get_reverse_number_sequence" => {
let sequence_length: u128 = foreign_call.inputs[0][0].to_field().to_u128();

vecmap((0..sequence_length).rev(), Value::from).into()
}
_ => panic!("unexpected foreign call type"),
}
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,51 @@
use dep::std::slice;

// Tests oracle usage in brillig/unconstrained functions
fn main(x: Field) {
// call through a brillig wrapper
oracle_print_array_wrapper([x, x]);

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

get_number_sequence_wrapper(20);
}

// TODO(#1911)
#[oracle(oracle_print_impl)]
unconstrained fn oracle_print(_x : Field) -> Field {}

unconstrained fn oracle_print_wrapper(x: Field) {
oracle_print(x);
}

// TODO(#1911)
#[oracle(oracle_print_array_impl)]
unconstrained fn oracle_print_array(_arr : [Field; 2]) -> Field {}

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

// TODO(#1911): This function does not need to be an oracle but acts
// as a useful test while we finalize code generation for slices in Brillig
#[oracle(get_number_sequence)]
unconstrained fn get_number_sequence(_size: Field) -> [Field] {}

// TODO(#1911)
#[oracle(get_reverse_number_sequence)]
unconstrained fn get_reverse_number_sequence(_size: Field) -> [Field] {}

unconstrained fn get_number_sequence_wrapper(size: Field) {
let slice = get_number_sequence(size);
for i in 0..19 as u32 {
assert(slice[i] == i as Field);
}

let reversed_slice = get_reverse_number_sequence(size);
// Regression test that we have not overwritten memory
for i in 0..19 as u32 {
assert(slice[i] == reversed_slice[19 - i]);
}
}

24 changes: 23 additions & 1 deletion crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::ssa_refactor::ir::{
value::{Value, ValueId},
};
use acvm::acir::brillig_vm::{
BinaryFieldOp, BinaryIntOp, HeapArray, RegisterIndex, RegisterOrMemory,
BinaryFieldOp, BinaryIntOp, HeapArray, HeapVector, RegisterIndex, RegisterOrMemory,
};
use acvm::FieldElement;
use iter_extended::vecmap;
Expand Down Expand Up @@ -214,6 +214,18 @@ impl<'block> BrilligBlock<'block> {
&input_registers,
&output_registers,
);

for output_register in output_registers {
if let RegisterOrMemory::HeapVector(HeapVector { size, .. }) =
output_register
{
// Update the stack pointer so that we do not overwrite
// dynamic memory returned from other external calls
self.brillig_context.update_stack_pointer(size);
}
// Single values and allocation of fixed sized arrays has already been handled
// inside of `allocate_external_call_result`
}
}
Value::Function(func_id) => {
let function_arguments: Vec<RegisterIndex> =
Expand Down Expand Up @@ -486,6 +498,16 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.allocate_fixed_length_array(pointer, array_size_in_memory);
RegisterOrMemory::HeapArray(HeapArray { pointer, size: array_size_in_memory })
}
Type::Slice(_) => {
let pointer =
self.function_context.get_or_create_register(self.brillig_context, result);
let array_size_register = self.brillig_context.allocate_register();
// Set the pointer to the current stack frame
// The stack pointer will then be update by the caller of this method
// once the external call is resolved and the array size is known
self.brillig_context.set_array_pointer(pointer);
RegisterOrMemory::HeapVector(HeapVector { pointer, size: array_size_register })
}
_ => {
unreachable!("ICE: unsupported return type for black box call {typ:?}")
}
Expand Down
24 changes: 21 additions & 3 deletions crates/noirc_evaluator/src/brillig/brillig_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,28 @@ impl BrilligContext {
size_register: RegisterIndex,
) {
debug_show::allocate_array_instruction(pointer_register, size_register);
self.set_array_pointer(pointer_register);
self.update_stack_pointer(size_register);
}

pub(crate) fn set_array_pointer(&mut self, pointer_register: RegisterIndex) {
debug_show::mov_instruction(pointer_register, ReservedRegisters::stack_pointer());
self.push_opcode(BrilligOpcode::Mov {
destination: pointer_register,
source: ReservedRegisters::stack_pointer(),
});
}

pub(crate) fn update_stack_pointer(&mut self, size_register: RegisterIndex) {
debug_show::binary_instruction(
ReservedRegisters::stack_pointer(),
size_register,
ReservedRegisters::stack_pointer(),
BrilligBinaryOp::Integer {
op: BinaryIntOp::Add,
bit_size: BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
},
);
self.push_opcode(BrilligOpcode::BinaryIntOp {
destination: ReservedRegisters::stack_pointer(),
op: BinaryIntOp::Add,
Expand Down Expand Up @@ -777,12 +795,12 @@ mod tests {
fn test_brillig_ir_foreign_call_return_vector() {
// pseudo-noir:
//
// #[oracle(make_number_sequence)]
// unconstrained fn make_number_sequence(size: u32) -> Vec<u32> {
// #[oracle(get_number_sequence)]
// unconstrained fn get_number_sequence(size: u32) -> Vec<u32> {
// }
//
// unconstrained fn main() -> Vec<u32> {
// let the_sequence = make_number_sequence(12);
// let the_sequence = get_number_sequence(12);
// assert(the_sequence.len() == 12);
// }
let mut context = BrilligContext::new(vec![], vec![]);
Expand Down