feat(brillig_gen): Return slices from foreign calls (#1909)

* working two foreign calls returning slices

* cargo fmt and clippy

* regression test for memory overwrite

* switch sequence tests to loops

* delete commented out code

* Update crates/nargo/src/ops/execute.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update crates/nargo/src/ops/execute.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update crates/nargo/src/ops/execute.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* fixup from suggested changes and cargo clippy

* link issues

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>

crates/nargo/Cargo.toml

@@ -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

crates/nargo/src/ops/execute.rs

@@ -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;
 
@@ -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());
@@ -57,6 +59,16 @@ fn execute_foreign_call(foreign_call: &ForeignCallWaitInfo) -> ForeignCallResult
 
             foreign_call.inputs[0][0].into()
         }
+        "get_number_sequence" => {
+            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"),
     }
 }

crates/nargo_cli/tests/test_data_ssa_refactor/brillig_oracle/src/main.nr

@@ -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]);
+    }
+}
+

crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -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;
@@ -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> =
@@ -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:?}")
             }

crates/noirc_evaluator/src/brillig/brillig_ir.rs

@@ -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,
@@ -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![]);