diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs index 2c2ab17230f..04519279d52 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs @@ -449,17 +449,16 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { } HeapValueType::Array { value_types, size } => { let array_address = self.memory.read_ref(value_address); - let array_start = self.memory.read_ref(array_address); - self.read_slice_of_values_from_memory(array_start, *size, value_types) + let items_start = MemoryAddress(array_address.to_usize() + 1); + self.read_slice_of_values_from_memory(items_start, *size, value_types) } HeapValueType::Vector { value_types } => { let vector_address = self.memory.read_ref(value_address); - let vector_start = self.memory.read_ref(vector_address); - let size_address: MemoryAddress = - (vector_address.to_usize() + 1).into(); + let size_address = MemoryAddress(vector_address.to_usize() + 1); + let items_start = MemoryAddress(vector_address.to_usize() + 2); let vector_size = self.memory.read(size_address).to_usize(); self.read_slice_of_values_from_memory( - vector_start, + items_start, vector_size, value_types, ) @@ -646,8 +645,8 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { current_pointer = MemoryAddress(current_pointer.to_usize() + 1); } HeapValueType::Array { .. } => { - let destination = self.memory.read_ref(current_pointer); - let destination = self.memory.read_ref(destination); + let destination = + MemoryAddress(self.memory.read_ref(current_pointer).0 + 1); self.write_slice_of_values_to_memory( destination, values, @@ -2005,33 +2004,31 @@ mod tests { vec![MemoryValue::new_field(FieldElement::from(9u128))]; // construct memory by declaring all inner arrays/vectors first + // Declare v2 let v2_ptr: usize = 0usize; - let mut memory = v2.clone(); - let v2_start = memory.len(); - memory.extend(vec![MemoryValue::from(v2_ptr), v2.len().into(), MemoryValue::from(1_u32)]); + let mut memory = vec![MemoryValue::from(1_u32), v2.len().into()]; + memory.extend(v2.clone()); let a4_ptr = memory.len(); + memory.extend(vec![MemoryValue::from(1_u32)]); memory.extend(a4.clone()); - let a4_start = memory.len(); - memory.extend(vec![MemoryValue::from(a4_ptr), MemoryValue::from(1_u32)]); let v6_ptr = memory.len(); + memory.extend(vec![MemoryValue::from(1_u32), v6.len().into()]); memory.extend(v6.clone()); - let v6_start = memory.len(); - memory.extend(vec![MemoryValue::from(v6_ptr), v6.len().into(), MemoryValue::from(1_u32)]); let a9_ptr = memory.len(); + memory.extend(vec![MemoryValue::from(1_u32)]); memory.extend(a9.clone()); - let a9_start = memory.len(); - memory.extend(vec![MemoryValue::from(a9_ptr), MemoryValue::from(1_u32)]); // finally we add the contents of the outer array - let outer_ptr = memory.len(); + memory.extend(vec![MemoryValue::from(1_u32)]); + let outer_start = memory.len(); let outer_array = vec![ MemoryValue::new_field(FieldElement::from(1u128)), MemoryValue::from(v2.len() as u32), - MemoryValue::from(v2_start), - MemoryValue::from(a4_start), + MemoryValue::from(v2_ptr), + MemoryValue::from(a4_ptr), MemoryValue::new_field(FieldElement::from(5u128)), MemoryValue::from(v6.len() as u32), - MemoryValue::from(v6_start), - MemoryValue::from(a9_start), + MemoryValue::from(v6_ptr), + MemoryValue::from(a9_ptr), ]; memory.extend(outer_array.clone()); @@ -2075,7 +2072,7 @@ mod tests { // input = 0 Opcode::Const { destination: r_input, - value: (outer_ptr).into(), + value: (outer_start).into(), bit_size: BitSize::Integer(IntegerBitSize::U32), }, // some_function(input) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs index bd9190c1cfe..2dde5d2ca49 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs @@ -1,13 +1,16 @@ use acvm::{ - acir::{brillig::BlackBoxOp, BlackBoxFunc}, + acir::{ + brillig::{BlackBoxOp, HeapVector, ValueOrArray}, + BlackBoxFunc, + }, AcirField, }; use crate::brillig::brillig_ir::{ - brillig_variable::{BrilligVariable, BrilligVector, SingleAddrVariable}, + brillig_variable::{BrilligVariable, SingleAddrVariable}, debug_show::DebugToString, registers::RegisterAllocator, - BrilligContext, + BrilligBinaryOp, BrilligContext, }; /// Transforms SSA's black box function calls into the corresponding brillig instructions @@ -24,12 +27,17 @@ pub(crate) fn convert_black_box_call { if let ( - [BrilligVariable::SingleAddr(public_key_x), BrilligVariable::SingleAddr(public_key_y), BrilligVariable::BrilligArray(signature), message], + [BrilligVariable::SingleAddr(public_key_x), BrilligVariable::SingleAddr(public_key_y), signature, message], [BrilligVariable::SingleAddr(result_register)], ) = (function_arguments, function_results) { - let message_hash = convert_array_or_vector(brillig_context, message, bb_func); - let signature_vector = brillig_context.array_to_vector_instruction(signature); + let message = convert_array_or_vector(brillig_context, *message, bb_func); + let signature = convert_array_or_vector(brillig_context, *signature, bb_func); brillig_context.black_box_op_instruction(BlackBoxOp::SchnorrVerify { public_key_x: public_key_x.address, public_key_y: public_key_y.address, - message: message_hash.to_heap_vector(), - signature: signature_vector.to_heap_vector(), + message, + signature, result: result_register.address, }); - deallocate_converted_vector(brillig_context, message, message_hash, bb_func); - brillig_context.deallocate_register(signature_vector.size); + brillig_context.deallocate_heap_vector(message); + brillig_context.deallocate_heap_vector(signature); } else { unreachable!("ICE: Schnorr verify expects two registers for the public key, an array for signature, an array for the message hash and one result register") } @@ -205,15 +250,18 @@ pub(crate) fn convert_black_box_call { if let ( [BrilligVariable::SingleAddr(input), BrilligVariable::SingleAddr(_modulus)], - [result_array], + [output], ) = (function_arguments, function_results) { - let output = convert_array_or_vector(brillig_context, result_array, bb_func); + let output = convert_array_or_vector(brillig_context, *output, bb_func); brillig_context.black_box_op_instruction(BlackBoxOp::BigIntToLeBytes { input: input.address, - output: output.to_heap_vector(), + output, }); - deallocate_converted_vector(brillig_context, result_array, output, bb_func); + brillig_context.deallocate_heap_vector(output); } else { unreachable!( "ICE: BigIntToLeBytes expects two register arguments and one array result" @@ -364,13 +415,18 @@ pub(crate) fn convert_black_box_call { if let ( [inputs, BrilligVariable::BrilligArray(iv), BrilligVariable::BrilligArray(key)], - [BrilligVariable::SingleAddr(out_len), outputs], + [BrilligVariable::SingleAddr(out_len), BrilligVariable::BrilligVector(outputs)], ) = (function_arguments, function_results) { - let inputs_vector = convert_array_or_vector(brillig_context, inputs, bb_func); - let outputs_vector = convert_array_or_vector(brillig_context, outputs, bb_func); + let inputs = convert_array_or_vector(brillig_context, *inputs, bb_func); + let iv = brillig_context.codegen_brillig_array_to_heap_array(*iv); + let key = brillig_context.codegen_brillig_array_to_heap_array(*key); + + let outputs_vector = + brillig_context.codegen_brillig_vector_to_heap_vector(*outputs); + brillig_context.black_box_op_instruction(BlackBoxOp::AES128Encrypt { - inputs: inputs_vector.to_heap_vector(), - iv: iv.to_heap_array(), - key: key.to_heap_array(), - outputs: outputs_vector.to_heap_vector(), + inputs, + iv, + key, + outputs: outputs_vector, }); + brillig_context.mov_instruction(out_len.address, outputs_vector.size); // Returns slice, so we need to allocate memory for it after the fact + brillig_context.codegen_usize_op_in_place( + outputs_vector.size, + BrilligBinaryOp::Add, + 2_usize, + ); brillig_context.increase_free_memory_pointer_instruction(outputs_vector.size); - deallocate_converted_vector(brillig_context, inputs, inputs_vector, bb_func); - deallocate_converted_vector(brillig_context, outputs, outputs_vector, bb_func); + // We also need to write the size of the vector to the memory + brillig_context.codegen_usize_op_in_place( + outputs_vector.pointer, + BrilligBinaryOp::Sub, + 1_usize, + ); + brillig_context.store_instruction(outputs_vector.pointer, out_len.address); + + brillig_context.deallocate_heap_vector(inputs); + brillig_context.deallocate_heap_vector(outputs_vector); + brillig_context.deallocate_heap_array(iv); + brillig_context.deallocate_heap_array(key); } else { unreachable!("ICE: AES128Encrypt expects three array arguments, one array result") } @@ -420,37 +501,22 @@ pub(crate) fn convert_black_box_call( brillig_context: &mut BrilligContext, - array_or_vector: &BrilligVariable, + array_or_vector: BrilligVariable, bb_func: &BlackBoxFunc, -) -> BrilligVector { +) -> HeapVector { + let array_or_vector = brillig_context.variable_to_value_or_array(array_or_vector); match array_or_vector { - BrilligVariable::BrilligArray(array) => brillig_context.array_to_vector_instruction(array), - BrilligVariable::BrilligVector(vector) => *vector, - _ => unreachable!( - "ICE: {} expected an array or a vector, but got {:?}", - bb_func.name(), - array_or_vector - ), - } -} - -/// Deallocates any new register allocated by the function above. -/// Concretely, the only allocated register between array and vector is the size register if the array was converted to a vector. -fn deallocate_converted_vector( - brillig_context: &mut BrilligContext, - original_array_or_vector: &BrilligVariable, - converted_vector: BrilligVector, - bb_func: &BlackBoxFunc, -) { - match original_array_or_vector { - BrilligVariable::BrilligArray(_) => { - brillig_context.deallocate_register(converted_vector.size); + ValueOrArray::HeapArray(array) => { + let vector = + HeapVector { pointer: array.pointer, size: brillig_context.allocate_register() }; + brillig_context.usize_const_instruction(vector.size, array.size.into()); + vector } - BrilligVariable::BrilligVector(_) => {} + ValueOrArray::HeapVector(vector) => vector, _ => unreachable!( "ICE: {} expected an array or a vector, but got {:?}", bb_func.name(), - original_array_or_vector + array_or_vector ), } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index ba23704bba9..c4f0e944099 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1,6 +1,6 @@ use crate::brillig::brillig_ir::artifact::Label; use crate::brillig::brillig_ir::brillig_variable::{ - type_to_heap_value_type, BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable, + type_to_heap_value_type, BrilligArray, BrilligVariable, SingleAddrVariable, }; use crate::brillig::brillig_ir::registers::Stack; use crate::brillig::brillig_ir::{ @@ -19,7 +19,6 @@ use crate::ssa::ir::{ value::{Value, ValueId}, }; use acvm::acir::brillig::{MemoryAddress, ValueOrArray}; -use acvm::brillig_vm::brillig::HeapVector; use acvm::{acir::AcirField, FieldElement}; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; use iter_extended::vecmap; @@ -59,7 +58,7 @@ impl<'block> BrilligBlock<'block> { variables .get_available_variables(function_context) .into_iter() - .flat_map(|variable| variable.extract_registers()) + .map(|variable| variable.extract_register()) .collect(), ); let last_uses = function_context.liveness.get_last_uses(&block_id).clone(); @@ -152,7 +151,8 @@ impl<'block> BrilligBlock<'block> { let destination = self.variables.get_allocation(self.function_context, *dest, dfg); let source = self.convert_ssa_value(*src, dfg); - self.pass_variable(source, destination); + self.brillig_context + .mov_instruction(destination.extract_register(), source.extract_register()); } self.brillig_context.jump_instruction( self.create_block_label_for_current_function(*destination_block), @@ -161,9 +161,9 @@ impl<'block> BrilligBlock<'block> { TerminatorInstruction::Return { return_values, .. } => { let return_registers: Vec<_> = return_values .iter() - .flat_map(|value_id| { + .map(|value_id| { let return_variable = self.convert_ssa_value(*value_id, dfg); - return_variable.extract_registers() + return_variable.extract_register() }) .collect(); self.brillig_context.codegen_return(&return_registers); @@ -171,52 +171,6 @@ impl<'block> BrilligBlock<'block> { } } - /// Passes an arbitrary variable from the registers of the source to the registers of the destination - fn pass_variable(&mut self, source: BrilligVariable, destination: BrilligVariable) { - match (source, destination) { - ( - BrilligVariable::SingleAddr(source_var), - BrilligVariable::SingleAddr(destination_var), - ) => { - self.brillig_context.mov_instruction(destination_var.address, source_var.address); - } - ( - BrilligVariable::BrilligArray(BrilligArray { - pointer: source_pointer, - size: _, - rc: source_rc, - }), - BrilligVariable::BrilligArray(BrilligArray { - pointer: destination_pointer, - size: _, - rc: destination_rc, - }), - ) => { - self.brillig_context.mov_instruction(destination_pointer, source_pointer); - self.brillig_context.mov_instruction(destination_rc, source_rc); - } - ( - BrilligVariable::BrilligVector(BrilligVector { - pointer: source_pointer, - size: source_size, - rc: source_rc, - }), - BrilligVariable::BrilligVector(BrilligVector { - pointer: destination_pointer, - size: destination_size, - rc: destination_rc, - }), - ) => { - self.brillig_context.mov_instruction(destination_pointer, source_pointer); - self.brillig_context.mov_instruction(destination_size, source_size); - self.brillig_context.mov_instruction(destination_rc, source_rc); - } - (_, _) => { - unreachable!("ICE: Cannot pass value from {:?} to {:?}", source, destination); - } - } - } - /// Allocates the block parameters that the given block is defining fn convert_block_params(&mut self, dfg: &DataFlowGraph) { // We don't allocate the block parameters here, we allocate the parameters the block is defining. @@ -332,37 +286,20 @@ impl<'block> BrilligBlock<'block> { } Instruction::Allocate => { let result_value = dfg.instruction_results(instruction_id)[0]; - let address_register = self.variables.define_single_addr_variable( + let pointer = self.variables.define_single_addr_variable( self.function_context, self.brillig_context, result_value, dfg, ); - match dfg.type_of_value(result_value) { - Type::Reference(element) => match *element { - Type::Array(..) => { - self.brillig_context - .codegen_allocate_array_reference(address_register.address); - } - Type::Slice(..) => { - self.brillig_context - .codegen_allocate_vector_reference(address_register.address); - } - _ => { - self.brillig_context - .codegen_allocate_single_addr_reference(address_register.address); - } - }, - _ => { - unreachable!("ICE: Allocate on non-reference type") - } - } + self.brillig_context.codegen_allocate_immediate_mem(pointer.address, 1); } Instruction::Store { address, value } => { let address_var = self.convert_ssa_single_addr_value(*address, dfg); let source_variable = self.convert_ssa_value(*value, dfg); - self.brillig_context.codegen_store_variable(address_var.address, source_variable); + self.brillig_context + .store_instruction(address_var.address, source_variable.extract_register()); } Instruction::Load { address } => { let target_variable = self.variables.define_variable( @@ -375,7 +312,7 @@ impl<'block> BrilligBlock<'block> { let address_variable = self.convert_ssa_single_addr_value(*address, dfg); self.brillig_context - .codegen_load_variable(target_variable, address_variable.address); + .load_instruction(target_variable.extract_register(), address_variable.address); } Instruction::Not(value) => { let condition_register = self.convert_ssa_single_addr_value(*value, dfg); @@ -391,15 +328,19 @@ impl<'block> BrilligBlock<'block> { 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).to_value_or_array() + let input_values = vecmap(arguments, |value_id| { + let variable = self.convert_ssa_value(*value_id, dfg); + self.brillig_context.variable_to_value_or_array(variable) }); let input_value_types = vecmap(arguments, |value_id| { let value_type = dfg.type_of_value(*value_id); type_to_heap_value_type(&value_type) }); - let output_registers = vecmap(result_ids, |value_id| { - self.allocate_external_call_result(*value_id, dfg).to_value_or_array() + let output_variables = vecmap(result_ids, |value_id| { + self.allocate_external_call_result(*value_id, dfg) + }); + let output_values = vecmap(&output_variables, |variable| { + self.brillig_context.variable_to_value_or_array(*variable) }); let output_value_types = vecmap(result_ids, |value_id| { let value_type = dfg.type_of_value(*value_id); @@ -407,34 +348,81 @@ impl<'block> BrilligBlock<'block> { }); self.brillig_context.foreign_call_instruction( func_name.to_owned(), - &input_registers, + &input_values, &input_value_types, - &output_registers, + &output_values, &output_value_types, ); - for (i, output_register) in output_registers.iter().enumerate() { - if let ValueOrArray::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.increase_free_memory_pointer_instruction(*size); - - // Update the dynamic slice length maintained in SSA - if let ValueOrArray::MemoryAddress(len_index) = output_registers[i - 1] - { - let element_size = dfg[result_ids[i]].get_type().element_size(); - self.brillig_context.mov_instruction(len_index, *size); - self.brillig_context.codegen_usize_op_in_place( - len_index, - BrilligBinaryOp::UnsignedDiv, - element_size, + // Deallocate the temporary heap arrays and vectors of the inputs + for input_value in input_values { + match input_value { + ValueOrArray::HeapArray(array) => { + self.brillig_context.deallocate_heap_array(array); + } + ValueOrArray::HeapVector(vector) => { + self.brillig_context.deallocate_heap_vector(vector); + } + _ => {} + } + } + + // Deallocate the temporary heap arrays and vectors of the outputs + for (i, (output_register, output_variable)) in + output_values.iter().zip(output_variables).enumerate() + { + match output_register { + // Returned vectors need to emit some bytecode to format the result as a BrilligVector + ValueOrArray::HeapVector(heap_vector) => { + // Update the stack pointer so that we do not overwrite + // dynamic memory returned from other external calls + // Single values and allocation of fixed sized arrays has already been handled + // inside of `allocate_external_call_result` + let total_size = self.brillig_context.allocate_register(); + self.brillig_context.codegen_usize_op( + heap_vector.size, + total_size, + BrilligBinaryOp::Add, + 2, // RC and Length ); - } else { - unreachable!("ICE: a vector must be preceded by a register containing its length"); + + self.brillig_context + .increase_free_memory_pointer_instruction(total_size); + let brillig_vector = output_variable.extract_vector(); + let size_pointer = self.brillig_context.allocate_register(); + + self.brillig_context.codegen_usize_op( + brillig_vector.pointer, + size_pointer, + BrilligBinaryOp::Add, + 1_usize, // Slices are [RC, Size, ...items] + ); + self.brillig_context + .store_instruction(size_pointer, heap_vector.size); + self.brillig_context.deallocate_register(size_pointer); + self.brillig_context.deallocate_register(total_size); + + // Update the dynamic slice length maintained in SSA + if let ValueOrArray::MemoryAddress(len_index) = output_values[i - 1] + { + let element_size = dfg[result_ids[i]].get_type().element_size(); + self.brillig_context + .mov_instruction(len_index, heap_vector.size); + self.brillig_context.codegen_usize_op_in_place( + len_index, + BrilligBinaryOp::UnsignedDiv, + element_size, + ); + } else { + unreachable!("ICE: a vector must be preceded by a register containing its length"); + } + self.brillig_context.deallocate_heap_vector(*heap_vector); + } + ValueOrArray::HeapArray(array) => { + self.brillig_context.deallocate_heap_array(*array); } + ValueOrArray::MemoryAddress(_) => {} } - // Single values and allocation of fixed sized arrays has already been handled - // inside of `allocate_external_call_result` } } Value::Function(func_id) => { @@ -511,21 +499,40 @@ impl<'block> BrilligBlock<'block> { result_ids[1], dfg, ); + let destination_vector = destination_variable.extract_vector(); + let source_array = source_variable.extract_array(); + let element_size = dfg.type_of_value(arguments[0]).element_size(); - self.brillig_context - .call_array_copy_procedure(source_variable, destination_variable); - - let BrilligVariable::BrilligArray(BrilligArray { size: source_size, .. }) = - source_variable - else { - unreachable!("ICE: AsSlice on non-array") - }; + let source_size_register = self + .brillig_context + .make_usize_constant_instruction(source_array.size.into()); // we need to explicitly set the destination_len_variable - self.brillig_context.usize_const_instruction( + self.brillig_context.codegen_usize_op( + source_size_register.address, destination_len_variable.address, - source_size.into(), + BrilligBinaryOp::UnsignedDiv, + element_size, ); + + self.brillig_context + .codegen_initialize_vector(destination_vector, source_size_register); + + // Items + let vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(destination_vector); + let array_items_pointer = + self.brillig_context.codegen_make_array_items_pointer(source_array); + + self.brillig_context.codegen_mem_copy( + array_items_pointer, + vector_items_pointer, + source_size_register, + ); + + self.brillig_context.deallocate_single_addr(source_size_register); + self.brillig_context.deallocate_register(vector_items_pointer); + self.brillig_context.deallocate_register(array_items_pointer); } Value::Intrinsic( Intrinsic::SlicePushBack @@ -641,11 +648,6 @@ impl<'block> BrilligBlock<'block> { ); let array_variable = self.convert_ssa_value(*array, dfg); - let array_pointer = match array_variable { - BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, - BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, - _ => unreachable!("ICE: array get on non-array"), - }; let index_variable = self.convert_ssa_single_addr_value(*index, dfg); @@ -653,11 +655,16 @@ impl<'block> BrilligBlock<'block> { self.validate_array_index(array_variable, index_variable); } - self.retrieve_variable_from_array( - array_pointer, + let items_pointer = + self.brillig_context.codegen_make_array_or_vector_items_pointer(array_variable); + + self.brillig_context.codegen_load_with_offset( + items_pointer, index_variable, - destination_variable, + destination_variable.extract_register(), ); + + self.brillig_context.deallocate_register(items_pointer); } Instruction::ArraySet { array, index, value, mutable: _ } => { let source_variable = self.convert_ssa_value(*array, dfg); @@ -718,28 +725,35 @@ impl<'block> BrilligBlock<'block> { } } Instruction::IncrementRc { value } => { - let rc_register = match self.convert_ssa_value(*value, dfg) { - BrilligVariable::BrilligArray(BrilligArray { rc, .. }) - | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, - other => unreachable!("ICE: increment rc on non-array: {other:?}"), - }; + let array_or_vector = self.convert_ssa_value(*value, dfg); + let rc_register = self.brillig_context.allocate_register(); + + // RC is always directly pointed by the array/vector pointer + self.brillig_context + .load_instruction(rc_register, array_or_vector.extract_register()); self.brillig_context.codegen_usize_op_in_place( rc_register, BrilligBinaryOp::Add, 1, ); + self.brillig_context + .store_instruction(array_or_vector.extract_register(), rc_register); + self.brillig_context.deallocate_register(rc_register); } Instruction::DecrementRc { value } => { - let rc_register = match self.convert_ssa_value(*value, dfg) { - BrilligVariable::BrilligArray(BrilligArray { rc, .. }) - | BrilligVariable::BrilligVector(BrilligVector { rc, .. }) => rc, - other => unreachable!("ICE: decrement rc on non-array: {other:?}"), - }; + let array_or_vector = self.convert_ssa_value(*value, dfg); + let rc_register = self.brillig_context.allocate_register(); + + self.brillig_context + .load_instruction(rc_register, array_or_vector.extract_register()); self.brillig_context.codegen_usize_op_in_place( rc_register, BrilligBinaryOp::Sub, 1, ); + self.brillig_context + .store_instruction(array_or_vector.extract_register(), rc_register); + self.brillig_context.deallocate_register(rc_register); } Instruction::EnableSideEffectsIf { .. } => { todo!("enable_side_effects not supported by brillig") @@ -774,7 +788,7 @@ impl<'block> BrilligBlock<'block> { // Convert the arguments to registers casting those to the types of the receiving function let argument_registers: Vec = arguments .iter() - .flat_map(|argument_id| self.convert_ssa_value(*argument_id, dfg).extract_registers()) + .map(|argument_id| self.convert_ssa_value(*argument_id, dfg).extract_register()) .collect(); let variables_to_save = self.variables.get_available_variables(self.function_context); @@ -802,7 +816,7 @@ impl<'block> BrilligBlock<'block> { // Collect the registers that should have been returned let returned_registers: Vec = variables_assigned_to .iter() - .flat_map(|returned_variable| returned_variable.extract_registers()) + .map(|returned_variable| returned_variable.extract_register()) .collect(); assert!( @@ -816,8 +830,7 @@ impl<'block> BrilligBlock<'block> { // Reset the register state to the one needed to hold the current available variables let variables = self.variables.get_available_variables(self.function_context); - let registers = - variables.into_iter().flat_map(|variable| variable.extract_registers()).collect(); + let registers = variables.into_iter().map(|variable| variable.extract_register()).collect(); self.brillig_context.set_allocated_registers(registers); } @@ -826,21 +839,13 @@ impl<'block> BrilligBlock<'block> { array_variable: BrilligVariable, index_register: SingleAddrVariable, ) { - let (size_as_register, should_deallocate_size) = match array_variable { - BrilligVariable::BrilligArray(BrilligArray { size, .. }) => { - (self.brillig_context.make_usize_constant_instruction(size.into()), true) - } - BrilligVariable::BrilligVector(BrilligVector { size, .. }) => { - (SingleAddrVariable::new_usize(size), false) - } - _ => unreachable!("ICE: validate array index on non-array"), - }; + let size = self.brillig_context.codegen_make_array_or_vector_length(array_variable); let condition = SingleAddrVariable::new(self.brillig_context.allocate_register(), 1); self.brillig_context.memory_op_instruction( index_register.address, - size_as_register.address, + size.address, condition.address, BrilligBinaryOp::LessThan, ); @@ -848,35 +853,10 @@ impl<'block> BrilligBlock<'block> { self.brillig_context .codegen_constrain(condition, Some("Array index out of bounds".to_owned())); - if should_deallocate_size { - self.brillig_context.deallocate_single_addr(size_as_register); - } + self.brillig_context.deallocate_single_addr(size); self.brillig_context.deallocate_single_addr(condition); } - pub(crate) fn retrieve_variable_from_array( - &mut self, - array_pointer: MemoryAddress, - index_var: SingleAddrVariable, - destination_variable: BrilligVariable, - ) { - match destination_variable { - BrilligVariable::SingleAddr(destination_register) => { - self.brillig_context.codegen_array_get( - array_pointer, - index_var, - destination_register.address, - ); - } - BrilligVariable::BrilligArray(..) | BrilligVariable::BrilligVector(..) => { - let reference = self.brillig_context.allocate_register(); - self.brillig_context.codegen_array_get(array_pointer, index_var, reference); - self.brillig_context.codegen_load_variable(destination_variable, reference); - self.brillig_context.deallocate_register(reference); - } - } - } - /// Array set operation in SSA returns a new array or slice that is a copy of the parameter array or slice /// With a specific value changed. /// @@ -890,19 +870,33 @@ impl<'block> BrilligBlock<'block> { value_variable: BrilligVariable, ) { assert!(index_register.bit_size == BRILLIG_MEMORY_ADDRESSING_BIT_SIZE); - let destination_pointer = match destination_variable { - BrilligVariable::BrilligArray(BrilligArray { pointer, .. }) => pointer, - BrilligVariable::BrilligVector(BrilligVector { pointer, .. }) => pointer, - _ => unreachable!("ICE: array_set SSA returns non-array"), - }; + match (source_variable, destination_variable) { + ( + BrilligVariable::BrilligArray(source_array), + BrilligVariable::BrilligArray(destination_array), + ) => { + self.brillig_context.call_array_copy_procedure(source_array, destination_array); + } + ( + BrilligVariable::BrilligVector(source_vector), + BrilligVariable::BrilligVector(destination_vector), + ) => { + self.brillig_context.call_vector_copy_procedure(source_vector, destination_vector); + } + _ => unreachable!("ICE: array set on non-array"), + } - self.brillig_context.call_array_copy_procedure(source_variable, destination_variable); // Then set the value in the newly created array - self.brillig_context.codegen_store_variable_in_array( - destination_pointer, + let items_pointer = + self.brillig_context.codegen_make_array_or_vector_items_pointer(destination_variable); + + self.brillig_context.codegen_store_with_offset( + items_pointer, index_register, - value_variable, + value_variable.extract_register(), ); + + self.brillig_context.deallocate_register(items_pointer); } /// Convert the SSA slice operations to brillig slice operations @@ -915,8 +909,7 @@ impl<'block> BrilligBlock<'block> { ) { let slice_id = arguments[1]; let element_size = dfg.type_of_value(slice_id).element_size(); - let source_variable = self.convert_ssa_value(slice_id, dfg); - let source_vector = self.convert_array_or_vector_to_vector(source_variable); + let source_vector = self.convert_ssa_value(slice_id, dfg).extract_vector(); let results = dfg.instruction_results(instruction_id); match intrinsic { @@ -1572,25 +1565,16 @@ impl<'block> BrilligBlock<'block> { ); // Initialize the variable - let pointer = match new_variable { + match new_variable { BrilligVariable::BrilligArray(brillig_array) => { - self.brillig_context.codegen_allocate_fixed_length_array( - brillig_array.pointer, - array.len(), - ); - self.brillig_context - .usize_const_instruction(brillig_array.rc, 1_usize.into()); - - brillig_array.pointer + self.brillig_context.codegen_initialize_array(brillig_array); } BrilligVariable::BrilligVector(vector) => { - self.brillig_context - .usize_const_instruction(vector.size, array.len().into()); - self.brillig_context - .codegen_allocate_array(vector.pointer, vector.size); - self.brillig_context.usize_const_instruction(vector.rc, 1_usize.into()); - - vector.pointer + let size = self + .brillig_context + .make_usize_constant_instruction(array.len().into()); + self.brillig_context.codegen_initialize_vector(vector, size); + self.brillig_context.deallocate_single_addr(size); } _ => unreachable!( "ICE: Cannot initialize array value created as {new_variable:?}" @@ -1598,8 +1582,13 @@ impl<'block> BrilligBlock<'block> { }; // Write the items + let items_pointer = self + .brillig_context + .codegen_make_array_or_vector_items_pointer(new_variable); + + self.initialize_constant_array(array, typ, dfg, items_pointer); - self.initialize_constant_array(array, typ, dfg, pointer); + self.brillig_context.deallocate_register(items_pointer); new_variable } @@ -1709,7 +1698,7 @@ impl<'block> BrilligBlock<'block> { for (subitem_index, subitem) in subitem_to_repeat_variables.into_iter().enumerate() { - ctx.codegen_store_variable_in_pointer(subitem_pointer.address, subitem); + ctx.store_instruction(subitem_pointer.address, subitem.extract_register()); if subitem_index != item_types.len() - 1 { ctx.memory_op_instruction( subitem_pointer.address, @@ -1736,7 +1725,7 @@ impl<'block> BrilligBlock<'block> { let initializer_fn = |ctx: &mut BrilligContext<_, _>, item_pointer: SingleAddrVariable| { - ctx.codegen_store_variable_in_pointer(item_pointer.address, subitem); + ctx.store_instruction(item_pointer.address, subitem.extract_register()); }; // for (let item_pointer = pointer; item_pointer < pointer + data_length; item_pointer += 1) { initializer_fn(iterator) } @@ -1765,7 +1754,7 @@ impl<'block> BrilligBlock<'block> { let element_variable = self.convert_ssa_value(*element_id, dfg); // Store the item in memory self.brillig_context - .codegen_store_variable_in_pointer(write_pointer_register, element_variable); + .store_instruction(write_pointer_register, element_variable.extract_register()); if element_idx != data.len() - 1 { // Increment the write_pointer_register @@ -1830,7 +1819,11 @@ impl<'block> BrilligBlock<'block> { // The stack pointer will then be updated by the caller of this method // once the external call is resolved and the array size is known self.brillig_context.load_free_memory_pointer_instruction(vector.pointer); - self.brillig_context.usize_const_instruction(vector.rc, 1_usize.into()); + self.brillig_context.indirect_const_instruction( + vector.pointer, + BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, + 1_usize.into(), + ); variable } @@ -1845,8 +1838,7 @@ impl<'block> BrilligBlock<'block> { unreachable!("ICE: allocate_foreign_call_array() expects an array, got {typ:?}") }; - self.brillig_context.codegen_allocate_fixed_length_array(array.pointer, array.size); - self.brillig_context.usize_const_instruction(array.rc, 1_usize.into()); + self.brillig_context.codegen_initialize_array(array); let mut index = 0_usize; for _ in 0..*size { @@ -1855,18 +1847,17 @@ impl<'block> BrilligBlock<'block> { Type::Array(_, nested_size) => { let inner_array = BrilligArray { pointer: self.brillig_context.allocate_register(), - rc: self.brillig_context.allocate_register(), size: *nested_size, }; self.allocate_foreign_call_result_array(element_type, inner_array); + // We add one since array.pointer points to [RC, ...items] let idx = - self.brillig_context.make_usize_constant_instruction(index.into()); - self.brillig_context.codegen_store_variable_in_array(array.pointer, idx, BrilligVariable::BrilligArray(inner_array)); + self.brillig_context.make_usize_constant_instruction((index + 1).into() ); + self.brillig_context.codegen_store_with_offset(array.pointer, idx, inner_array.pointer); self.brillig_context.deallocate_single_addr(idx); self.brillig_context.deallocate_register(inner_array.pointer); - self.brillig_context.deallocate_register(inner_array.rc); } Type::Slice(_) => unreachable!("ICE: unsupported slice type in allocate_nested_array(), expects an array or a numeric type"), _ => (), @@ -1893,13 +1884,17 @@ impl<'block> BrilligBlock<'block> { self.brillig_context .usize_const_instruction(result_register, (size / element_size).into()); } - BrilligVariable::BrilligVector(BrilligVector { size, .. }) => { + BrilligVariable::BrilligVector(vector) => { + let size = self.brillig_context.codegen_make_vector_length(vector); + self.brillig_context.codegen_usize_op( - size, + size.address, result_register, BrilligBinaryOp::UnsignedDiv, element_size, ); + + self.brillig_context.deallocate_single_addr(size); } _ => { unreachable!("ICE: Cannot get length of {array_variable:?}") diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs index 90af2e42211..393d4c967c2 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs @@ -91,9 +91,7 @@ impl BlockVariables { .ssa_value_allocations .get(value_id) .expect("ICE: Variable allocation not found"); - variable.extract_registers().iter().for_each(|register| { - brillig_context.deallocate_register(*register); - }); + brillig_context.deallocate_register(variable.extract_register()); } /// Checks if a variable is allocated. @@ -142,27 +140,12 @@ pub(crate) fn allocate_value( bit_size: get_bit_size_from_ssa_type(&typ), }) } - Type::Array(item_typ, elem_count) => { - let pointer_register = brillig_context.allocate_register(); - let rc_register = brillig_context.allocate_register(); - let size = compute_array_length(&item_typ, elem_count); - - BrilligVariable::BrilligArray(BrilligArray { - pointer: pointer_register, - size, - rc: rc_register, - }) - } - Type::Slice(_) => { - let pointer_register = brillig_context.allocate_register(); - let size_register = brillig_context.allocate_register(); - let rc_register = brillig_context.allocate_register(); - - BrilligVariable::BrilligVector(BrilligVector { - pointer: pointer_register, - size: size_register, - rc: rc_register, - }) - } + Type::Array(item_typ, elem_count) => BrilligVariable::BrilligArray(BrilligArray { + pointer: brillig_context.allocate_register(), + size: compute_array_length(&item_typ, elem_count), + }), + Type::Slice(_) => BrilligVariable::BrilligVector(BrilligVector { + pointer: brillig_context.allocate_register(), + }), } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs index 9dbf0ee9bb6..36a3b34aeb0 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs @@ -13,38 +13,50 @@ impl<'block> BrilligBlock<'block> { variables_to_insert: &[BrilligVariable], ) { // First we need to allocate the target vector incrementing the size by variables_to_insert.len() + let source_size = self.brillig_context.codegen_make_vector_length(source_vector); + + let target_size = SingleAddrVariable::new_usize(self.brillig_context.allocate_register()); self.brillig_context.codegen_usize_op( - source_vector.size, - target_vector.size, + source_size.address, + target_size.address, BrilligBinaryOp::Add, variables_to_insert.len(), ); - self.brillig_context.codegen_allocate_array(target_vector.pointer, target_vector.size); - // We initialize the RC of the target vector to 1 - self.brillig_context.usize_const_instruction(target_vector.rc, 1_usize.into()); + + self.brillig_context.codegen_initialize_vector(target_vector, target_size); // Now we copy the source vector into the target vector + let source_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(source_vector); + let target_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(target_vector); + self.brillig_context.codegen_mem_copy( - source_vector.pointer, - target_vector.pointer, - SingleAddrVariable::new_usize(source_vector.size), + source_vector_items_pointer, + target_vector_items_pointer, + source_size, ); for (index, variable) in variables_to_insert.iter().enumerate() { let target_index = self.brillig_context.make_usize_constant_instruction(index.into()); self.brillig_context.memory_op_instruction( target_index.address, - source_vector.size, + source_size.address, target_index.address, BrilligBinaryOp::Add, ); - self.brillig_context.codegen_store_variable_in_array( - target_vector.pointer, + self.brillig_context.codegen_store_with_offset( + target_vector_items_pointer, target_index, - *variable, + variable.extract_register(), ); self.brillig_context.deallocate_single_addr(target_index); } + + self.brillig_context.deallocate_single_addr(source_size); + self.brillig_context.deallocate_single_addr(target_size); + self.brillig_context.deallocate_register(source_vector_items_pointer); + self.brillig_context.deallocate_register(target_vector_items_pointer); } pub(crate) fn slice_push_front_operation( @@ -54,20 +66,27 @@ impl<'block> BrilligBlock<'block> { variables_to_insert: &[BrilligVariable], ) { // First we need to allocate the target vector incrementing the size by variables_to_insert.len() + let source_size = self.brillig_context.codegen_make_vector_length(source_vector); + + let target_size = SingleAddrVariable::new_usize(self.brillig_context.allocate_register()); self.brillig_context.codegen_usize_op( - source_vector.size, - target_vector.size, + source_size.address, + target_size.address, BrilligBinaryOp::Add, variables_to_insert.len(), ); - self.brillig_context.codegen_allocate_array(target_vector.pointer, target_vector.size); - // We initialize the RC of the target vector to 1 - self.brillig_context.usize_const_instruction(target_vector.rc, 1_usize.into()); + + self.brillig_context.codegen_initialize_vector(target_vector, target_size); // Now we offset the target pointer by variables_to_insert.len() + let source_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(source_vector); + let target_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(target_vector); + let destination_copy_pointer = self.brillig_context.allocate_register(); self.brillig_context.codegen_usize_op( - target_vector.pointer, + target_vector_items_pointer, destination_copy_pointer, BrilligBinaryOp::Add, variables_to_insert.len(), @@ -75,23 +94,27 @@ impl<'block> BrilligBlock<'block> { // Now we copy the source vector into the target vector starting at index variables_to_insert.len() self.brillig_context.codegen_mem_copy( - source_vector.pointer, + source_vector_items_pointer, destination_copy_pointer, - SingleAddrVariable::new_usize(source_vector.size), + source_size, ); // Then we write the items to insert at the start for (index, variable) in variables_to_insert.iter().enumerate() { let target_index = self.brillig_context.make_usize_constant_instruction(index.into()); - self.brillig_context.codegen_store_variable_in_array( - target_vector.pointer, + self.brillig_context.codegen_store_with_offset( + target_vector_items_pointer, target_index, - *variable, + variable.extract_register(), ); self.brillig_context.deallocate_single_addr(target_index); } self.brillig_context.deallocate_register(destination_copy_pointer); + self.brillig_context.deallocate_single_addr(source_size); + self.brillig_context.deallocate_single_addr(target_size); + self.brillig_context.deallocate_register(source_vector_items_pointer); + self.brillig_context.deallocate_register(target_vector_items_pointer); } pub(crate) fn slice_pop_front_operation( @@ -101,20 +124,27 @@ impl<'block> BrilligBlock<'block> { removed_items: &[BrilligVariable], ) { // First we need to allocate the target vector decrementing the size by removed_items.len() + let source_size = self.brillig_context.codegen_make_vector_length(source_vector); + + let target_size = SingleAddrVariable::new_usize(self.brillig_context.allocate_register()); self.brillig_context.codegen_usize_op( - source_vector.size, - target_vector.size, + source_size.address, + target_size.address, BrilligBinaryOp::Sub, removed_items.len(), ); - self.brillig_context.codegen_allocate_array(target_vector.pointer, target_vector.size); - // We initialize the RC of the target vector to 1 - self.brillig_context.usize_const_instruction(target_vector.rc, 1_usize.into()); + + self.brillig_context.codegen_initialize_vector(target_vector, target_size); // Now we offset the source pointer by removed_items.len() + let source_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(source_vector); + let target_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(target_vector); + let source_copy_pointer = self.brillig_context.allocate_register(); self.brillig_context.codegen_usize_op( - source_vector.pointer, + source_vector_items_pointer, source_copy_pointer, BrilligBinaryOp::Add, removed_items.len(), @@ -123,17 +153,25 @@ impl<'block> BrilligBlock<'block> { // Now we copy the source vector starting at index removed_items.len() into the target vector self.brillig_context.codegen_mem_copy( source_copy_pointer, - target_vector.pointer, - SingleAddrVariable::new_usize(target_vector.size), + target_vector_items_pointer, + target_size, ); for (index, variable) in removed_items.iter().enumerate() { let target_index = self.brillig_context.make_usize_constant_instruction(index.into()); - self.retrieve_variable_from_array(source_vector.pointer, target_index, *variable); + self.brillig_context.codegen_load_with_offset( + source_vector_items_pointer, + target_index, + variable.extract_register(), + ); self.brillig_context.deallocate_single_addr(target_index); } self.brillig_context.deallocate_register(source_copy_pointer); + self.brillig_context.deallocate_single_addr(source_size); + self.brillig_context.deallocate_single_addr(target_size); + self.brillig_context.deallocate_register(source_vector_items_pointer); + self.brillig_context.deallocate_register(target_vector_items_pointer); } pub(crate) fn slice_pop_back_operation( @@ -143,34 +181,49 @@ impl<'block> BrilligBlock<'block> { removed_items: &[BrilligVariable], ) { // First we need to allocate the target vector decrementing the size by removed_items.len() + let source_size = self.brillig_context.codegen_make_vector_length(source_vector); + + let target_size = SingleAddrVariable::new_usize(self.brillig_context.allocate_register()); self.brillig_context.codegen_usize_op( - source_vector.size, - target_vector.size, + source_size.address, + target_size.address, BrilligBinaryOp::Sub, removed_items.len(), ); - self.brillig_context.codegen_allocate_array(target_vector.pointer, target_vector.size); - // We initialize the RC of the target vector to 1 - self.brillig_context.usize_const_instruction(target_vector.rc, 1_usize.into()); + + self.brillig_context.codegen_initialize_vector(target_vector, target_size); // Now we copy all elements except the last items into the target vector + let source_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(source_vector); + let target_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(target_vector); + self.brillig_context.codegen_mem_copy( - source_vector.pointer, - target_vector.pointer, - SingleAddrVariable::new_usize(target_vector.size), + source_vector_items_pointer, + target_vector_items_pointer, + target_size, ); for (index, variable) in removed_items.iter().enumerate() { let target_index = self.brillig_context.make_usize_constant_instruction(index.into()); self.brillig_context.memory_op_instruction( target_index.address, - target_vector.size, + target_size.address, target_index.address, BrilligBinaryOp::Add, ); - self.retrieve_variable_from_array(source_vector.pointer, target_index, *variable); + self.brillig_context.codegen_load_with_offset( + source_vector_items_pointer, + target_index, + variable.extract_register(), + ); self.brillig_context.deallocate_single_addr(target_index); } + self.brillig_context.deallocate_single_addr(source_size); + self.brillig_context.deallocate_single_addr(target_size); + self.brillig_context.deallocate_register(source_vector_items_pointer); + self.brillig_context.deallocate_register(target_vector_items_pointer); } pub(crate) fn slice_insert_operation( @@ -181,23 +234,34 @@ impl<'block> BrilligBlock<'block> { items: &[BrilligVariable], ) { // First we need to allocate the target vector incrementing the size by items.len() + let source_size = self.brillig_context.codegen_make_vector_length(source_vector); + + let target_size = SingleAddrVariable::new_usize(self.brillig_context.allocate_register()); self.brillig_context.codegen_usize_op( - source_vector.size, - target_vector.size, + source_size.address, + target_size.address, BrilligBinaryOp::Add, items.len(), ); - self.brillig_context.codegen_allocate_array(target_vector.pointer, target_vector.size); - // We initialize the RC of the target vector to 1 - self.brillig_context.usize_const_instruction(target_vector.rc, 1_usize.into()); + + self.brillig_context.codegen_initialize_vector(target_vector, target_size); // Copy the elements to the left of the index - self.brillig_context.codegen_mem_copy(source_vector.pointer, target_vector.pointer, index); + let source_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(source_vector); + let target_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(target_vector); + + self.brillig_context.codegen_mem_copy( + source_vector_items_pointer, + target_vector_items_pointer, + index, + ); // Compute the source pointer just at the index let source_pointer_at_index = self.brillig_context.allocate_register(); self.brillig_context.memory_op_instruction( - source_vector.pointer, + source_vector_items_pointer, index.address, source_pointer_at_index, BrilligBinaryOp::Add, @@ -206,7 +270,7 @@ impl<'block> BrilligBlock<'block> { // Compute the target pointer after the inserted elements let target_pointer_after_index = self.brillig_context.allocate_register(); self.brillig_context.memory_op_instruction( - target_vector.pointer, + target_vector_items_pointer, index.address, target_pointer_after_index, BrilligBinaryOp::Add, @@ -220,7 +284,7 @@ impl<'block> BrilligBlock<'block> { // Compute the number of elements to the right of the index let item_count = self.brillig_context.allocate_register(); self.brillig_context.memory_op_instruction( - source_vector.size, + source_size.address, index.address, item_count, BrilligBinaryOp::Sub, @@ -243,10 +307,10 @@ impl<'block> BrilligBlock<'block> { target_index.address, BrilligBinaryOp::Add, ); - self.brillig_context.codegen_store_variable_in_array( - target_vector.pointer, + self.brillig_context.codegen_store_with_offset( + target_vector_items_pointer, target_index, - *variable, + variable.extract_register(), ); self.brillig_context.deallocate_single_addr(target_index); } @@ -254,6 +318,10 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.deallocate_register(source_pointer_at_index); self.brillig_context.deallocate_register(target_pointer_after_index); self.brillig_context.deallocate_register(item_count); + self.brillig_context.deallocate_single_addr(source_size); + self.brillig_context.deallocate_single_addr(target_size); + self.brillig_context.deallocate_register(source_vector_items_pointer); + self.brillig_context.deallocate_register(target_vector_items_pointer); } pub(crate) fn slice_remove_operation( @@ -264,23 +332,34 @@ impl<'block> BrilligBlock<'block> { removed_items: &[BrilligVariable], ) { // First we need to allocate the target vector decrementing the size by removed_items.len() + let source_size = self.brillig_context.codegen_make_vector_length(source_vector); + + let target_size = SingleAddrVariable::new_usize(self.brillig_context.allocate_register()); self.brillig_context.codegen_usize_op( - source_vector.size, - target_vector.size, + source_size.address, + target_size.address, BrilligBinaryOp::Sub, removed_items.len(), ); - self.brillig_context.codegen_allocate_array(target_vector.pointer, target_vector.size); - // We initialize the RC of the target vector to 1 - self.brillig_context.usize_const_instruction(target_vector.rc, 1_usize.into()); + + self.brillig_context.codegen_initialize_vector(target_vector, target_size); // Copy the elements to the left of the index - self.brillig_context.codegen_mem_copy(source_vector.pointer, target_vector.pointer, index); + let source_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(source_vector); + let target_vector_items_pointer = + self.brillig_context.codegen_make_vector_items_pointer(target_vector); + + self.brillig_context.codegen_mem_copy( + source_vector_items_pointer, + target_vector_items_pointer, + index, + ); // Compute the source pointer after the removed items let source_pointer_after_index = self.brillig_context.allocate_register(); self.brillig_context.memory_op_instruction( - source_vector.pointer, + source_vector_items_pointer, index.address, source_pointer_after_index, BrilligBinaryOp::Add, @@ -294,7 +373,7 @@ impl<'block> BrilligBlock<'block> { // Compute the target pointer at the index let target_pointer_at_index = self.brillig_context.allocate_register(); self.brillig_context.memory_op_instruction( - target_vector.pointer, + target_vector_items_pointer, index.address, target_pointer_at_index, BrilligBinaryOp::Add, @@ -303,7 +382,7 @@ impl<'block> BrilligBlock<'block> { // Compute the number of elements to the right of the index let item_count = self.brillig_context.allocate_register(); self.brillig_context.memory_op_instruction( - source_vector.size, + source_size.address, index.address, item_count, BrilligBinaryOp::Sub, @@ -331,26 +410,21 @@ impl<'block> BrilligBlock<'block> { target_index.address, BrilligBinaryOp::Add, ); - self.retrieve_variable_from_array(source_vector.pointer, target_index, *variable); + self.brillig_context.codegen_load_with_offset( + source_vector_items_pointer, + target_index, + variable.extract_register(), + ); self.brillig_context.deallocate_single_addr(target_index); } self.brillig_context.deallocate_register(source_pointer_after_index); self.brillig_context.deallocate_register(target_pointer_at_index); self.brillig_context.deallocate_register(item_count); - } - - pub(crate) fn convert_array_or_vector_to_vector( - &mut self, - source_variable: BrilligVariable, - ) -> BrilligVector { - match source_variable { - BrilligVariable::BrilligVector(source_vector) => source_vector, - BrilligVariable::BrilligArray(source_array) => { - self.brillig_context.array_to_vector_instruction(&source_array) - } - _ => unreachable!("ICE: unsupported slice push back source {:?}", source_variable), - } + self.brillig_context.deallocate_single_addr(source_size); + self.brillig_context.deallocate_single_addr(target_size); + self.brillig_context.deallocate_register(source_vector_items_pointer); + self.brillig_context.deallocate_register(target_vector_items_pointer); } } @@ -365,7 +439,7 @@ mod tests { use crate::brillig::brillig_gen::brillig_fn::FunctionContext; use crate::brillig::brillig_ir::artifact::{BrilligParameter, Label}; use crate::brillig::brillig_ir::brillig_variable::{ - BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable, + BrilligVariable, BrilligVector, SingleAddrVariable, }; use crate::brillig::brillig_ir::registers::Stack; use crate::brillig::brillig_ir::tests::{ @@ -410,42 +484,33 @@ mod tests { push_back: bool, array: Vec, item_to_push: FieldElement, - expected_return: Vec, + mut expected_return: Vec, ) { let arguments = vec![ - BrilligParameter::Array( + BrilligParameter::Slice( vec![BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE)], array.len(), ), BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), ]; + let result_length = array.len() + 1; let returns = vec![BrilligParameter::Array( vec![BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE)], - array.len() + 1, + result_length + 1, // Leading length since the we return a vector )]; + expected_return.insert(0, FieldElement::from(result_length)); let (_, mut function_context, mut context) = create_test_environment(); // Allocate the parameters - let array_variable = BrilligArray { - pointer: context.allocate_register(), - size: array.len(), - rc: context.allocate_register(), - }; + let source_vector = BrilligVector { pointer: context.allocate_register() }; let item_to_insert = SingleAddrVariable { address: context.allocate_register(), bit_size: BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, }; - // Cast the source array to a vector - let source_vector = context.array_to_vector_instruction(&array_variable); - // Allocate the results - let target_vector = BrilligVector { - pointer: context.allocate_register(), - size: context.allocate_register(), - rc: context.allocate_register(), - }; + let target_vector = BrilligVector { pointer: context.allocate_register() }; let mut block = create_brillig_block(&mut function_context, &mut context); @@ -463,7 +528,7 @@ mod tests { ); } - context.codegen_return(&[target_vector.pointer, target_vector.rc]); + context.codegen_return(&[target_vector.pointer]); let bytecode = create_entry_point_bytecode(context, arguments, returns).byte_code; let (vm, return_data_offset, return_data_size) = @@ -527,39 +592,31 @@ mod tests { fn test_case_pop( pop_back: bool, array: Vec, - expected_return_array: Vec, + mut expected_return_array: Vec, expected_return_item: FieldElement, ) { - let arguments = vec![BrilligParameter::Array( + let arguments = vec![BrilligParameter::Slice( vec![BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE)], array.len(), )]; + let result_length = array.len() - 1; + let returns = vec![ BrilligParameter::Array( vec![BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE)], - array.len() - 1, + result_length + 1, ), BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), ]; + expected_return_array.insert(0, FieldElement::from(result_length)); let (_, mut function_context, mut context) = create_test_environment(); // Allocate the parameters - let array_variable = BrilligArray { - pointer: context.allocate_register(), - size: array.len(), - rc: context.allocate_register(), - }; - - // Cast the source array to a vector - let source_vector = context.array_to_vector_instruction(&array_variable); + let source_vector = BrilligVector { pointer: context.allocate_register() }; // Allocate the results - let target_vector = BrilligVector { - pointer: context.allocate_register(), - size: context.allocate_register(), - rc: context.allocate_register(), - }; + let target_vector = BrilligVector { pointer: context.allocate_register() }; let removed_item = SingleAddrVariable { address: context.allocate_register(), bit_size: BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, @@ -581,11 +638,7 @@ mod tests { ); } - context.codegen_return(&[ - target_vector.pointer, - target_vector.rc, - removed_item.address, - ]); + context.codegen_return(&[target_vector.pointer, removed_item.address]); let bytecode = create_entry_point_bytecode(context, arguments, returns).byte_code; let expected_return: Vec<_> = @@ -632,29 +685,27 @@ mod tests { array: Vec, item: FieldElement, index: FieldElement, - expected_return: Vec, + mut expected_return: Vec, ) { let arguments = vec![ - BrilligParameter::Array( + BrilligParameter::Slice( vec![BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE)], array.len(), ), BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), ]; + let result_length = array.len() + 1; let returns = vec![BrilligParameter::Array( vec![BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE)], - array.len() + 1, + result_length + 1, )]; + expected_return.insert(0, FieldElement::from(result_length)); let (_, mut function_context, mut context) = create_test_environment(); // Allocate the parameters - let array_variable = BrilligArray { - pointer: context.allocate_register(), - size: array.len(), - rc: context.allocate_register(), - }; + let source_vector = BrilligVector { pointer: context.allocate_register() }; let item_to_insert = SingleAddrVariable { address: context.allocate_register(), bit_size: BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, @@ -664,15 +715,8 @@ mod tests { BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, ); - // Cast the source array to a vector - let source_vector = context.array_to_vector_instruction(&array_variable); - // Allocate the results - let target_vector = BrilligVector { - pointer: context.allocate_register(), - size: context.allocate_register(), - rc: context.allocate_register(), - }; + let target_vector = BrilligVector { pointer: context.allocate_register() }; let mut block = create_brillig_block(&mut function_context, &mut context); @@ -683,7 +727,7 @@ mod tests { &[BrilligVariable::SingleAddr(item_to_insert)], ); - context.codegen_return(&[target_vector.pointer, target_vector.rc]); + context.codegen_return(&[target_vector.pointer]); let calldata = array.into_iter().chain(vec![item]).chain(vec![index]).collect(); let bytecode = create_entry_point_bytecode(context, arguments, returns).byte_code; @@ -773,46 +817,38 @@ mod tests { fn test_case_remove( array: Vec, index: FieldElement, - expected_array: Vec, + mut expected_array: Vec, expected_removed_item: FieldElement, ) { let arguments = vec![ - BrilligParameter::Array( + BrilligParameter::Slice( vec![BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE)], array.len(), ), BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), ]; + let result_length = array.len() - 1; + let returns = vec![ BrilligParameter::Array( vec![BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE)], - array.len() - 1, + result_length + 1, ), BrilligParameter::SingleAddr(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), ]; + expected_array.insert(0, FieldElement::from(result_length)); let (_, mut function_context, mut context) = create_test_environment(); // Allocate the parameters - let array_variable = BrilligArray { - pointer: context.allocate_register(), - size: array.len(), - rc: context.allocate_register(), - }; + let source_vector = BrilligVector { pointer: context.allocate_register() }; let index_to_insert = SingleAddrVariable::new( context.allocate_register(), BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, ); - // Cast the source array to a vector - let source_vector = context.array_to_vector_instruction(&array_variable); - // Allocate the results - let target_vector = BrilligVector { - pointer: context.allocate_register(), - size: context.allocate_register(), - rc: context.allocate_register(), - }; + let target_vector = BrilligVector { pointer: context.allocate_register() }; let removed_item = SingleAddrVariable { address: context.allocate_register(), bit_size: BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, @@ -827,11 +863,7 @@ mod tests { &[BrilligVariable::SingleAddr(removed_item)], ); - context.codegen_return(&[ - target_vector.pointer, - target_vector.size, - removed_item.address, - ]); + context.codegen_return(&[target_vector.pointer, removed_item.address]); let calldata: Vec<_> = array.into_iter().chain(vec![index]).collect(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 05117b96c5d..b52b239e6b9 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -237,8 +237,12 @@ pub(crate) mod tests { returns: Vec, ) -> GeneratedBrillig { let artifact = context.artifact(); - let mut entry_point_artifact = - BrilligContext::new_entry_point_artifact(arguments, returns, FunctionId::test_new(0)); + let mut entry_point_artifact = BrilligContext::new_entry_point_artifact( + arguments, + returns, + FunctionId::test_new(0), + true, + ); entry_point_artifact.link_with(&artifact); entry_point_artifact.finish() } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/brillig_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/brillig_variable.rs index 3200bd54265..81d61e05cc4 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/brillig_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/brillig_variable.rs @@ -1,6 +1,6 @@ use acvm::{ acir::{brillig::BitSize, AcirField}, - brillig_vm::brillig::{HeapArray, HeapValueType, HeapVector, MemoryAddress, ValueOrArray}, + brillig_vm::brillig::{HeapValueType, MemoryAddress}, FieldElement, }; use serde::{Deserialize, Serialize}; @@ -34,43 +34,12 @@ impl SingleAddrVariable { pub(crate) struct BrilligArray { pub(crate) pointer: MemoryAddress, pub(crate) size: usize, - pub(crate) rc: MemoryAddress, -} - -impl BrilligArray { - pub(crate) fn to_heap_array(self) -> HeapArray { - HeapArray { pointer: self.pointer, size: self.size } - } - - pub(crate) fn registers_count() -> usize { - 2 - } - - pub(crate) fn extract_registers(self) -> Vec { - vec![self.pointer, self.rc] - } } /// The representation of a noir slice in the Brillig IR #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy)] pub(crate) struct BrilligVector { pub(crate) pointer: MemoryAddress, - pub(crate) size: MemoryAddress, - pub(crate) rc: MemoryAddress, -} - -impl BrilligVector { - pub(crate) fn to_heap_vector(self) -> HeapVector { - HeapVector { pointer: self.pointer, size: self.size } - } - - pub(crate) fn registers_count() -> usize { - 3 - } - - pub(crate) fn extract_registers(self) -> Vec { - vec![self.pointer, self.size, self.rc] - } } /// The representation of a noir value in the Brillig IR @@ -103,23 +72,11 @@ impl BrilligVariable { } } - pub(crate) fn extract_registers(self) -> Vec { - match self { - BrilligVariable::SingleAddr(single_addr) => vec![single_addr.address], - BrilligVariable::BrilligArray(array) => array.extract_registers(), - BrilligVariable::BrilligVector(vector) => vector.extract_registers(), - } - } - - pub(crate) fn to_value_or_array(self) -> ValueOrArray { + pub(crate) fn extract_register(self) -> MemoryAddress { match self { - BrilligVariable::SingleAddr(single_addr) => { - ValueOrArray::MemoryAddress(single_addr.address) - } - BrilligVariable::BrilligArray(array) => ValueOrArray::HeapArray(array.to_heap_array()), - BrilligVariable::BrilligVector(vector) => { - ValueOrArray::HeapVector(vector.to_heap_vector()) - } + BrilligVariable::SingleAddr(single_addr) => single_addr.address, + BrilligVariable::BrilligArray(array) => array.pointer, + BrilligVariable::BrilligVector(vector) => vector.pointer, } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_calls.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_calls.rs index e65e2766257..185a6a08a04 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_calls.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_calls.rs @@ -13,8 +13,7 @@ impl BrilligContext< // // Note that here it is important that the stack pointer register is at register 0, // as after the first register save we add to the pointer. - let mut used_registers: Vec<_> = - vars.iter().flat_map(|var| var.extract_registers()).collect(); + let mut used_registers: Vec<_> = vars.iter().map(|var| var.extract_register()).collect(); // Also dump the previous stack pointer used_registers.push(ReservedRegisters::previous_stack_pointer()); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs index 85aee0676cd..e5b57293d1e 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs @@ -162,7 +162,7 @@ impl BrilligContext< // + 1 due to the revert data id being the first item returned size: Self::flattened_tuple_size(&revert_data_types) + 1, }; - ctx.codegen_allocate_fixed_length_array(revert_data.pointer, revert_data.size); + ctx.codegen_allocate_immediate_mem(revert_data.pointer, revert_data.size); let current_revert_data_pointer = ctx.allocate_register(); ctx.mov_instruction(current_revert_data_pointer, revert_data.pointer); @@ -185,14 +185,17 @@ impl BrilligContext< ); } BrilligParameter::Array(item_type, item_count) => { - let variable_pointer = revert_variable.extract_array().pointer; + let deflattened_items_pointer = + ctx.codegen_make_array_items_pointer(revert_variable.extract_array()); ctx.flatten_array( &item_type, item_count, current_revert_data_pointer, - variable_pointer, + deflattened_items_pointer, ); + + ctx.deallocate_register(deflattened_items_pointer); } BrilligParameter::Slice(_, _) => { unimplemented!("Slices are not supported as revert data") @@ -255,7 +258,7 @@ impl BrilligContext< item_type: &[BrilligParameter], item_count: usize, flattened_array_pointer: MemoryAddress, - deflattened_array_pointer: MemoryAddress, + deflattened_items_pointer: MemoryAddress, ) { if Self::has_nested_arrays(item_type) { let movement_register = self.allocate_register(); @@ -279,12 +282,12 @@ impl BrilligContext< match subitem { BrilligParameter::SingleAddr(_) => { - self.codegen_array_get( - deflattened_array_pointer, + self.codegen_load_with_offset( + deflattened_items_pointer, source_index, movement_register, ); - self.codegen_array_set( + self.codegen_store_with_offset( flattened_array_pointer, target_index, movement_register, @@ -295,34 +298,22 @@ impl BrilligContext< nested_array_item_type, nested_array_item_count, ) => { - let nested_array_reference = self.allocate_register(); - self.codegen_array_get( - deflattened_array_pointer, - source_index, - nested_array_reference, - ); + let deflattened_nested_array = BrilligArray { + pointer: self.allocate_register(), + size: *nested_array_item_count, + }; - let nested_array_variable = - BrilligVariable::BrilligArray(BrilligArray { - pointer: self.allocate_register(), - size: nested_array_item_type.len() * nested_array_item_count, - rc: self.allocate_register(), - }); - - self.codegen_load_variable( - nested_array_variable, - nested_array_reference, + self.codegen_load_with_offset( + deflattened_items_pointer, + source_index, + deflattened_nested_array.pointer, ); + let deflattened_nested_array_items = + self.codegen_make_array_items_pointer(deflattened_nested_array); let flattened_nested_array_pointer = self.allocate_register(); - - self.mov_instruction( - flattened_nested_array_pointer, - flattened_array_pointer, - ); - self.memory_op_instruction( - flattened_nested_array_pointer, + flattened_array_pointer, target_index.address, flattened_nested_array_pointer, BrilligBinaryOp::Add, @@ -332,15 +323,12 @@ impl BrilligContext< nested_array_item_type, *nested_array_item_count, flattened_nested_array_pointer, - nested_array_variable.extract_array().pointer, + deflattened_nested_array_items, ); - self.deallocate_register(nested_array_reference); + self.deallocate_register(deflattened_nested_array.pointer); + self.deallocate_register(deflattened_nested_array_items); self.deallocate_register(flattened_nested_array_pointer); - nested_array_variable - .extract_registers() - .into_iter() - .for_each(|register| self.deallocate_register(register)); target_offset += Self::flattened_size(subitem); } @@ -356,7 +344,7 @@ impl BrilligContext< } else { let item_count = self.make_usize_constant_instruction((item_count * item_type.len()).into()); - self.codegen_mem_copy(deflattened_array_pointer, flattened_array_pointer, item_count); + self.codegen_mem_copy(deflattened_items_pointer, flattened_array_pointer, item_count); self.deallocate_single_addr(item_count); } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs index 21f68daab64..d92412677ca 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs @@ -74,20 +74,22 @@ impl BrilligContext< ) { assert!(source_field.bit_size == F::max_num_bits()); - let size = SingleAddrVariable::new_usize(self.allocate_register()); - self.usize_const_instruction(size.address, target_array.size.into()); - self.usize_const_instruction(target_array.rc, 1_usize.into()); - self.codegen_allocate_array(target_array.pointer, size.address); + self.codegen_initialize_array(target_array); + + let heap_array = self.codegen_brillig_array_to_heap_array(target_array); self.black_box_op_instruction(BlackBoxOp::ToRadix { input: source_field.address, radix, - output: target_array.to_heap_array(), + output: heap_array, output_bits, }); if big_endian { - self.codegen_array_reverse(target_array.pointer, size.address); + let items_len = self.make_usize_constant_instruction(target_array.size.into()); + self.codegen_array_reverse(heap_array.pointer, items_len.address); + self.deallocate_single_addr(items_len); } + self.deallocate_register(heap_array.pointer); } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs index a33024dc382..ea8969eddf3 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs @@ -1,4 +1,7 @@ -use acvm::{acir::brillig::MemoryAddress, AcirField}; +use acvm::{ + acir::brillig::{HeapArray, HeapVector, MemoryAddress, ValueOrArray}, + AcirField, +}; use crate::brillig::brillig_ir::BrilligBinaryOp; @@ -6,25 +9,25 @@ use super::{ brillig_variable::{BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable}, debug_show::DebugToString, registers::RegisterAllocator, - BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, + BrilligContext, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, }; impl BrilligContext { /// Allocates an array of size `size` and stores the pointer to the array /// in `pointer_register` - pub(crate) fn codegen_allocate_fixed_length_array( + pub(crate) fn codegen_allocate_immediate_mem( &mut self, pointer_register: MemoryAddress, size: usize, ) { let size_register = self.make_usize_constant_instruction(size.into()); - self.codegen_allocate_array(pointer_register, size_register.address); + self.codegen_allocate_mem(pointer_register, size_register.address); self.deallocate_single_addr(size_register); } /// Allocates an array of size contained in size_register and stores the /// pointer to the array in `pointer_register` - pub(crate) fn codegen_allocate_array( + pub(crate) fn codegen_allocate_mem( &mut self, pointer_register: MemoryAddress, size_register: MemoryAddress, @@ -33,128 +36,40 @@ impl BrilligContext< self.increase_free_memory_pointer_instruction(size_register); } - /// Allocates a variable in memory and stores the - /// pointer to the array in `pointer_register` - fn codegen_allocate_variable_reference( - &mut self, - pointer_register: MemoryAddress, - size: usize, - ) { - // A variable can be stored in up to three values, so we reserve three values for that. - let size_register = self.make_usize_constant_instruction(size.into()); - self.mov_instruction(pointer_register, ReservedRegisters::free_memory_pointer()); - self.memory_op_instruction( - ReservedRegisters::free_memory_pointer(), - size_register.address, - ReservedRegisters::free_memory_pointer(), - BrilligBinaryOp::Add, - ); - self.deallocate_single_addr(size_register); - } - - pub(crate) fn codegen_allocate_single_addr_reference( + /// Gets the value stored at base_ptr + index and stores it in result + pub(crate) fn codegen_load_with_offset( &mut self, - pointer_register: MemoryAddress, - ) { - self.codegen_allocate_variable_reference(pointer_register, 1); - } - - pub(crate) fn codegen_allocate_array_reference(&mut self, pointer_register: MemoryAddress) { - self.codegen_allocate_variable_reference(pointer_register, BrilligArray::registers_count()); - } - - pub(crate) fn codegen_allocate_vector_reference(&mut self, pointer_register: MemoryAddress) { - self.codegen_allocate_variable_reference( - pointer_register, - BrilligVector::registers_count(), - ); - } - - /// Gets the value in the array at index `index` and stores it in `result` - pub(crate) fn codegen_array_get( - &mut self, - array_ptr: MemoryAddress, + base_ptr: MemoryAddress, index: SingleAddrVariable, result: MemoryAddress, ) { assert!(index.bit_size == BRILLIG_MEMORY_ADDRESSING_BIT_SIZE); - // Computes array_ptr + index, ie array[index] - let index_of_element_in_memory = self.allocate_register(); - self.memory_op_instruction( - array_ptr, - index.address, - index_of_element_in_memory, - BrilligBinaryOp::Add, - ); - self.load_instruction(result, index_of_element_in_memory); + let final_index = self.allocate_register(); + self.memory_op_instruction(base_ptr, index.address, final_index, BrilligBinaryOp::Add); + self.load_instruction(result, final_index); // Free up temporary register - self.deallocate_register(index_of_element_in_memory); + self.deallocate_register(final_index); } - /// Sets the item in the array at index `index` to `value` - pub(crate) fn codegen_array_set( + /// Stores value at base_ptr + index + pub(crate) fn codegen_store_with_offset( &mut self, - array_ptr: MemoryAddress, + base_ptr: MemoryAddress, index: SingleAddrVariable, value: MemoryAddress, ) { assert!(index.bit_size == BRILLIG_MEMORY_ADDRESSING_BIT_SIZE); - // Computes array_ptr + index, ie array[index] - let index_of_element_in_memory = self.allocate_register(); + let final_index = self.allocate_register(); self.binary_instruction( - SingleAddrVariable::new_usize(array_ptr), + SingleAddrVariable::new_usize(base_ptr), index, - SingleAddrVariable::new_usize(index_of_element_in_memory), + SingleAddrVariable::new_usize(final_index), BrilligBinaryOp::Add, ); - self.store_instruction(index_of_element_in_memory, value); + self.store_instruction(final_index, value); // Free up temporary register - self.deallocate_register(index_of_element_in_memory); - } - - pub(crate) fn codegen_store_variable_in_array( - &mut self, - array_pointer: MemoryAddress, - index: SingleAddrVariable, - value_variable: BrilligVariable, - ) { - assert!(index.bit_size == BRILLIG_MEMORY_ADDRESSING_BIT_SIZE); - let final_pointer_register = self.allocate_register(); - self.memory_op_instruction( - array_pointer, - index.address, - final_pointer_register, - BrilligBinaryOp::Add, - ); - self.codegen_store_variable_in_pointer(final_pointer_register, value_variable); - self.deallocate_register(final_pointer_register); - } - - pub(crate) fn codegen_store_variable_in_pointer( - &mut self, - destination_pointer: MemoryAddress, - value_variable: BrilligVariable, - ) { - match value_variable { - BrilligVariable::SingleAddr(value_variable) => { - self.store_instruction(destination_pointer, value_variable.address); - } - BrilligVariable::BrilligArray(_) => { - let reference: MemoryAddress = self.allocate_register(); - self.codegen_allocate_array_reference(reference); - self.codegen_store_variable(reference, value_variable); - self.store_instruction(destination_pointer, reference); - self.deallocate_register(reference); - } - BrilligVariable::BrilligVector(_) => { - let reference = self.allocate_register(); - self.codegen_allocate_vector_reference(reference); - self.codegen_store_variable(reference, value_variable); - self.store_instruction(destination_pointer, reference); - self.deallocate_register(reference); - } - } + self.deallocate_register(final_index); } /// Copies the values of memory pointed by source with length stored in `num_elements_register` @@ -177,96 +92,22 @@ impl BrilligContext< let value_register = self.allocate_register(); self.codegen_loop(num_elements_variable.address, |ctx, iterator| { - ctx.codegen_array_get(source_pointer, iterator, value_register); - ctx.codegen_array_set(destination_pointer, iterator, value_register); + ctx.codegen_load_with_offset(source_pointer, iterator, value_register); + ctx.codegen_store_with_offset(destination_pointer, iterator, value_register); }); self.deallocate_register(value_register); } } - /// Loads a variable stored previously - pub(crate) fn codegen_load_variable( - &mut self, - destination: BrilligVariable, - variable_pointer: MemoryAddress, - ) { - match destination { - BrilligVariable::SingleAddr(single_addr) => { - self.load_instruction(single_addr.address, variable_pointer); - } - BrilligVariable::BrilligArray(BrilligArray { pointer, size: _, rc }) => { - self.load_instruction(pointer, variable_pointer); - - let rc_pointer = self.allocate_register(); - self.mov_instruction(rc_pointer, variable_pointer); - self.codegen_usize_op_in_place(rc_pointer, BrilligBinaryOp::Add, 1_usize); - - self.load_instruction(rc, rc_pointer); - self.deallocate_register(rc_pointer); - } - BrilligVariable::BrilligVector(BrilligVector { pointer, size, rc }) => { - self.load_instruction(pointer, variable_pointer); - - let size_pointer = self.allocate_register(); - self.mov_instruction(size_pointer, variable_pointer); - self.codegen_usize_op_in_place(size_pointer, BrilligBinaryOp::Add, 1_usize); - - self.load_instruction(size, size_pointer); - self.deallocate_register(size_pointer); - - let rc_pointer = self.allocate_register(); - self.mov_instruction(rc_pointer, variable_pointer); - self.codegen_usize_op_in_place(rc_pointer, BrilligBinaryOp::Add, 2_usize); - - self.load_instruction(rc, rc_pointer); - self.deallocate_register(rc_pointer); - } - } - } - - /// Stores a variable by saving its registers to memory - pub(crate) fn codegen_store_variable( + /// This instruction will reverse the order of the `size` elements pointed by `pointer`. + pub(crate) fn codegen_array_reverse( &mut self, - variable_pointer: MemoryAddress, - source: BrilligVariable, + items_pointer: MemoryAddress, + size: MemoryAddress, ) { - match source { - BrilligVariable::SingleAddr(single_addr) => { - self.store_instruction(variable_pointer, single_addr.address); - } - BrilligVariable::BrilligArray(BrilligArray { pointer, size: _, rc }) => { - self.store_instruction(variable_pointer, pointer); - - let rc_pointer: MemoryAddress = self.allocate_register(); - self.mov_instruction(rc_pointer, variable_pointer); - self.codegen_usize_op_in_place(rc_pointer, BrilligBinaryOp::Add, 1_usize); - self.store_instruction(rc_pointer, rc); - self.deallocate_register(rc_pointer); - } - BrilligVariable::BrilligVector(BrilligVector { pointer, size, rc }) => { - self.store_instruction(variable_pointer, pointer); - - let size_pointer = self.allocate_register(); - self.mov_instruction(size_pointer, variable_pointer); - self.codegen_usize_op_in_place(size_pointer, BrilligBinaryOp::Add, 1_usize); - self.store_instruction(size_pointer, size); - - let rc_pointer: MemoryAddress = self.allocate_register(); - self.mov_instruction(rc_pointer, variable_pointer); - self.codegen_usize_op_in_place(rc_pointer, BrilligBinaryOp::Add, 2_usize); - self.store_instruction(rc_pointer, rc); - - self.deallocate_register(size_pointer); - self.deallocate_register(rc_pointer); - } - } - } - - /// This instruction will reverse the order of the `size` elements pointed by `pointer`. - pub(crate) fn codegen_array_reverse(&mut self, pointer: MemoryAddress, size: MemoryAddress) { if self.can_call_procedures { - self.call_array_reverse_procedure(pointer, size); + self.call_array_reverse_procedure(items_pointer, size); return; } @@ -285,12 +126,20 @@ impl BrilligContext< let index_at_end_of_array_var = SingleAddrVariable::new_usize(index_at_end_of_array); // Load both values - ctx.codegen_array_get(pointer, iterator_register, start_value_register); - ctx.codegen_array_get(pointer, index_at_end_of_array_var, end_value_register); + ctx.codegen_load_with_offset(items_pointer, iterator_register, start_value_register); + ctx.codegen_load_with_offset( + items_pointer, + index_at_end_of_array_var, + end_value_register, + ); // Write both values - ctx.codegen_array_set(pointer, iterator_register, end_value_register); - ctx.codegen_array_set(pointer, index_at_end_of_array_var, start_value_register); + ctx.codegen_store_with_offset(items_pointer, iterator_register, end_value_register); + ctx.codegen_store_with_offset( + items_pointer, + index_at_end_of_array_var, + start_value_register, + ); }); self.deallocate_register(iteration_count); @@ -298,4 +147,140 @@ impl BrilligContext< self.deallocate_register(end_value_register); self.deallocate_register(index_at_end_of_array); } + + /// Converts a BrilligArray (pointer to [RC, ...items]) to a HeapArray (pointer to [items]) + pub(crate) fn codegen_brillig_array_to_heap_array(&mut self, array: BrilligArray) -> HeapArray { + let heap_array = HeapArray { pointer: self.allocate_register(), size: array.size }; + self.codegen_usize_op(array.pointer, heap_array.pointer, BrilligBinaryOp::Add, 1); + heap_array + } + + pub(crate) fn codegen_brillig_vector_to_heap_vector( + &mut self, + vector: BrilligVector, + ) -> HeapVector { + let heap_vector = + HeapVector { pointer: self.allocate_register(), size: self.allocate_register() }; + let current_pointer = self.allocate_register(); + + // Prepare a pointer to the size + self.codegen_usize_op(vector.pointer, current_pointer, BrilligBinaryOp::Add, 1); + self.load_instruction(heap_vector.size, current_pointer); + // Now prepare the pointer to the items + self.codegen_usize_op(current_pointer, heap_vector.pointer, BrilligBinaryOp::Add, 1); + + self.deallocate_register(current_pointer); + heap_vector + } + + pub(crate) fn variable_to_value_or_array(&mut self, variable: BrilligVariable) -> ValueOrArray { + match variable { + BrilligVariable::SingleAddr(SingleAddrVariable { address, .. }) => { + ValueOrArray::MemoryAddress(address) + } + BrilligVariable::BrilligArray(array) => { + ValueOrArray::HeapArray(self.codegen_brillig_array_to_heap_array(array)) + } + BrilligVariable::BrilligVector(vector) => { + ValueOrArray::HeapVector(self.codegen_brillig_vector_to_heap_vector(vector)) + } + } + } + + /// Returns a variable holding the length of a given vector + pub(crate) fn codegen_make_vector_length( + &mut self, + vector: BrilligVector, + ) -> SingleAddrVariable { + let result = SingleAddrVariable::new_usize(self.allocate_register()); + self.codegen_usize_op(vector.pointer, result.address, BrilligBinaryOp::Add, 1); + self.load_instruction(result.address, result.address); + result + } + + /// Returns a pointer to the items of a given vector + pub(crate) fn codegen_make_vector_items_pointer( + &mut self, + vector: BrilligVector, + ) -> MemoryAddress { + let result = self.allocate_register(); + self.codegen_usize_op(vector.pointer, result, BrilligBinaryOp::Add, 2); + result + } + + /// Returns a variable holding the length of a given array + pub(crate) fn codegen_make_array_length(&mut self, array: BrilligArray) -> SingleAddrVariable { + let result = SingleAddrVariable::new_usize(self.allocate_register()); + self.usize_const_instruction(result.address, array.size.into()); + result + } + + /// Returns a pointer to the items of a given array + pub(crate) fn codegen_make_array_items_pointer( + &mut self, + array: BrilligArray, + ) -> MemoryAddress { + let result = self.allocate_register(); + self.codegen_usize_op(array.pointer, result, BrilligBinaryOp::Add, 1); + result + } + + pub(crate) fn codegen_make_array_or_vector_length( + &mut self, + variable: BrilligVariable, + ) -> SingleAddrVariable { + match variable { + BrilligVariable::BrilligArray(array) => self.codegen_make_array_length(array), + BrilligVariable::BrilligVector(vector) => self.codegen_make_vector_length(vector), + _ => unreachable!("ICE: Expected array or vector, got {variable:?}"), + } + } + + pub(crate) fn codegen_make_array_or_vector_items_pointer( + &mut self, + variable: BrilligVariable, + ) -> MemoryAddress { + match variable { + BrilligVariable::BrilligArray(array) => self.codegen_make_array_items_pointer(array), + BrilligVariable::BrilligVector(vector) => { + self.codegen_make_vector_items_pointer(vector) + } + _ => unreachable!("ICE: Expected array or vector, got {variable:?}"), + } + } + + /// Initializes an array, allocating memory to store its representation and initializing the reference counter. + pub(crate) fn codegen_initialize_array(&mut self, array: BrilligArray) { + self.codegen_allocate_immediate_mem(array.pointer, array.size + 1); + self.indirect_const_instruction( + array.pointer, + BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, + 1_usize.into(), + ); + } + + /// Initializes a vector, allocating memory to store its representation and initializing the reference counter and size. + pub(crate) fn codegen_initialize_vector( + &mut self, + vector: BrilligVector, + size: SingleAddrVariable, + ) { + let allocation_size = self.allocate_register(); + self.codegen_usize_op(size.address, allocation_size, BrilligBinaryOp::Add, 2); + self.codegen_allocate_mem(vector.pointer, allocation_size); + self.deallocate_register(allocation_size); + + // Write RC + self.indirect_const_instruction( + vector.pointer, + BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, + 1_usize.into(), + ); + + // Write size + let len_write_pointer = self.allocate_register(); + self.codegen_usize_op(vector.pointer, len_write_pointer, BrilligBinaryOp::Add, 1); + self.store_instruction(len_write_pointer, size.address); + self.deallocate_register(len_write_pointer); + } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index c85940cc1c7..bde128d0b6b 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -7,7 +7,7 @@ use super::{ registers::Stack, BrilligBinaryOp, BrilligContext, ReservedRegisters, }; -use acvm::{acir::brillig::MemoryAddress, acir::AcirField}; +use acvm::acir::{brillig::MemoryAddress, AcirField}; pub(crate) const MAX_STACK_SIZE: usize = 2048; pub(crate) const MAX_SCRATCH_SPACE: usize = 64; @@ -18,10 +18,12 @@ impl BrilligContext { arguments: Vec, return_parameters: Vec, target_function: FunctionId, + disable_procedures: bool, ) -> BrilligArtifact { let mut context = BrilligContext::new(false); - context.disable_procedures(); - + if disable_procedures { + context.disable_procedures(); + } context.codegen_entry_point(&arguments, &return_parameters); context.add_external_call_instruction(target_function); @@ -85,11 +87,9 @@ impl BrilligContext { ) => { let flattened_size = array.size; self.usize_const_instruction(array.pointer, current_calldata_pointer.into()); - self.usize_const_instruction(array.rc, 1_usize.into()); - // Deflatten the array let deflattened_address = - self.deflatten_array(item_type, array.size, array.pointer); + self.deflatten_array(item_type, *item_count, array.pointer, false); self.mov_instruction(array.pointer, deflattened_address); array.size = item_type.len() * item_count; self.deallocate_register(deflattened_address); @@ -102,17 +102,10 @@ impl BrilligContext { ) => { let flattened_size = Self::flattened_size(argument); self.usize_const_instruction(vector.pointer, current_calldata_pointer.into()); - self.usize_const_instruction(vector.rc, 1_usize.into()); - self.usize_const_instruction(vector.size, flattened_size.into()); - - // Deflatten the vector let deflattened_address = - self.deflatten_array(item_type, flattened_size, vector.pointer); + self.deflatten_array(item_type, *item_count, vector.pointer, true); self.mov_instruction(vector.pointer, deflattened_address); - self.usize_const_instruction( - vector.size, - (item_type.len() * item_count).into(), - ); + self.deallocate_register(deflattened_address); current_calldata_pointer += flattened_size; @@ -140,13 +133,10 @@ impl BrilligContext { BrilligVariable::BrilligArray(BrilligArray { pointer: self.allocate_register(), size: flattened_size, - rc: self.allocate_register(), }) } BrilligParameter::Slice(_, _) => BrilligVariable::BrilligVector(BrilligVector { pointer: self.allocate_register(), - size: self.allocate_register(), - rc: self.allocate_register(), }), }) .collect() @@ -191,19 +181,33 @@ impl BrilligContext { item_type: &[BrilligParameter], item_count: usize, flattened_array_pointer: MemoryAddress, + is_vector: bool, ) -> MemoryAddress { + let deflattened_array_pointer = self.allocate_register(); + let deflattened_size_variable = + self.make_usize_constant_instruction((item_count * item_type.len()).into()); + + let deflattened_items_pointer = if is_vector { + let vector = BrilligVector { pointer: deflattened_array_pointer }; + + self.codegen_initialize_vector(vector, deflattened_size_variable); + + self.codegen_make_vector_items_pointer(vector) + } else { + let arr = BrilligArray { + pointer: deflattened_array_pointer, + size: item_count * item_type.len(), + }; + self.codegen_initialize_array(arr); + self.codegen_make_array_items_pointer(arr) + }; + if Self::has_nested_arrays(item_type) { let movement_register = self.allocate_register(); - let deflattened_array_pointer = self.allocate_register(); let target_item_size = item_type.len(); let source_item_size = Self::flattened_tuple_size(item_type); - self.codegen_allocate_fixed_length_array( - deflattened_array_pointer, - item_count * target_item_size, - ); - for item_index in 0..item_count { let source_item_base_index = item_index * source_item_size; let target_item_base_index = item_index * target_item_size; @@ -221,13 +225,13 @@ impl BrilligContext { match subitem { BrilligParameter::SingleAddr(_) => { - self.codegen_array_get( + self.codegen_load_with_offset( flattened_array_pointer, source_index, movement_register, ); - self.codegen_array_set( - deflattened_array_pointer, + self.codegen_store_with_offset( + deflattened_items_pointer, target_index, movement_register, ); @@ -238,9 +242,8 @@ impl BrilligContext { nested_array_item_count, ) => { let nested_array_pointer = self.allocate_register(); - self.mov_instruction(nested_array_pointer, flattened_array_pointer); self.memory_op_instruction( - nested_array_pointer, + flattened_array_pointer, source_index.address, nested_array_pointer, BrilligBinaryOp::Add, @@ -249,31 +252,16 @@ impl BrilligContext { nested_array_item_type, *nested_array_item_count, nested_array_pointer, + false, ); - let reference = self.allocate_register(); - let rc = self.allocate_register(); - self.usize_const_instruction(rc, 1_usize.into()); - - self.codegen_allocate_array_reference(reference); - let array_variable = BrilligVariable::BrilligArray(BrilligArray { - pointer: deflattened_nested_array_pointer, - size: nested_array_item_type.len() * nested_array_item_count, - rc, - }); - self.codegen_store_variable(reference, array_variable); - - self.codegen_array_set( - deflattened_array_pointer, + self.codegen_store_with_offset( + deflattened_items_pointer, target_index, - reference, + deflattened_nested_array_pointer, ); self.deallocate_register(nested_array_pointer); - self.deallocate_register(reference); - array_variable - .extract_registers() - .into_iter() - .for_each(|register| self.deallocate_register(register)); + self.deallocate_register(deflattened_nested_array_pointer); source_offset += Self::flattened_size(subitem); } @@ -284,15 +272,18 @@ impl BrilligContext { self.deallocate_single_addr(target_index); } } - self.deallocate_register(movement_register); - - deflattened_array_pointer } else { - let deflattened_array_pointer = self.allocate_register(); - self.mov_instruction(deflattened_array_pointer, flattened_array_pointer); - deflattened_array_pointer + self.codegen_mem_copy( + flattened_array_pointer, + deflattened_items_pointer, + deflattened_size_variable, + ); } + + self.deallocate_single_addr(deflattened_size_variable); + self.deallocate_register(deflattened_items_pointer); + deflattened_array_pointer } /// Adds the instructions needed to handle return parameters @@ -319,7 +310,6 @@ impl BrilligContext { BrilligVariable::BrilligArray(BrilligArray { pointer: self.allocate_register(), size: item_types.len() * item_count, - rc: self.allocate_register(), }) } BrilligParameter::Slice(..) => unreachable!("ICE: Cannot return slices"), @@ -344,7 +334,8 @@ impl BrilligContext { return_data_index += 1; } BrilligParameter::Array(item_type, item_count) => { - let returned_pointer = returned_variable.extract_array().pointer; + let deflattened_items_pointer = + self.codegen_make_array_items_pointer(returned_variable.extract_array()); let pointer_to_return_data = self.make_usize_constant_instruction(return_data_index.into()); @@ -352,10 +343,12 @@ impl BrilligContext { item_type, *item_count, pointer_to_return_data.address, - returned_pointer, + deflattened_items_pointer, ); self.deallocate_single_addr(pointer_to_return_data); + self.deallocate_register(deflattened_items_pointer); + return_data_index += Self::flattened_size(return_param); } BrilligParameter::Slice(..) => { @@ -407,9 +400,15 @@ mod tests { let array_pointer = context.allocate_register(); let array_value = context.allocate_register(); - context.load_instruction(array_pointer, array_pointer); - context.load_instruction(array_pointer, array_pointer); - context.load_instruction(array_value, array_pointer); + let items_pointer = context + .codegen_make_array_items_pointer(BrilligArray { pointer: array_pointer, size: 2 }); + + // Load the nested array + context.load_instruction(array_pointer, items_pointer); + let items_pointer = context + .codegen_make_array_items_pointer(BrilligArray { pointer: array_pointer, size: 2 }); + // Load the first item of the nested array. + context.load_instruction(array_value, items_pointer); context.codegen_return(&[array_value]); @@ -443,13 +442,9 @@ mod tests { let mut context = create_context(FunctionId::test_new(0)); // Allocate the parameter - let brillig_array = BrilligArray { - pointer: context.allocate_register(), - size: 2, - rc: context.allocate_register(), - }; + let brillig_array = BrilligArray { pointer: context.allocate_register(), size: 2 }; - context.codegen_return(&brillig_array.extract_registers()); + context.codegen_return(&[brillig_array.pointer]); let bytecode = create_entry_point_bytecode(context, arguments, returns).byte_code; let (vm, return_data_pointer, return_data_size) = diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs index fab43041e65..faeade7fcf1 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs @@ -13,7 +13,7 @@ use crate::ssa::ir::function::FunctionId; use super::{ artifact::{Label, UnresolvedJumpLocation}, - brillig_variable::{BrilligArray, BrilligVector, SingleAddrVariable}, + brillig_variable::SingleAddrVariable, debug_show::DebugToString, procedures::ProcedureId, registers::RegisterAllocator, @@ -309,12 +309,6 @@ impl BrilligContext< self.push_opcode(BrilligOpcode::Store { destination_pointer, source }); } - /// Utility method to transform a HeapArray to a HeapVector by making a runtime constant with the size. - pub(crate) fn array_to_vector_instruction(&mut self, array: &BrilligArray) -> BrilligVector { - let size_register = self.make_usize_constant_instruction(array.size.into()); - BrilligVector { size: size_register.address, pointer: array.pointer, rc: array.rc } - } - /// Emits a load instruction pub(crate) fn load_instruction( &mut self, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/array_copy.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/array_copy.rs index 0ea51603991..5b97bbc8f7a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/array_copy.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/array_copy.rs @@ -4,94 +4,70 @@ use acvm::{acir::brillig::MemoryAddress, AcirField}; use super::ProcedureId; use crate::brillig::brillig_ir::{ - brillig_variable::{BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable}, + brillig_variable::{BrilligArray, SingleAddrVariable}, debug_show::DebugToString, registers::{RegisterAllocator, ScratchSpace}, - BrilligBinaryOp, BrilligContext, + BrilligBinaryOp, BrilligContext, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, }; impl BrilligContext { - /// Conditionally copies a source array/vector to a destination array/vector. - /// If the reference count of the source array/vector is 1, then we can directly copy the pointer of the source array/vector to the destination array/vector. + /// Conditionally copies a source array to a destination array. + /// If the reference count of the source array is 1, then we can directly copy the pointer of the source array to the destination array. pub(crate) fn call_array_copy_procedure( &mut self, - source_variable: BrilligVariable, - destination_variable: BrilligVariable, + source_array: BrilligArray, + destination_array: BrilligArray, ) { - // Args - let source_pointer = MemoryAddress::from(ScratchSpace::start()); - let size_register = MemoryAddress::from(ScratchSpace::start() + 1); - let is_rc_one_register = MemoryAddress::from(ScratchSpace::start() + 2); + let source_array_pointer_arg = MemoryAddress::from(ScratchSpace::start()); + let source_array_memory_size_arg = MemoryAddress::from(ScratchSpace::start() + 1); + let new_array_pointer_return = MemoryAddress::from(ScratchSpace::start() + 2); - // Returns - let destination_pointer = MemoryAddress::from(ScratchSpace::start() + 3); - - match source_variable { - BrilligVariable::BrilligArray(BrilligArray { pointer, rc, .. }) - | BrilligVariable::BrilligVector(BrilligVector { pointer, rc, .. }) => { - self.mov_instruction(source_pointer, pointer); - self.codegen_usize_op(rc, is_rc_one_register, BrilligBinaryOp::Equals, 1_usize); - } - _ => unreachable!("ICE: array_copy on non-array"), - } - - match source_variable { - BrilligVariable::BrilligArray(BrilligArray { size, .. }) => { - self.usize_const_instruction(size_register, size.into()); - } - BrilligVariable::BrilligVector(BrilligVector { size, .. }) => { - self.mov_instruction(size_register, size); - } - _ => unreachable!("ICE: array_copy on non-array"), - } + self.mov_instruction(source_array_pointer_arg, source_array.pointer); + self.usize_const_instruction(source_array_memory_size_arg, (source_array.size + 1).into()); self.add_procedure_call_instruction(ProcedureId::ArrayCopy); - match destination_variable { - BrilligVariable::BrilligArray(BrilligArray { pointer, rc, .. }) - | BrilligVariable::BrilligVector(BrilligVector { pointer, rc, .. }) => { - self.mov_instruction(pointer, destination_pointer); - self.usize_const_instruction(rc, 1_usize.into()); - } - _ => unreachable!("ICE: array_copy on non-array"), - } - - if let BrilligVariable::BrilligVector(BrilligVector { size, .. }) = destination_variable { - self.mov_instruction(size, size_register); - } + self.mov_instruction(destination_array.pointer, new_array_pointer_return); } } pub(super) fn compile_array_copy_procedure( brillig_context: &mut BrilligContext, ) { - // Args - let source_pointer = MemoryAddress::from(ScratchSpace::start()); - let size_register = MemoryAddress::from(ScratchSpace::start() + 1); - let is_rc_one_register = MemoryAddress::from(ScratchSpace::start() + 2); - - // Returns - let destination_pointer = MemoryAddress::from(ScratchSpace::start() + 3); + let source_array_pointer_arg = MemoryAddress::from(ScratchSpace::start()); + let source_array_memory_size_arg = MemoryAddress::from(ScratchSpace::start() + 1); + let new_array_pointer_return = MemoryAddress::from(ScratchSpace::start() + 2); brillig_context.set_allocated_registers(vec![ - source_pointer, - destination_pointer, - size_register, - is_rc_one_register, + source_array_pointer_arg, + source_array_memory_size_arg, + new_array_pointer_return, ]); - brillig_context.codegen_branch(is_rc_one_register, |ctx, cond| { + let rc = SingleAddrVariable::new_usize(brillig_context.allocate_register()); + brillig_context.load_instruction(rc.address, source_array_pointer_arg); + + let is_rc_one = SingleAddrVariable::new(brillig_context.allocate_register(), 1); + brillig_context.codegen_usize_op(rc.address, is_rc_one.address, BrilligBinaryOp::Equals, 1); + + brillig_context.codegen_branch(is_rc_one.address, |ctx, cond| { if cond { // Reference count is 1, we can mutate the array directly - ctx.mov_instruction(destination_pointer, source_pointer); + ctx.mov_instruction(new_array_pointer_return, source_array_pointer_arg); } else { // First issue a array copy to the destination - ctx.codegen_allocate_array(destination_pointer, size_register); + ctx.codegen_allocate_mem(new_array_pointer_return, source_array_memory_size_arg); ctx.codegen_mem_copy( - source_pointer, - destination_pointer, - SingleAddrVariable::new_usize(size_register), + source_array_pointer_arg, + new_array_pointer_return, + SingleAddrVariable::new_usize(source_array_memory_size_arg), + ); + // Then set the new rc to 1 + ctx.indirect_const_instruction( + new_array_pointer_return, + BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, + 1_usize.into(), ); } }); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs index 92857823945..d2a011f8aa5 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs @@ -1,10 +1,12 @@ mod array_copy; mod array_reverse; mod mem_copy; +mod vector_copy; use array_copy::compile_array_copy_procedure; use array_reverse::compile_array_reverse_procedure; use mem_copy::compile_mem_copy_procedure; +use vector_copy::compile_vector_copy_procedure; use crate::brillig::brillig_ir::AcirField; @@ -21,6 +23,7 @@ use super::{ pub(crate) enum ProcedureId { ArrayCopy, ArrayReverse, + VectorCopy, MemCopy, } @@ -34,6 +37,7 @@ pub(crate) fn compile_procedure( ProcedureId::MemCopy => compile_mem_copy_procedure(&mut brillig_context), ProcedureId::ArrayCopy => compile_array_copy_procedure(&mut brillig_context), ProcedureId::ArrayReverse => compile_array_reverse_procedure(&mut brillig_context), + ProcedureId::VectorCopy => compile_vector_copy_procedure(&mut brillig_context), }; brillig_context.stop_instruction(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/vector_copy.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/vector_copy.rs new file mode 100644 index 00000000000..87895a975f8 --- /dev/null +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/vector_copy.rs @@ -0,0 +1,70 @@ +use std::vec; + +use acvm::{acir::brillig::MemoryAddress, AcirField}; + +use super::ProcedureId; +use crate::brillig::brillig_ir::{ + brillig_variable::{BrilligVector, SingleAddrVariable}, + debug_show::DebugToString, + registers::{RegisterAllocator, ScratchSpace}, + BrilligBinaryOp, BrilligContext, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, +}; + +impl BrilligContext { + /// Conditionally copies a source array to a destination array. + /// If the reference count of the source array is 1, then we can directly copy the pointer of the source array to the destination array. + pub(crate) fn call_vector_copy_procedure( + &mut self, + source_vector: BrilligVector, + destination_vector: BrilligVector, + ) { + let source_vector_pointer_arg = MemoryAddress::from(ScratchSpace::start()); + let new_vector_pointer_return = MemoryAddress::from(ScratchSpace::start() + 1); + + self.mov_instruction(source_vector_pointer_arg, source_vector.pointer); + + self.add_procedure_call_instruction(ProcedureId::VectorCopy); + + self.mov_instruction(destination_vector.pointer, new_vector_pointer_return); + } +} + +pub(super) fn compile_vector_copy_procedure( + brillig_context: &mut BrilligContext, +) { + let source_vector_pointer_arg = MemoryAddress::from(ScratchSpace::start()); + let new_vector_pointer_return = MemoryAddress::from(ScratchSpace::start() + 1); + + brillig_context + .set_allocated_registers(vec![source_vector_pointer_arg, new_vector_pointer_return]); + + let rc = SingleAddrVariable::new_usize(brillig_context.allocate_register()); + brillig_context.load_instruction(rc.address, source_vector_pointer_arg); + + let is_rc_one = SingleAddrVariable::new(brillig_context.allocate_register(), 1); + brillig_context.codegen_usize_op(rc.address, is_rc_one.address, BrilligBinaryOp::Equals, 1); + + brillig_context.codegen_branch(is_rc_one.address, |ctx, cond| { + if cond { + // Reference count is 1, we can mutate the array directly + ctx.mov_instruction(new_vector_pointer_return, source_vector_pointer_arg); + } else { + let source_vector = BrilligVector { pointer: source_vector_pointer_arg }; + let result_vector = BrilligVector { pointer: new_vector_pointer_return }; + + // Allocate the memory for the new vec + let allocation_size = ctx.codegen_make_vector_length(source_vector); + ctx.codegen_usize_op_in_place(allocation_size.address, BrilligBinaryOp::Add, 2_usize); + ctx.codegen_allocate_mem(result_vector.pointer, allocation_size.address); + + ctx.codegen_mem_copy(source_vector.pointer, result_vector.pointer, allocation_size); + // Then set the new rc to 1 + ctx.indirect_const_instruction( + result_vector.pointer, + BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, + 1_usize.into(), + ); + ctx.deallocate_single_addr(allocation_size); + } + }); +} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs index 43f85c159d1..75fb60fc9f2 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs @@ -1,4 +1,4 @@ -use acvm::acir::brillig::MemoryAddress; +use acvm::acir::brillig::{HeapArray, HeapVector, MemoryAddress}; use crate::brillig::brillig_ir::entry_point::MAX_STACK_SIZE; @@ -222,4 +222,13 @@ impl BrilligContext { pub(crate) fn deallocate_single_addr(&mut self, var: SingleAddrVariable) { self.deallocate_register(var.address); } + + pub(crate) fn deallocate_heap_array(&mut self, arr: HeapArray) { + self.deallocate_register(arr.pointer); + } + + pub(crate) fn deallocate_heap_vector(&mut self, vec: HeapVector) { + self.deallocate_register(vec.pointer); + self.deallocate_register(vec.size); + } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 15b44fde65d..5091854a2ed 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -960,6 +960,7 @@ impl<'a> Context<'a> { arguments, BrilligFunctionContext::return_values(func), func.id(), + false, ); entry_point.name = func.name().to_string(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 0edb7daf530..04d4e893bf8 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -420,25 +420,20 @@ impl FunctionBuilder { /// within the given value. If the given value is not an array and does not contain /// any arrays, this does nothing. pub(crate) fn increment_array_reference_count(&mut self, value: ValueId) { - self.update_array_reference_count(value, true, None); + self.update_array_reference_count(value, true); } /// Insert instructions to decrement the reference count of any array(s) stored /// within the given value. If the given value is not an array and does not contain /// any arrays, this does nothing. pub(crate) fn decrement_array_reference_count(&mut self, value: ValueId) { - self.update_array_reference_count(value, false, None); + self.update_array_reference_count(value, false); } /// Increment or decrement the given value's reference count if it is an array. /// If it is not an array, this does nothing. Note that inc_rc and dec_rc instructions /// are ignored outside of unconstrained code. - fn update_array_reference_count( - &mut self, - value: ValueId, - increment: bool, - load_address: Option, - ) { + fn update_array_reference_count(&mut self, value: ValueId, increment: bool) { match self.type_of_value(value) { Type::Numeric(_) => (), Type::Function => (), @@ -446,40 +441,16 @@ impl FunctionBuilder { if element.contains_an_array() { let reference = value; let value = self.insert_load(reference, element.as_ref().clone()); - self.update_array_reference_count(value, increment, Some(reference)); + self.update_array_reference_count(value, increment); } } - typ @ Type::Array(..) | typ @ Type::Slice(..) => { + Type::Array(..) | Type::Slice(..) => { // If there are nested arrays or slices, we wait until ArrayGet // is issued to increment the count of that array. - let update_rc = |this: &mut Self, value| { - if increment { - this.insert_inc_rc(value); - } else { - this.insert_dec_rc(value); - } - }; - - update_rc(self, value); - let dfg = &self.current_function.dfg; - - // This is a bit odd, but in brillig the inc_rc instruction operates on - // a copy of the array's metadata, so we need to re-store a loaded array - // even if there have been no other changes to it. - if let Some(address) = load_address { - // If we already have a load from the Type::Reference case, avoid inserting - // another load and rc update. - self.insert_store(address, value); - } else if let Value::Instruction { instruction, .. } = &dfg[value] { - let instruction = &dfg[*instruction]; - if let Instruction::Load { address } = instruction { - // We can't re-use `value` in case the original address was stored - // to again in the meantime. So introduce another load. - let address = *address; - let new_load = self.insert_load(address, typ); - update_rc(self, new_load); - self.insert_store(address, new_load); - } + if increment { + self.insert_inc_rc(value); + } else { + self.insert_dec_rc(value); } } }