From 857ff97b196174a0999f0fe7e387bfca5c3b7cd3 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 10 Jan 2024 20:12:16 +0000 Subject: [PATCH] feat: remove truncation from brillig casts (#3997) # Description ## Problem\* Resolves ## Summary\* As part of #3984, I noticed that I forgot to remove the truncation behaviour from casts in brillig. This PR updates them to be a simple `mov` instruction. In order to do this I've added a proper implementation of `Instruction::Truncate`. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../src/brillig/brillig_gen/brillig_block.rs | 59 ++++--------------- .../noirc_evaluator/src/brillig/brillig_ir.rs | 55 +++++++---------- .../src/brillig/brillig_ir/debug_show.rs | 4 +- 3 files changed, 37 insertions(+), 81 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 0e06a36fd9..db005d9d43 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -521,7 +521,7 @@ impl<'block> BrilligBlock<'block> { unreachable!("unsupported function call type {:?}", dfg[*func]) } }, - Instruction::Truncate { value, .. } => { + Instruction::Truncate { value, bit_size, .. } => { let result_ids = dfg.instruction_results(instruction_id); let destination_register = self.variables.define_register_variable( self.function_context, @@ -530,9 +530,13 @@ impl<'block> BrilligBlock<'block> { dfg, ); let source_register = self.convert_ssa_register_value(*value, dfg); - self.brillig_context.truncate_instruction(destination_register, source_register); + self.brillig_context.truncate_instruction( + destination_register, + source_register, + *bit_size, + ); } - Instruction::Cast(value, target_type) => { + Instruction::Cast(value, _) => { let result_ids = dfg.instruction_results(instruction_id); let destination_register = self.variables.define_register_variable( self.function_context, @@ -541,12 +545,7 @@ impl<'block> BrilligBlock<'block> { dfg, ); let source_register = self.convert_ssa_register_value(*value, dfg); - self.convert_cast( - destination_register, - source_register, - target_type, - &dfg.type_of_value(*value), - ); + self.convert_cast(destination_register, source_register); } Instruction::ArrayGet { array, index } => { let result_ids = dfg.instruction_results(instruction_id); @@ -1092,43 +1091,11 @@ impl<'block> BrilligBlock<'block> { /// Converts an SSA cast to a sequence of Brillig opcodes. /// Casting is only necessary when shrinking the bit size of a numeric value. - fn convert_cast( - &mut self, - destination: RegisterIndex, - source: RegisterIndex, - target_type: &Type, - source_type: &Type, - ) { - fn numeric_to_bit_size(typ: &NumericType) -> u32 { - match typ { - NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => *bit_size, - NumericType::NativeField => FieldElement::max_num_bits(), - } - } - // Casting is only valid for numeric types - // This should be checked by the frontend, so we panic if this is the case - let (source_numeric_type, target_numeric_type) = match (source_type, target_type) { - (Type::Numeric(source_numeric_type), Type::Numeric(target_numeric_type)) => { - (source_numeric_type, target_numeric_type) - } - _ => unimplemented!("The cast operation is only valid for integers."), - }; - let source_bit_size = numeric_to_bit_size(source_numeric_type); - let target_bit_size = numeric_to_bit_size(target_numeric_type); - // Casting from a larger bit size to a smaller bit size (narrowing cast) - // requires a cast instruction. - // If its a widening cast, ie casting from a smaller bit size to a larger bit size - // we simply put a mov instruction as a no-op - // - // Field elements by construction always have the largest bit size - // This means that casting to a Field element, will always be a widening cast - // and therefore a no-op. Conversely, casting from a Field element - // will always be a narrowing cast and therefore a cast instruction - if source_bit_size > target_bit_size { - self.brillig_context.cast_instruction(destination, source, target_bit_size); - } else { - self.brillig_context.mov_instruction(destination, source); - } + fn convert_cast(&mut self, destination: RegisterIndex, source: RegisterIndex) { + // We assume that `source` is a valid `target_type` as it's expected that a truncate instruction was emitted + // to ensure this is the case. + + self.brillig_context.mov_instruction(destination, source); } /// Converts the Binary instruction into a sequence of Brillig opcodes. diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index ff182aaa7d..3c4e77b09e 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -687,10 +687,29 @@ impl BrilligContext { &mut self, destination_of_truncated_value: RegisterIndex, value_to_truncate: RegisterIndex, + bit_size: u32, ) { - // Effectively a no-op because brillig already has implicit truncation on integer - // operations. We need only copy the value to it's destination. - self.mov_instruction(destination_of_truncated_value, value_to_truncate); + self.debug_show.truncate_instruction( + destination_of_truncated_value, + value_to_truncate, + bit_size, + ); + assert!( + bit_size <= BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE, + "tried to truncate to a bit size greater than allowed {bit_size}" + ); + + // The brillig VM performs all arithmetic operations modulo 2**bit_size + // So to truncate any value to a target bit size we can just issue a no-op arithmetic operation + // With bit size equal to target_bit_size + let zero_register = self.make_constant(Value::from(FieldElement::zero())); + self.binary_instruction( + value_to_truncate, + zero_register, + destination_of_truncated_value, + BrilligBinaryOp::Integer { op: BinaryIntOp::Add, bit_size }, + ); + self.deallocate_register(zero_register); } /// Emits a stop instruction @@ -761,36 +780,6 @@ impl BrilligContext { self.deallocate_register(scratch_register_j); } - /// Emits a modulo instruction against 2**target_bit_size - /// - /// Integer arithmetic in Brillig is currently constrained to 127 bit integers. - /// We restrict the cast operation, so that integer types over 127 bits - /// cannot be created. - pub(crate) fn cast_instruction( - &mut self, - destination: RegisterIndex, - source: RegisterIndex, - target_bit_size: u32, - ) { - self.debug_show.cast_instruction(destination, source, target_bit_size); - assert!( - target_bit_size <= BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE, - "tried to cast to a bit size greater than allowed {target_bit_size}" - ); - - // The brillig VM performs all arithmetic operations modulo 2**bit_size - // So to cast any value to a target bit size we can just issue a no-op arithmetic operation - // With bit size equal to target_bit_size - let zero_register = self.make_constant(Value::from(FieldElement::zero())); - self.binary_instruction( - source, - zero_register, - destination, - BrilligBinaryOp::Integer { op: BinaryIntOp::Add, bit_size: target_bit_size }, - ); - self.deallocate_register(zero_register); - } - /// Adds a unresolved external `Call` instruction to the bytecode. /// This calls into another function compiled into this brillig artifact. pub(crate) fn add_external_call_instruction(&mut self, func_label: T) { diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs index aae74849b8..74b24b280c 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs @@ -326,7 +326,7 @@ impl DebugShow { } /// Debug function for cast_instruction - pub(crate) fn cast_instruction( + pub(crate) fn truncate_instruction( &self, destination: RegisterIndex, source: RegisterIndex, @@ -334,7 +334,7 @@ impl DebugShow { ) { debug_println!( self.enable_debug_trace, - " CAST {} FROM {} TO {} BITS", + " TRUNCATE {} FROM {} TO {} BITS", destination, source, target_bit_size