Skip to content

Commit

Permalink
feat: remove truncation from brillig casts (#3997)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## 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.
  • Loading branch information
TomAFrench authored Jan 10, 2024
1 parent b003f0a commit 857ff97
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 81 deletions.
59 changes: 13 additions & 46 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
55 changes: 22 additions & 33 deletions compiler/noirc_evaluator/src/brillig/brillig_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<T: ToString>(&mut self, func_label: T) {
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,15 +326,15 @@ impl DebugShow {
}

/// Debug function for cast_instruction
pub(crate) fn cast_instruction(
pub(crate) fn truncate_instruction(
&self,
destination: RegisterIndex,
source: RegisterIndex,
target_bit_size: u32,
) {
debug_println!(
self.enable_debug_trace,
" CAST {} FROM {} TO {} BITS",
" TRUNCATE {} FROM {} TO {} BITS",
destination,
source,
target_bit_size
Expand Down

0 comments on commit 857ff97

Please sign in to comment.