From adf31cdfe22b8271e9c01838211d5d6572adf4e3 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 19 Aug 2024 11:26:04 +0000 Subject: [PATCH 1/4] feat: added indirect const --- avm-transpiler/src/transpile.rs | 44 +++++++++--- .../dsl/acir_format/serde/acir.hpp | 68 +++++++++++++++++++ .../noir-repo/acvm-repo/acir/codegen/acir.cpp | 56 ++++++++++++++- .../acir/tests/test_program_serialization.rs | 28 ++++---- .../test/shared/complex_foreign_call.ts | 14 ++-- .../acvm_js/test/shared/foreign_call.ts | 8 +-- .../acvm-repo/brillig/src/opcodes.rs | 5 ++ .../noir-repo/acvm-repo/brillig_vm/src/lib.rs | 7 ++ .../src/brillig/brillig_gen/brillig_block.rs | 17 +++-- .../brillig_ir/codegen_control_flow.rs | 8 ++- .../src/brillig/brillig_ir/debug_show.rs | 9 +++ .../src/brillig/brillig_ir/instructions.rs | 56 +++++++++++---- .../tooling/profiler/src/opcode_formatter.rs | 1 + 13 files changed, 265 insertions(+), 56 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 437933ce294..e3805bc8ec7 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -119,7 +119,10 @@ pub fn brillig_to_avm( }); } BrilligOpcode::Const { destination, value, bit_size } => { - handle_const(&mut avm_instrs, destination, value, bit_size); + handle_const(&mut avm_instrs, destination, value, bit_size, false); + } + BrilligOpcode::IndirectConst { destination_pointer, value, bit_size } => { + handle_const(&mut avm_instrs, destination_pointer, value, bit_size, true); } BrilligOpcode::Mov { destination, source } => { avm_instrs.push(generate_mov_instruction( @@ -371,7 +374,7 @@ fn handle_cast( ); avm_instrs.extend([ // We cast to Field to be able to use toradix. - generate_cast_instruction(source_offset, dest_offset, AvmTypeTag::FIELD), + generate_cast_instruction(source_offset, false, dest_offset, false, AvmTypeTag::FIELD), // Toradix with radix 2 and 1 limb is the same as modulo 2. // We need to insert an instruction explicitly because we want to fine-tune 'indirect'. AvmInstruction { @@ -386,11 +389,11 @@ fn handle_cast( ], }, // Then we cast back to u8 (which is what we use for u1). - generate_cast_instruction(dest_offset, dest_offset, AvmTypeTag::UINT8), + generate_cast_instruction(dest_offset, false, dest_offset, false, AvmTypeTag::UINT8), ]); } else { let tag = tag_from_bit_size(bit_size); - avm_instrs.push(generate_cast_instruction(source_offset, dest_offset, tag)); + avm_instrs.push(generate_cast_instruction(source_offset, false, dest_offset, false, tag)); } } @@ -667,12 +670,13 @@ fn handle_const( destination: &MemoryAddress, value: &FieldElement, bit_size: &BitSize, + indirect: bool, ) { let tag = tag_from_bit_size(*bit_size); let dest = destination.to_usize() as u32; if !matches!(tag, AvmTypeTag::FIELD) { - avm_instrs.push(generate_set_instruction(tag, dest, value.to_u128())); + avm_instrs.push(generate_set_instruction(tag, dest, value.to_u128(), indirect)); } else { // We can't fit a field in an instruction. This should've been handled in Brillig. let field = value; @@ -680,17 +684,22 @@ fn handle_const( panic!("SET: Field value doesn't fit in 128 bits, that's not supported!"); } avm_instrs.extend([ - generate_set_instruction(AvmTypeTag::UINT128, dest, field.to_u128()), - generate_cast_instruction(dest, dest, AvmTypeTag::FIELD), + generate_set_instruction(AvmTypeTag::UINT128, dest, field.to_u128(), indirect), + generate_cast_instruction(dest, indirect, dest, indirect, AvmTypeTag::FIELD), ]); } } /// Generates an AVM SET instruction. -fn generate_set_instruction(tag: AvmTypeTag, dest: u32, value: u128) -> AvmInstruction { +fn generate_set_instruction( + tag: AvmTypeTag, + dest: u32, + value: u128, + indirect: bool, +) -> AvmInstruction { AvmInstruction { opcode: AvmOpcode::SET, - indirect: Some(ALL_DIRECT), + indirect: if indirect { Some(FIRST_OPERAND_INDIRECT) } else { Some(ALL_DIRECT) }, tag: Some(tag), operands: vec![ // const @@ -709,10 +718,23 @@ fn generate_set_instruction(tag: AvmTypeTag, dest: u32, value: u128) -> AvmInstr } /// Generates an AVM CAST instruction. -fn generate_cast_instruction(source: u32, destination: u32, dst_tag: AvmTypeTag) -> AvmInstruction { +fn generate_cast_instruction( + source: u32, + source_indirect: bool, + destination: u32, + destination_indirect: bool, + dst_tag: AvmTypeTag, +) -> AvmInstruction { + let mut indirect_flags = ALL_DIRECT; + if source_indirect { + indirect_flags |= ZEROTH_OPERAND_INDIRECT; + } + if destination_indirect { + indirect_flags |= FIRST_OPERAND_INDIRECT; + } AvmInstruction { opcode: AvmOpcode::CAST, - indirect: Some(ALL_DIRECT), + indirect: Some(indirect_flags), tag: Some(dst_tag), operands: vec![AvmOperand::U32 { value: source }, AvmOperand::U32 { value: destination }], } diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp index dc4a52628c1..aa19db50939 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -657,6 +657,16 @@ struct BrilligOpcode { static Const bincodeDeserialize(std::vector); }; + struct IndirectConst { + Program::MemoryAddress destination_pointer; + Program::BitSize bit_size; + std::string value; + + friend bool operator==(const IndirectConst&, const IndirectConst&); + std::vector bincodeSerialize() const; + static IndirectConst bincodeDeserialize(std::vector); + }; + struct Return { friend bool operator==(const Return&, const Return&); std::vector bincodeSerialize() const; @@ -747,6 +757,7 @@ struct BrilligOpcode { CalldataCopy, Call, Const, + IndirectConst, Return, ForeignCall, Mov, @@ -6459,6 +6470,63 @@ Program::BrilligOpcode::Const serde::Deserializable BrilligOpcode::IndirectConst::bincodeSerialize() const +{ + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); +} + +inline BrilligOpcode::IndirectConst BrilligOpcode::IndirectConst::bincodeDeserialize(std::vector input) +{ + auto deserializer = serde::BincodeDeserializer(input); + auto value = serde::Deserializable::deserialize(deserializer); + if (deserializer.get_buffer_offset() < input.size()) { + throw_or_abort("Some input bytes were not read"); + } + return value; +} + +} // end of namespace Program + +template <> +template +void serde::Serializable::serialize( + const Program::BrilligOpcode::IndirectConst& obj, Serializer& serializer) +{ + serde::Serializable::serialize(obj.destination_pointer, serializer); + serde::Serializable::serialize(obj.bit_size, serializer); + serde::Serializable::serialize(obj.value, serializer); +} + +template <> +template +Program::BrilligOpcode::IndirectConst serde::Deserializable::deserialize( + Deserializer& deserializer) +{ + Program::BrilligOpcode::IndirectConst obj; + obj.destination_pointer = serde::Deserializable::deserialize(deserializer); + obj.bit_size = serde::Deserializable::deserialize(deserializer); + obj.value = serde::Deserializable::deserialize(deserializer); + return obj; +} + +namespace Program { + inline bool operator==(const BrilligOpcode::Return& lhs, const BrilligOpcode::Return& rhs) { return true; diff --git a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp index 2f883502869..89fb0a66be8 100644 --- a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp +++ b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp @@ -635,6 +635,16 @@ namespace Program { static Const bincodeDeserialize(std::vector); }; + struct IndirectConst { + Program::MemoryAddress destination_pointer; + Program::BitSize bit_size; + std::string value; + + friend bool operator==(const IndirectConst&, const IndirectConst&); + std::vector bincodeSerialize() const; + static IndirectConst bincodeDeserialize(std::vector); + }; + struct Return { friend bool operator==(const Return&, const Return&); std::vector bincodeSerialize() const; @@ -716,7 +726,7 @@ namespace Program { static Stop bincodeDeserialize(std::vector); }; - std::variant value; + std::variant value; friend bool operator==(const BrilligOpcode&, const BrilligOpcode&); std::vector bincodeSerialize() const; @@ -5386,6 +5396,50 @@ Program::BrilligOpcode::Const serde::Deserializable BrilligOpcode::IndirectConst::bincodeSerialize() const { + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); + } + + inline BrilligOpcode::IndirectConst BrilligOpcode::IndirectConst::bincodeDeserialize(std::vector input) { + auto deserializer = serde::BincodeDeserializer(input); + auto value = serde::Deserializable::deserialize(deserializer); + if (deserializer.get_buffer_offset() < input.size()) { + throw serde::deserialization_error("Some input bytes were not read"); + } + return value; + } + +} // end of namespace Program + +template <> +template +void serde::Serializable::serialize(const Program::BrilligOpcode::IndirectConst &obj, Serializer &serializer) { + serde::Serializable::serialize(obj.destination_pointer, serializer); + serde::Serializable::serialize(obj.bit_size, serializer); + serde::Serializable::serialize(obj.value, serializer); +} + +template <> +template +Program::BrilligOpcode::IndirectConst serde::Deserializable::deserialize(Deserializer &deserializer) { + Program::BrilligOpcode::IndirectConst obj; + obj.destination_pointer = serde::Deserializable::deserialize(deserializer); + obj.bit_size = serde::Deserializable::deserialize(deserializer); + obj.value = serde::Deserializable::deserialize(deserializer); + return obj; +} + namespace Program { inline bool operator==(const BrilligOpcode::Return &lhs, const BrilligOpcode::Return &rhs) { diff --git a/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs b/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs index 1a634eeea9c..cf0964c78f1 100644 --- a/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs +++ b/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs @@ -204,11 +204,11 @@ fn simple_brillig_foreign_call() { let bytes = Program::serialize_program(&program); let expected_serialization: Vec = vec![ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 80, 49, 10, 192, 32, 12, 52, 45, 45, 165, 155, 63, - 209, 31, 248, 25, 7, 23, 7, 17, 223, 175, 96, 2, 65, 162, 139, 30, 132, 203, 221, 65, 72, - 2, 170, 227, 107, 5, 216, 63, 200, 164, 57, 200, 115, 200, 102, 15, 22, 206, 205, 50, 124, - 223, 107, 108, 128, 155, 106, 113, 217, 141, 252, 10, 25, 225, 103, 121, 136, 197, 167, - 188, 250, 213, 76, 75, 158, 22, 178, 10, 176, 188, 242, 119, 164, 1, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 80, 49, 10, 192, 32, 12, 52, 45, 45, 133, 110, 190, + 68, 127, 224, 103, 28, 92, 28, 68, 124, 191, 130, 9, 4, 137, 46, 122, 16, 46, 119, 7, 33, + 9, 168, 142, 175, 21, 96, 255, 32, 147, 230, 32, 207, 33, 155, 61, 88, 56, 55, 203, 240, + 125, 175, 177, 1, 110, 170, 197, 101, 55, 242, 43, 100, 132, 159, 229, 33, 22, 159, 242, + 234, 87, 51, 45, 121, 90, 200, 42, 48, 209, 35, 111, 164, 1, 0, 0, ]; assert_eq!(bytes, expected_serialization) @@ -307,15 +307,15 @@ fn complex_brillig_foreign_call() { let bytes = Program::serialize_program(&program); let expected_serialization: Vec = vec![ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 84, 75, 10, 132, 48, 12, 77, 90, 199, 145, 217, - 205, 13, 6, 102, 14, 208, 241, 4, 222, 69, 220, 41, 186, 244, 248, 90, 140, 24, 159, 5, 23, - 86, 208, 7, 37, 253, 228, 243, 146, 144, 50, 77, 200, 198, 197, 178, 127, 136, 52, 34, 253, - 189, 165, 53, 102, 221, 66, 164, 59, 134, 63, 199, 243, 229, 206, 226, 104, 110, 192, 209, - 158, 192, 145, 84, 255, 47, 216, 239, 152, 125, 137, 90, 63, 27, 152, 159, 132, 166, 249, - 74, 229, 252, 20, 153, 97, 161, 189, 145, 161, 237, 224, 173, 128, 19, 235, 189, 126, 192, - 17, 97, 4, 177, 75, 162, 101, 154, 187, 84, 113, 97, 136, 255, 82, 89, 150, 109, 211, 213, - 85, 111, 65, 21, 233, 126, 213, 254, 7, 239, 12, 118, 104, 171, 161, 63, 176, 144, 46, 7, - 244, 246, 124, 191, 105, 41, 241, 92, 246, 1, 235, 222, 207, 212, 69, 5, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 84, 75, 10, 132, 48, 12, 77, 90, 199, 17, 102, 55, + 39, 24, 152, 57, 64, 199, 19, 120, 23, 113, 167, 232, 210, 227, 107, 49, 98, 124, 22, 92, + 88, 65, 31, 148, 244, 147, 207, 75, 66, 202, 52, 33, 27, 23, 203, 254, 33, 210, 136, 244, + 247, 150, 214, 152, 117, 11, 145, 238, 24, 254, 28, 207, 151, 59, 139, 163, 185, 1, 71, + 123, 2, 71, 82, 253, 191, 96, 191, 99, 246, 37, 106, 253, 108, 96, 126, 18, 154, 230, 43, + 149, 243, 83, 100, 134, 133, 246, 70, 134, 182, 131, 183, 2, 78, 172, 247, 250, 1, 71, 132, + 17, 196, 46, 137, 150, 105, 238, 82, 197, 133, 33, 254, 75, 101, 89, 182, 77, 87, 87, 189, + 5, 85, 164, 251, 85, 251, 31, 188, 51, 216, 161, 173, 134, 254, 192, 66, 186, 28, 208, 219, + 243, 253, 166, 165, 196, 115, 217, 7, 253, 216, 100, 109, 69, 5, 0, 0, ]; assert_eq!(bytes, expected_serialization) diff --git a/noir/noir-repo/acvm-repo/acvm_js/test/shared/complex_foreign_call.ts b/noir/noir-repo/acvm-repo/acvm_js/test/shared/complex_foreign_call.ts index 82f983e407b..53597ece157 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/test/shared/complex_foreign_call.ts +++ b/noir/noir-repo/acvm-repo/acvm_js/test/shared/complex_foreign_call.ts @@ -2,13 +2,13 @@ import { WitnessMap } from '@noir-lang/acvm_js'; // See `complex_brillig_foreign_call` integration test in `acir/tests/test_program_serialization.rs`. export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 84, 75, 10, 132, 48, 12, 77, 90, 199, 145, 217, 205, 13, 6, 102, 14, 208, 241, - 4, 222, 69, 220, 41, 186, 244, 248, 90, 140, 24, 159, 5, 23, 86, 208, 7, 37, 253, 228, 243, 146, 144, 50, 77, 200, - 198, 197, 178, 127, 136, 52, 34, 253, 189, 165, 53, 102, 221, 66, 164, 59, 134, 63, 199, 243, 229, 206, 226, 104, 110, - 192, 209, 158, 192, 145, 84, 255, 47, 216, 239, 152, 125, 137, 90, 63, 27, 152, 159, 132, 166, 249, 74, 229, 252, 20, - 153, 97, 161, 189, 145, 161, 237, 224, 173, 128, 19, 235, 189, 126, 192, 17, 97, 4, 177, 75, 162, 101, 154, 187, 84, - 113, 97, 136, 255, 82, 89, 150, 109, 211, 213, 85, 111, 65, 21, 233, 126, 213, 254, 7, 239, 12, 118, 104, 171, 161, - 63, 176, 144, 46, 7, 244, 246, 124, 191, 105, 41, 241, 92, 246, 1, 235, 222, 207, 212, 69, 5, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 84, 75, 10, 132, 48, 12, 77, 90, 199, 17, 102, 55, 39, 24, 152, 57, 64, 199, + 19, 120, 23, 113, 167, 232, 210, 227, 107, 49, 98, 124, 22, 92, 88, 65, 31, 148, 244, 147, 207, 75, 66, 202, 52, 33, + 27, 23, 203, 254, 33, 210, 136, 244, 247, 150, 214, 152, 117, 11, 145, 238, 24, 254, 28, 207, 151, 59, 139, 163, 185, + 1, 71, 123, 2, 71, 82, 253, 191, 96, 191, 99, 246, 37, 106, 253, 108, 96, 126, 18, 154, 230, 43, 149, 243, 83, 100, + 134, 133, 246, 70, 134, 182, 131, 183, 2, 78, 172, 247, 250, 1, 71, 132, 17, 196, 46, 137, 150, 105, 238, 82, 197, + 133, 33, 254, 75, 101, 89, 182, 77, 87, 87, 189, 5, 85, 164, 251, 85, 251, 31, 188, 51, 216, 161, 173, 134, 254, 192, + 66, 186, 28, 208, 219, 243, 253, 166, 165, 196, 115, 217, 7, 253, 216, 100, 109, 69, 5, 0, 0, ]); export const initialWitnessMap: WitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], diff --git a/noir/noir-repo/acvm-repo/acvm_js/test/shared/foreign_call.ts b/noir/noir-repo/acvm-repo/acvm_js/test/shared/foreign_call.ts index dad7c7e1568..3500e03776d 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/test/shared/foreign_call.ts +++ b/noir/noir-repo/acvm-repo/acvm_js/test/shared/foreign_call.ts @@ -2,10 +2,10 @@ import { WitnessMap } from '@noir-lang/acvm_js'; // See `simple_brillig_foreign_call` integration test in `acir/tests/test_program_serialization.rs`. export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 80, 49, 10, 192, 32, 12, 52, 45, 45, 165, 155, 63, 209, 31, 248, 25, 7, 23, 7, - 17, 223, 175, 96, 2, 65, 162, 139, 30, 132, 203, 221, 65, 72, 2, 170, 227, 107, 5, 216, 63, 200, 164, 57, 200, 115, - 200, 102, 15, 22, 206, 205, 50, 124, 223, 107, 108, 128, 155, 106, 113, 217, 141, 252, 10, 25, 225, 103, 121, 136, - 197, 167, 188, 250, 213, 76, 75, 158, 22, 178, 10, 176, 188, 242, 119, 164, 1, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 80, 49, 10, 192, 32, 12, 52, 45, 45, 133, 110, 190, 68, 127, 224, 103, 28, 92, + 28, 68, 124, 191, 130, 9, 4, 137, 46, 122, 16, 46, 119, 7, 33, 9, 168, 142, 175, 21, 96, 255, 32, 147, 230, 32, 207, + 33, 155, 61, 88, 56, 55, 203, 240, 125, 175, 177, 1, 110, 170, 197, 101, 55, 242, 43, 100, 132, 159, 229, 33, 22, 159, + 242, 234, 87, 51, 45, 121, 90, 200, 42, 48, 209, 35, 111, 164, 1, 0, 0, ]); export const initialWitnessMap: WitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000005'], diff --git a/noir/noir-repo/acvm-repo/brillig/src/opcodes.rs b/noir/noir-repo/acvm-repo/brillig/src/opcodes.rs index fdcae01b5b5..f2ba04a4d59 100644 --- a/noir/noir-repo/acvm-repo/brillig/src/opcodes.rs +++ b/noir/noir-repo/acvm-repo/brillig/src/opcodes.rs @@ -223,6 +223,11 @@ pub enum BrilligOpcode { bit_size: BitSize, value: F, }, + IndirectConst { + destination_pointer: MemoryAddress, + bit_size: BitSize, + value: F, + }, Return, /// Used to get data from an outside source. /// Also referred to as an Oracle. However, we don't use that name as 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 936ad120335..81d7c9261b7 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs @@ -339,6 +339,13 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { self.memory.write(*destination, MemoryValue::new_from_field(*value, *bit_size)); self.increment_program_counter() } + Opcode::IndirectConst { destination_pointer, bit_size, value } => { + // Convert our destination_pointer to an address + let destination = self.memory.read_ref(*destination_pointer); + // Use our usize destination index to set the value in memory + self.memory.write(destination, MemoryValue::new_from_field(*value, *bit_size)); + self.increment_program_counter() + } Opcode::BlackBox(black_box_op) => { match evaluate_black_box( black_box_op, 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 76ab613ed1f..3edab8c3af3 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 @@ -1796,10 +1796,19 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.mov_instruction(write_pointer_register, pointer); for (element_idx, element_id) in data.iter().enumerate() { - 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); + if let Some((constant, typ)) = dfg.get_numeric_constant_with_type(*element_id) { + self.brillig_context.indirect_const_instruction( + write_pointer_register, + typ.bit_size(), + constant, + ); + } else { + 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); + } + if element_idx != data.len() - 1 { // Increment the write_pointer_register self.brillig_context.memory_op_instruction( 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 8e52cf072b4..2533217de2c 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 @@ -200,8 +200,11 @@ impl BrilligContext { let current_revert_data_pointer = ctx.allocate_register(); ctx.mov_instruction(current_revert_data_pointer, revert_data.pointer); - let revert_data_id = ctx.make_constant_instruction((error_selector as u128).into(), 64); - ctx.store_instruction(current_revert_data_pointer, revert_data_id.address); + ctx.indirect_const_instruction( + current_revert_data_pointer, + 64, + (error_selector as u128).into(), + ); ctx.codegen_usize_op_in_place(current_revert_data_pointer, BrilligBinaryOp::Add, 1); for (revert_variable, revert_param) in @@ -238,7 +241,6 @@ impl BrilligContext { ctx.trap_instruction(revert_data); ctx.deallocate_register(revert_data.pointer); ctx.deallocate_register(current_revert_data_pointer); - ctx.deallocate_single_addr(revert_data_id); }); } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs index b258905d657..b0d63d2fa32 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs @@ -173,6 +173,15 @@ impl DebugShow { debug_println!(self.enable_debug_trace, " CONST {} = {}", result, constant); } + /// Stores the value of `constant` in the `result` register + pub(crate) fn indirect_const_instruction( + &self, + result_pointer: MemoryAddress, + constant: F, + ) { + debug_println!(self.enable_debug_trace, " ICONST {} = {}", result_pointer, constant); + } + /// Processes a not instruction. Append with "_" as this is a high-level instruction. pub(crate) fn not_instruction( &self, 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 69a6b12c9b0..6c29cbc7bbb 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 @@ -372,17 +372,28 @@ impl BrilligContext { /// Stores the value of `constant` in the `result` register pub(crate) fn const_instruction(&mut self, result: SingleAddrVariable, constant: F) { self.debug_show.const_instruction(result.address, constant); - self.constant(result, constant); + self.constant(result.address, result.bit_size, constant, false); } - fn constant(&mut self, result: SingleAddrVariable, constant: F) { + /// Stores the value of `constant` in the result_pointer + pub(crate) fn indirect_const_instruction( + &mut self, + result_pointer: MemoryAddress, + bit_size: u32, + constant: F, + ) { + self.debug_show.indirect_const_instruction(result_pointer, constant); + self.constant(result_pointer, bit_size, constant, true); + } + + fn constant(&mut self, result: MemoryAddress, bit_size: u32, constant: F, indirect: bool) { assert!( - result.bit_size >= constant.num_bits(), + bit_size >= constant.num_bits(), "Constant {} does not fit in bit size {}", constant, - result.bit_size + bit_size ); - if result.bit_size > 128 && constant.num_bits() > 128 { + if bit_size > 128 && constant.num_bits() > 128 { let high = F::from_be_bytes_reduce( constant.to_be_bytes().get(0..16).expect("FieldElement::to_be_bytes() too short!"), ); @@ -390,24 +401,45 @@ impl BrilligContext { let high_register = SingleAddrVariable::new(self.allocate_register(), 254); let low_register = SingleAddrVariable::new(self.allocate_register(), 254); let intermediate_register = SingleAddrVariable::new(self.allocate_register(), 254); - self.constant(high_register, high); - self.constant(low_register, low); + + self.constant(high_register.address, high_register.bit_size, high, false); + self.constant(low_register.address, low_register.bit_size, low, false); // I want to multiply high by 2^128, but I can't get that big constant in. // So I'll multiply by 2^64 twice. - self.constant(intermediate_register, F::from(1_u128 << 64)); + self.constant( + intermediate_register.address, + intermediate_register.bit_size, + F::from(1_u128 << 64), + false, + ); self.binary(high_register, intermediate_register, high_register, BrilligBinaryOp::Mul); self.binary(high_register, intermediate_register, high_register, BrilligBinaryOp::Mul); // Now we can add. self.binary(high_register, low_register, intermediate_register, BrilligBinaryOp::Add); - self.cast(result, intermediate_register); + if indirect { + self.cast( + SingleAddrVariable::new(intermediate_register.address, bit_size), + intermediate_register, + ); + self.store_instruction(result, intermediate_register.address); + } else { + self.cast(SingleAddrVariable::new(result, bit_size), intermediate_register); + } + self.deallocate_single_addr(high_register); self.deallocate_single_addr(low_register); self.deallocate_single_addr(intermediate_register); + } else if indirect { + self.push_opcode(BrilligOpcode::IndirectConst { + destination_pointer: result, + value: constant, + bit_size: BitSize::try_from_u32::(bit_size).unwrap(), + }); } else { self.push_opcode(BrilligOpcode::Const { - destination: result.address, + destination: result, value: constant, - bit_size: BitSize::try_from_u32::(result.bit_size).unwrap(), + bit_size: BitSize::try_from_u32::(bit_size).unwrap(), }); } } @@ -429,7 +461,7 @@ impl BrilligContext { fn make_constant(&mut self, constant: F, bit_size: u32) -> SingleAddrVariable { let var = SingleAddrVariable::new(self.allocate_register(), bit_size); - self.constant(var, constant); + self.constant(var.address, var.bit_size, constant, false); var } diff --git a/noir/noir-repo/tooling/profiler/src/opcode_formatter.rs b/noir/noir-repo/tooling/profiler/src/opcode_formatter.rs index 772c87ad1cb..aacee42540c 100644 --- a/noir/noir-repo/tooling/profiler/src/opcode_formatter.rs +++ b/noir/noir-repo/tooling/profiler/src/opcode_formatter.rs @@ -129,6 +129,7 @@ fn format_brillig_opcode_kind(opcode: &BrilligOpcode) -> String { BrilligOpcode::Cast { .. } => "cast".to_string(), BrilligOpcode::ConditionalMov { .. } => "cmov".to_string(), BrilligOpcode::Const { .. } => "const".to_string(), + BrilligOpcode::IndirectConst { .. } => "iconst".to_string(), BrilligOpcode::ForeignCall { function, .. } => format!("foreign_call({})", function), BrilligOpcode::Jump { .. } => "jump".to_string(), BrilligOpcode::JumpIf { .. } => "jump_if".to_string(), From 83add0caf94487990978f2cf1193f741857a32d3 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 19 Aug 2024 12:31:36 +0000 Subject: [PATCH 2/4] fix: avm support for indirect set --- avm-transpiler/src/transpile.rs | 2 +- yarn-project/simulator/src/avm/opcodes/memory.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index e3805bc8ec7..af09cd8fd34 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -699,7 +699,7 @@ fn generate_set_instruction( ) -> AvmInstruction { AvmInstruction { opcode: AvmOpcode::SET, - indirect: if indirect { Some(FIRST_OPERAND_INDIRECT) } else { Some(ALL_DIRECT) }, + indirect: if indirect { Some(ZEROTH_OPERAND_INDIRECT) } else { Some(ALL_DIRECT) }, tag: Some(tag), operands: vec![ // const diff --git a/yarn-project/simulator/src/avm/opcodes/memory.ts b/yarn-project/simulator/src/avm/opcodes/memory.ts index 5bf4c56c623..3c37672c028 100644 --- a/yarn-project/simulator/src/avm/opcodes/memory.ts +++ b/yarn-project/simulator/src/avm/opcodes/memory.ts @@ -77,9 +77,10 @@ export class Set extends Instruction { if ([TypeTag.FIELD, TypeTag.UNINITIALIZED, TypeTag.INVALID].includes(this.inTag)) { throw new InstructionExecutionError(`Invalid tag ${TypeTag[this.inTag]} for SET.`); } + const [dstOffset] = Addressing.fromWire(this.indirect).resolve([this.dstOffset], memory); const res = TaggedMemory.integralFromTag(this.value, this.inTag); - memory.set(this.dstOffset, res); + memory.set(dstOffset, res); memory.assert(memoryOperations); context.machineState.incrementPc(); From 1366153e48e0656c6f9662c728217676eced4ff0 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 19 Aug 2024 12:50:50 +0000 Subject: [PATCH 3/4] fix: indirect const can generate extra opcodes --- avm-transpiler/src/transpile.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index af09cd8fd34..03abb07d289 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -1128,6 +1128,7 @@ pub fn map_brillig_pcs_to_avm_pcs(brillig_bytecode: &[BrilligOpcode 2, + BrilligOpcode::IndirectConst { bit_size: BitSize::Field, .. } => 2, BrilligOpcode::Cast { bit_size: BitSize::Integer(IntegerBitSize::U1), .. } => 3, _ => 1, }; From 7ba10a97ed3935875cc6b61634fa7688d329fddd Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 19 Aug 2024 13:53:54 +0000 Subject: [PATCH 4/4] test: add test for indirect const --- .../noir-repo/acvm-repo/brillig_vm/src/lib.rs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) 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 81d7c9261b7..5097ecf4707 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs @@ -1196,6 +1196,34 @@ mod tests { assert_eq!(memory, expected); } + #[test] + fn iconst_opcode() { + let opcodes = &[ + Opcode::Const { + destination: MemoryAddress(0), + bit_size: BitSize::Integer(MEMORY_ADDRESSING_BIT_SIZE), + value: FieldElement::from(8_usize), + }, + Opcode::IndirectConst { + destination_pointer: MemoryAddress(0), + bit_size: BitSize::Integer(MEMORY_ADDRESSING_BIT_SIZE), + value: FieldElement::from(27_usize), + }, + ]; + let mut vm = VM::new(vec![], opcodes, vec![], &StubbedBlackBoxSolver); + + let status = vm.process_opcode(); + assert_eq!(status, VMStatus::InProgress); + + let status = vm.process_opcode(); + assert_eq!(status, VMStatus::Finished { return_data_offset: 0, return_data_size: 0 }); + + let VM { memory, .. } = vm; + + let destination_value = memory.read(MemoryAddress::from(8)); + assert_eq!(destination_value.to_field(), (27_usize).into()); + } + #[test] fn load_opcode() { /// Brillig code for the following: