From 3ad624cc343834e8c078c3fe99f3add08f22ffe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Wed, 11 Sep 2024 11:14:04 +0200 Subject: [PATCH] feat: Change the layout of arrays and vectors to be a single pointer (#8448) Previously, we used to store arrays as two stack elements (rc, items_pointer) and vectors as three (rc, size, items_pointer). Now we move away from that and we just represent them as one pointer to (rc, ...items) in the case of arrays and (rc, size, ...items) in the case of vectors. This makes moving around arrays and vectors cheaper and avoids needing to re-store arrays and vectors in references after modifying the reference counters.
Changes in bytecode size ``` contracts: Transpiling AppSubscription::constructor with size 1123 => 968 contracts: Transpiling Auth::constructor with size 683 => 556 contracts: Transpiling Auth::get_authorized with size 132 => 128 contracts: Transpiling Auth::get_authorized_delay with size 181 => 175 contracts: Transpiling Auth::get_scheduled_authorized with size 113 => 107 contracts: Transpiling Auth::set_authorized with size 980 => 849 contracts: Transpiling Auth::set_authorized_delay with size 938 => 818 contracts: Transpiling AuthRegistry::_set_authorized with size 80 => 86 contracts: Transpiling AuthRegistry::consume with size 634 => 514 contracts: Transpiling AuthRegistry::is_consumable with size 127 => 123 contracts: Transpiling AuthRegistry::is_reject_all with size 112 => 106 contracts: Transpiling AuthRegistry::set_authorized with size 75 => 81 contracts: Transpiling AuthRegistry::set_reject_all with size 60 => 64 contracts: Transpiling AuthWitTest::consume_public with size 66 => 89 contracts: Transpiling AvmInitializerTest::constructor with size 679 => 552 contracts: Transpiling AvmInitializerTest::read_storage_immutable with size 97 => 89 contracts: Transpiling AvmTest::add_args_return with size 17 => 17 contracts: Transpiling AvmTest::add_storage_map with size 174 => 174 contracts: Transpiling AvmTest::add_u128 with size 34 => 34 contracts: Transpiling AvmTest::assert_nullifier_exists with size 18 => 18 contracts: Transpiling AvmTest::assert_same with size 20 => 20 contracts: Transpiling AvmTest::assert_timestamp with size 17 => 17 contracts: Transpiling AvmTest::assertion_failure with size 16 => 16 contracts: Transpiling AvmTest::check_selector with size 16 => 16 contracts: Transpiling AvmTest::create_different_nullifier_in_nested_call with size 128 => 127 contracts: Transpiling AvmTest::create_same_nullifier_in_nested_call with size 126 => 125 contracts: Transpiling AvmTest::debug_logging with size 238 => 282 contracts: Transpiling AvmTest::elliptic_curve_add_and_double with size 35 => 35 contracts: Transpiling AvmTest::emit_nullifier_and_check with size 19 => 19 contracts: Transpiling AvmTest::emit_unencrypted_log with size 501 => 524 contracts: Transpiling AvmTest::get_address with size 15 => 15 contracts: Transpiling AvmTest::get_args_hash with size 20 => 40 contracts: Transpiling AvmTest::get_block_number with size 15 => 15 contracts: Transpiling AvmTest::get_chain_id with size 15 => 15 contracts: Transpiling AvmTest::get_da_gas_left with size 15 => 15 contracts: Transpiling AvmTest::get_fee_per_da_gas with size 15 => 15 contracts: Transpiling AvmTest::get_fee_per_l2_gas with size 15 => 15 contracts: Transpiling AvmTest::get_function_selector with size 15 => 15 contracts: Transpiling AvmTest::get_l2_gas_left with size 15 => 15 contracts: Transpiling AvmTest::get_sender with size 15 => 15 contracts: Transpiling AvmTest::get_storage_address with size 15 => 15 contracts: Transpiling AvmTest::get_timestamp with size 15 => 15 contracts: Transpiling AvmTest::get_transaction_fee with size 15 => 15 contracts: Transpiling AvmTest::get_version with size 15 => 15 contracts: Transpiling AvmTest::keccak_f1600 with size 63 => 77 contracts: Transpiling AvmTest::keccak_hash with size 783 => 708 contracts: Transpiling AvmTest::l1_to_l2_msg_exists with size 19 => 19 contracts: Transpiling AvmTest::modulo2 with size 18 => 18 contracts: Transpiling AvmTest::nested_call_to_add with size 204 => 202 contracts: Transpiling AvmTest::nested_call_to_add_with_gas with size 204 => 204 contracts: Transpiling AvmTest::nested_static_call_to_add with size 204 => 202 contracts: Transpiling AvmTest::nested_static_call_to_set_storage with size 125 => 124 contracts: Transpiling AvmTest::new_note_hash with size 13 => 13 contracts: Transpiling AvmTest::new_nullifier with size 13 => 13 contracts: Transpiling AvmTest::note_hash_exists with size 19 => 19 contracts: Transpiling AvmTest::nullifier_collision with size 14 => 14 contracts: Transpiling AvmTest::nullifier_exists with size 19 => 19 contracts: Transpiling AvmTest::pedersen_commit with size 64 => 73 contracts: Transpiling AvmTest::pedersen_hash with size 21 => 42 contracts: Transpiling AvmTest::pedersen_hash_with_index with size 21 => 42 contracts: Transpiling AvmTest::poseidon2_hash with size 376 => 306 contracts: Transpiling AvmTest::read_storage_list with size 117 => 115 contracts: Transpiling AvmTest::read_storage_map with size 109 => 103 contracts: Transpiling AvmTest::read_storage_single with size 90 => 82 contracts: Transpiling AvmTest::send_l2_to_l1_msg with size 14 => 14 contracts: Transpiling AvmTest::set_opcode_big_field with size 21 => 21 contracts: Transpiling AvmTest::set_opcode_small_field with size 15 => 15 contracts: Transpiling AvmTest::set_opcode_u32 with size 15 => 15 contracts: Transpiling AvmTest::set_opcode_u64 with size 15 => 15 contracts: Transpiling AvmTest::set_opcode_u8 with size 15 => 15 contracts: Transpiling AvmTest::set_read_storage_single with size 125 => 119 contracts: Transpiling AvmTest::set_storage_list with size 45 => 47 contracts: Transpiling AvmTest::set_storage_map with size 73 => 79 contracts: Transpiling AvmTest::set_storage_single with size 41 => 43 contracts: Transpiling AvmTest::sha256_hash with size 48 => 62 contracts: Transpiling AvmTest::test_get_contract_instance with size 185 => 178 contracts: Transpiling AvmTest::test_get_contract_instance_raw with size 60 => 69 contracts: Transpiling AvmTest::to_radix_le with size 35 => 40 contracts: Transpiling AvmTest::u128_addition_overflow with size 272 => 275 contracts: Transpiling AvmTest::u128_from_integer_overflow with size 138 => 140 contracts: Transpiling AvmTest::variable_base_msm with size 80 => 90 contracts: Transpiling Benchmarking::broadcast with size 110 => 113 contracts: Transpiling Benchmarking::increment_balance with size 264 => 261 contracts: Transpiling CardGame::on_card_played with size 872 => 882 contracts: Transpiling CardGame::on_cards_claimed with size 709 => 745 contracts: Transpiling CardGame::on_game_joined with size 655 => 676 contracts: Transpiling CardGame::start_game with size 1149 => 1150 contracts: Transpiling Child::pub_get_value with size 24 => 24 contracts: Transpiling Child::pub_inc_value with size 133 => 136 contracts: Transpiling Child::pub_inc_value_internal with size 138 => 141 contracts: Transpiling Child::pub_set_value with size 51 => 62 contracts: Transpiling Child::set_value_twice_with_nested_first with size 168 => 178 contracts: Transpiling Child::set_value_twice_with_nested_last with size 168 => 178 contracts: Transpiling Child::set_value_with_two_nested_calls with size 100 => 135 contracts: Transpiling Claim::constructor with size 790 => 656 contracts: Transpiling Crowdfunding::_publish_donation_receipts with size 136 => 153 contracts: Transpiling Crowdfunding::init with size 903 => 762 contracts: Transpiling DelegatedOn::public_set_value with size 44 => 46 contracts: Transpiling Delegator::public_delegate_set_value with size 97 => 94 contracts: Transpiling DocsExample::get_shared_immutable_constrained_public with size 104 => 97 contracts: Transpiling DocsExample::get_shared_immutable_constrained_public_indirect with size 66 => 92 contracts: Transpiling DocsExample::get_shared_immutable_constrained_public_multiple with size 136 => 134 contracts: Transpiling DocsExample::initialize_public_immutable with size 159 => 155 contracts: Transpiling DocsExample::initialize_shared_immutable with size 159 => 155 contracts: Transpiling DocsExample::spend_public_authwit with size 16 => 16 contracts: Transpiling DocsExample::update_leader with size 52 => 54 contracts: Transpiling EasyPrivateVoting::add_to_tally_public with size 225 => 212 contracts: Transpiling EasyPrivateVoting::constructor with size 736 => 613 contracts: Transpiling EasyPrivateVoting::end_vote with size 133 => 127 contracts: Transpiling FeeJuice::_increase_public_balance with size 190 => 188 contracts: Transpiling FeeJuice::balance_of_public with size 121 => 115 contracts: Transpiling FeeJuice::check_balance with size 174 => 168 contracts: Transpiling FeeJuice::claim_public with size 1553 => 1331 contracts: Transpiling FeeJuice::set_portal with size 220 => 205 contracts: Transpiling FPC::constructor with size 679 => 552 contracts: Transpiling FPC::pay_refund with size 420 => 385 contracts: Transpiling FPC::pay_refund_with_shielded_rebate with size 420 => 385 contracts: Transpiling FPC::prepare_fee with size 340 => 297 contracts: Transpiling ImportTest::pub_call_public_fn with size 125 => 124 contracts: Transpiling InclusionProofs::constructor with size 595 => 477 contracts: Transpiling InclusionProofs::push_nullifier_public with size 20 => 20 contracts: Transpiling InclusionProofs::test_nullifier_inclusion_from_public with size 24 => 24 contracts: Transpiling KeyRegistry::register_initial_keys with size 1405 => 1058 contracts: Transpiling KeyRegistry::rotate_npk_m with size 1190 => 965 contracts: Transpiling Lending::_borrow with size 2269 => 2274 contracts: Transpiling Lending::_deposit with size 230 => 227 contracts: Transpiling Lending::_repay with size 1351 => 1364 contracts: Transpiling Lending::_withdraw with size 2322 => 2297 contracts: Transpiling Lending::borrow_public with size 267 => 238 contracts: Transpiling Lending::deposit_public with size 569 => 497 contracts: Transpiling Lending::get_asset with size 159 => 158 contracts: Transpiling Lending::get_assets with size 184 => 170 contracts: Transpiling Lending::get_position with size 872 => 848 contracts: Transpiling Lending::init with size 290 => 292 contracts: Transpiling Lending::repay_public with size 501 => 443 contracts: Transpiling Lending::update_accumulator with size 2414 => 2415 contracts: Transpiling Lending::withdraw_public with size 267 => 238 contracts: Transpiling Parent::pub_entry_point with size 58 => 81 contracts: Transpiling Parent::pub_entry_point_twice with size 87 => 133 contracts: Transpiling Parent::public_nested_static_call with size 1016 => 966 contracts: Transpiling Parent::public_static_call with size 82 => 108 contracts: Transpiling PriceFeed::get_price with size 117 => 112 contracts: Transpiling PriceFeed::set_price with size 71 => 75 contracts: Transpiling PrivateFPC::constructor with size 682 => 555 contracts: Transpiling Router::_check_block_number with size 155 => 163 contracts: Transpiling Router::_check_timestamp with size 158 => 166 contracts: Transpiling StatefulTest::get_public_value with size 109 => 103 contracts: Transpiling StatefulTest::increment_public_value with size 138 => 134 contracts: Transpiling StatefulTest::increment_public_value_no_init_check with size 131 => 127 contracts: Transpiling StatefulTest::public_constructor with size 748 => 613 contracts: Transpiling StaticChild::pub_get_value with size 27 => 27 contracts: Transpiling StaticChild::pub_illegal_inc_value with size 136 => 139 contracts: Transpiling StaticChild::pub_inc_value with size 133 => 136 contracts: Transpiling StaticChild::pub_set_value with size 51 => 62 contracts: Transpiling StaticParent::public_call with size 58 => 81 contracts: Transpiling StaticParent::public_get_value_from_child with size 136 => 148 contracts: Transpiling StaticParent::public_nested_static_call with size 279 => 272 contracts: Transpiling StaticParent::public_static_call with size 82 => 108 contracts: Transpiling Test::assert_public_global_vars with size 42 => 42 contracts: Transpiling Test::consume_message_from_arbitrary_sender_public with size 1105 => 915 contracts: Transpiling Test::consume_mint_public_message with size 1330 => 1137 contracts: Transpiling Test::create_l2_to_l1_message_arbitrary_recipient_public with size 14 => 14 contracts: Transpiling Test::create_l2_to_l1_message_public with size 26 => 28 contracts: Transpiling Test::dummy_public_call with size 16 => 16 contracts: Transpiling Test::emit_nullifier_public with size 13 => 13 contracts: Transpiling Test::emit_unencrypted with size 219 => 255 contracts: Transpiling Test::is_time_equal with size 20 => 20 contracts: Transpiling TestLog::emit_unencrypted_events with size 223 => 251 contracts: Transpiling Token::_increase_public_balance with size 189 => 187 contracts: Transpiling Token::_reduce_total_supply with size 168 => 162 contracts: Transpiling Token::admin with size 100 => 92 contracts: Transpiling Token::assert_minter_and_mint with size 243 => 228 contracts: Transpiling Token::balance_of_public with size 128 => 122 contracts: Transpiling Token::burn_public with size 729 => 628 contracts: Transpiling Token::complete_refund with size 416 => 458 contracts: Transpiling Token::constructor with size 1132 => 1027 contracts: Transpiling Token::is_minter with size 119 => 113 contracts: Transpiling Token::mint_private with size 508 => 519 contracts: Transpiling Token::mint_public with size 370 => 350 contracts: Transpiling Token::public_get_decimals with size 103 => 95 contracts: Transpiling Token::public_get_name with size 100 => 92 contracts: Transpiling Token::public_get_symbol with size 100 => 92 contracts: Transpiling Token::set_admin with size 133 => 127 contracts: Transpiling Token::set_minter with size 152 => 148 contracts: Transpiling Token::shield with size 896 => 830 contracts: Transpiling Token::total_supply with size 112 => 104 contracts: Transpiling Token::transfer_public with size 748 => 651 contracts: Transpiling TokenBlacklist::_increase_public_balance with size 189 => 187 contracts: Transpiling TokenBlacklist::_reduce_total_supply with size 168 => 162 contracts: Transpiling TokenBlacklist::balance_of_public with size 128 => 122 contracts: Transpiling TokenBlacklist::burn_public with size 858 => 752 contracts: Transpiling TokenBlacklist::constructor with size 1548 => 1307 contracts: Transpiling TokenBlacklist::get_roles with size 181 => 179 contracts: Transpiling TokenBlacklist::mint_private with size 563 => 578 contracts: Transpiling TokenBlacklist::mint_public with size 550 => 529 contracts: Transpiling TokenBlacklist::shield with size 1025 => 954 contracts: Transpiling TokenBlacklist::total_supply with size 112 => 104 contracts: Transpiling TokenBlacklist::transfer_public with size 1009 => 902 contracts: Transpiling TokenBlacklist::update_roles with size 1197 => 1074 contracts: Transpiling TokenBridge::_assert_token_is_same with size 103 => 95 contracts: Transpiling TokenBridge::_call_mint_on_token with size 277 => 254 contracts: Transpiling TokenBridge::claim_public with size 1644 => 1401 contracts: Transpiling TokenBridge::constructor with size 707 => 582 contracts: Transpiling TokenBridge::exit_to_l1_public with size 853 => 813 contracts: Transpiling TokenBridge::get_portal_address_public with size 108 => 100 contracts: Transpiling TokenBridge::get_token with size 100 => 92 contracts: Transpiling Uniswap::_approve_bridge_and_exit_input_asset_to_L1 with size 3337 => 2622 contracts: Transpiling Uniswap::_assert_token_is_same with size 68 => 93 contracts: Transpiling Uniswap::constructor with size 679 => 552 contracts: Transpiling Uniswap::swap_public with size 2114 => 2047 ```
--- .../noir-repo/acvm-repo/brillig_vm/src/lib.rs | 43 +- .../brillig/brillig_gen/brillig_black_box.rs | 306 ++++++++----- .../src/brillig/brillig_gen/brillig_block.rs | 409 +++++++++--------- .../brillig_gen/brillig_block_variables.rs | 33 +- .../brillig/brillig_gen/brillig_slice_ops.rs | 340 ++++++++------- .../noirc_evaluator/src/brillig/brillig_ir.rs | 8 +- .../brillig/brillig_ir/brillig_variable.rs | 53 +-- .../src/brillig/brillig_ir/codegen_calls.rs | 3 +- .../brillig_ir/codegen_control_flow.rs | 62 ++- .../brillig/brillig_ir/codegen_intrinsic.rs | 14 +- .../src/brillig/brillig_ir/codegen_memory.rs | 371 ++++++++-------- .../src/brillig/brillig_ir/entry_point.rs | 129 +++--- .../src/brillig/brillig_ir/instructions.rs | 8 +- .../brillig_ir/procedures/array_copy.rs | 96 ++-- .../src/brillig/brillig_ir/procedures/mod.rs | 4 + .../brillig_ir/procedures/vector_copy.rs | 70 +++ .../src/brillig/brillig_ir/registers.rs | 11 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 1 + .../src/ssa/function_builder/mod.rs | 47 +- 19 files changed, 1018 insertions(+), 990 deletions(-) create mode 100644 noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/vector_copy.rs 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); } } }