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 9 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
13 changes: 12 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 @@ -57,6 +58,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,10 +1,14 @@
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);
}

#[oracle(oracle_print_impl)]
Expand All @@ -21,3 +25,22 @@ unconstrained fn oracle_print_array_wrapper(arr: [Field; 2]) {
oracle_print_array(arr);
}

#[oracle(get_number_sequence)]
unconstrained fn get_number_sequence(_size: Field) -> [Field] {}

#[oracle(get_reverse_number_sequence)]
unconstrained fn get_reverse_number_sequence(_size: Field) -> [Field] {}
vezenovm marked this conversation as resolved.
Show resolved Hide resolved

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