From 1a058501f9f2950cf55c6d29bad1d412cf78b98a Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 19 Aug 2024 16:12:09 +0000 Subject: [PATCH 1/2] feat: to_radix optimization --- avm-transpiler/src/transpile.rs | 3 +- .../dsl/acir_format/serde/acir.hpp | 6 ++++ .../noir-repo/acvm-repo/acir/codegen/acir.cpp | 4 +++ noir/noir-repo/acvm-repo/acir/src/lib.rs | 4 +-- .../acvm-repo/brillig/src/black_box.rs | 1 + .../acvm-repo/brillig_vm/src/black_box.rs | 15 ++++++-- .../src/brillig/brillig_gen/brillig_block.rs | 4 +-- .../brillig/brillig_ir/codegen_intrinsic.rs | 34 ++----------------- .../src/brillig/brillig_ir/debug_show.rs | 2 +- 9 files changed, 32 insertions(+), 41 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 437933ce294b..02b99d21574c 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -822,7 +822,8 @@ fn handle_black_box_function(avm_instrs: &mut Vec, operation: &B ..Default::default() }); } - BlackBoxOp::ToRadix { input, radix, output } => { + // We ignore the output bits flag since we represent bits as bytes + BlackBoxOp::ToRadix { input, radix, output, output_bits: _ } => { let num_limbs = output.size as u32; let input_offset = input.0 as u32; let output_offset = output.pointer.0 as u32; 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 dc4a52628c12..ae2bd59e5444 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -464,6 +464,7 @@ struct BlackBoxOp { Program::MemoryAddress input; uint32_t radix; Program::HeapArray output; + bool output_bits; friend bool operator==(const ToRadix&, const ToRadix&); std::vector bincodeSerialize() const; @@ -5400,6 +5401,9 @@ inline bool operator==(const BlackBoxOp::ToRadix& lhs, const BlackBoxOp::ToRadix if (!(lhs.output == rhs.output)) { return false; } + if (!(lhs.output_bits == rhs.output_bits)) { + return false; + } return true; } @@ -5430,6 +5434,7 @@ void serde::Serializable::serialize(const Program: serde::Serializable::serialize(obj.input, serializer); serde::Serializable::serialize(obj.radix, serializer); serde::Serializable::serialize(obj.output, serializer); + serde::Serializable::serialize(obj.output_bits, serializer); } template <> @@ -5441,6 +5446,7 @@ Program::BlackBoxOp::ToRadix serde::Deserializable obj.input = serde::Deserializable::deserialize(deserializer); obj.radix = serde::Deserializable::deserialize(deserializer); obj.output = serde::Deserializable::deserialize(deserializer); + obj.output_bits = serde::Deserializable::deserialize(deserializer); return obj; } diff --git a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp index 2f8835028693..aae00c45a2b0 100644 --- a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp +++ b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp @@ -464,6 +464,7 @@ namespace Program { Program::MemoryAddress input; uint32_t radix; Program::HeapArray output; + bool output_bits; friend bool operator==(const ToRadix&, const ToRadix&); std::vector bincodeSerialize() const; @@ -4529,6 +4530,7 @@ namespace Program { if (!(lhs.input == rhs.input)) { return false; } if (!(lhs.radix == rhs.radix)) { return false; } if (!(lhs.output == rhs.output)) { return false; } + if (!(lhs.output_bits == rhs.output_bits)) { return false; } return true; } @@ -4555,6 +4557,7 @@ void serde::Serializable::serialize(const Program: serde::Serializable::serialize(obj.input, serializer); serde::Serializable::serialize(obj.radix, serializer); serde::Serializable::serialize(obj.output, serializer); + serde::Serializable::serialize(obj.output_bits, serializer); } template <> @@ -4564,6 +4567,7 @@ Program::BlackBoxOp::ToRadix serde::Deserializable obj.input = serde::Deserializable::deserialize(deserializer); obj.radix = serde::Deserializable::deserialize(deserializer); obj.output = serde::Deserializable::deserialize(deserializer); + obj.output_bits = serde::Deserializable::deserialize(deserializer); return obj; } diff --git a/noir/noir-repo/acvm-repo/acir/src/lib.rs b/noir/noir-repo/acvm-repo/acir/src/lib.rs index 845a1d6ad5ac..7f5334e263cb 100644 --- a/noir/noir-repo/acvm-repo/acir/src/lib.rs +++ b/noir/noir-repo/acvm-repo/acir/src/lib.rs @@ -96,7 +96,7 @@ mod reflection { // Comment this out to write updated C++ code to file. if let Some(old_hash) = old_hash { let new_hash = fxhash::hash64(&source); - assert_eq!(new_hash, old_hash, "Serialization format has changed"); + // assert_eq!(new_hash, old_hash, "Serialization format has changed"); } write_to_file(&source, &path); @@ -130,7 +130,7 @@ mod reflection { // Comment this out to write updated C++ code to file. if let Some(old_hash) = old_hash { let new_hash = fxhash::hash64(&source); - assert_eq!(new_hash, old_hash, "Serialization format has changed"); + // assert_eq!(new_hash, old_hash, "Serialization format has changed"); } write_to_file(&source, &path); diff --git a/noir/noir-repo/acvm-repo/brillig/src/black_box.rs b/noir/noir-repo/acvm-repo/brillig/src/black_box.rs index 60e4af11ea21..c3240c6ff1e4 100644 --- a/noir/noir-repo/acvm-repo/brillig/src/black_box.rs +++ b/noir/noir-repo/acvm-repo/brillig/src/black_box.rs @@ -132,5 +132,6 @@ pub enum BlackBoxOp { input: MemoryAddress, radix: u32, output: HeapArray, + output_bits: bool, }, } diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs index b49757944adb..3f1a44b921b4 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs @@ -1,4 +1,4 @@ -use acir::brillig::{BlackBoxOp, HeapArray, HeapVector}; +use acir::brillig::{BlackBoxOp, HeapArray, HeapVector, IntegerBitSize}; use acir::{AcirField, BlackBoxFunc}; use acvm_blackbox_solver::BigIntSolver; use acvm_blackbox_solver::{ @@ -6,6 +6,7 @@ use acvm_blackbox_solver::{ keccakf1600, sha256, sha256compression, BlackBoxFunctionSolver, BlackBoxResolutionError, }; use num_bigint::BigUint; +use num_traits::Zero; use crate::memory::MemoryValue; use crate::Memory; @@ -366,7 +367,7 @@ pub(crate) fn evaluate_black_box memory.write_slice(memory.read_ref(output.pointer), &state); Ok(()) } - BlackBoxOp::ToRadix { input, radix, output } => { + BlackBoxOp::ToRadix { input, radix, output, output_bits } => { let input: F = *memory.read(*input).extract_field().expect("ToRadix input not a field"); let mut input = BigUint::from_bytes_be(&input.to_be_bytes()); @@ -376,7 +377,15 @@ pub(crate) fn evaluate_black_box for _ in 0..output.size { let limb = &input % &radix; - limbs.push(MemoryValue::new_field(F::from_be_bytes_reduce(&limb.to_bytes_be()))); + if *output_bits { + limbs.push(MemoryValue::new_integer( + if limb.is_zero() { 0 } else { 1 }, + IntegerBitSize::U1, + )); + } else { + let limb: u8 = limb.try_into().unwrap(); + limbs.push(MemoryValue::new_integer(limb as u128, IntegerBitSize::U8)); + }; input /= &radix; } 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 74fdbfcdce69..c751f4607fba 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 @@ -549,7 +549,7 @@ impl<'block> BrilligBlock<'block> { radix, limb_count, matches!(endianness, Endian::Big), - 8, + false, ); } Value::Intrinsic(Intrinsic::ToBits(endianness)) => { @@ -595,7 +595,7 @@ impl<'block> BrilligBlock<'block> { 2, limb_count, matches!(endianness, Endian::Big), - 1, + true, ); } 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 3e3ac3f1edea..5c171f167fcd 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 @@ -71,7 +71,7 @@ impl BrilligContext< radix: u32, limb_count: usize, big_endian: bool, - limb_bit_size: u32, + output_bits: bool, // If true will generate bit limbs, if false will generate byte limbs ) { assert!(source_field.bit_size == F::max_num_bits()); @@ -83,39 +83,9 @@ impl BrilligContext< input: source_field.address, radix, output: HeapArray { pointer: target_vector.pointer, size: limb_count }, + output_bits, }); - if limb_bit_size != F::max_num_bits() { - let end_pointer = self.allocate_register(); - let temporary_register = self.allocate_register(); - - self.memory_op_instruction( - target_vector.pointer, - target_vector.size, - end_pointer, - BrilligBinaryOp::Add, - ); - - self.codegen_for_loop( - Some(target_vector.pointer), - end_pointer, - None, - |ctx, item_pointer| { - ctx.load_instruction(temporary_register, item_pointer.address); - - ctx.cast( - SingleAddrVariable::new(temporary_register, limb_bit_size), - SingleAddrVariable::new(temporary_register, F::max_num_bits()), - ); - - ctx.store_instruction(item_pointer.address, temporary_register); - }, - ); - - self.deallocate_register(end_pointer); - self.deallocate_register(temporary_register); - } - if big_endian { self.codegen_array_reverse(target_vector.pointer, target_vector.size); } 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 b258905d657e..65cc7c47f6b6 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 @@ -453,7 +453,7 @@ impl DebugShow { output ); } - BlackBoxOp::ToRadix { input, radix, output } => { + BlackBoxOp::ToRadix { input, radix, output, output_bits: _ } => { debug_println!( self.enable_debug_trace, " TO_RADIX {} {} -> {}", From 30d83d92d129b05a9bfbb15096d19800f5b7c64e Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 19 Aug 2024 16:14:40 +0000 Subject: [PATCH 2/2] chore: remove extraneous comment --- noir/noir-repo/acvm-repo/acir/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir/noir-repo/acvm-repo/acir/src/lib.rs b/noir/noir-repo/acvm-repo/acir/src/lib.rs index 7f5334e263cb..845a1d6ad5ac 100644 --- a/noir/noir-repo/acvm-repo/acir/src/lib.rs +++ b/noir/noir-repo/acvm-repo/acir/src/lib.rs @@ -96,7 +96,7 @@ mod reflection { // Comment this out to write updated C++ code to file. if let Some(old_hash) = old_hash { let new_hash = fxhash::hash64(&source); - // assert_eq!(new_hash, old_hash, "Serialization format has changed"); + assert_eq!(new_hash, old_hash, "Serialization format has changed"); } write_to_file(&source, &path); @@ -130,7 +130,7 @@ mod reflection { // Comment this out to write updated C++ code to file. if let Some(old_hash) = old_hash { let new_hash = fxhash::hash64(&source); - // assert_eq!(new_hash, old_hash, "Serialization format has changed"); + assert_eq!(new_hash, old_hash, "Serialization format has changed"); } write_to_file(&source, &path);