Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove truncation from brillig casts #3997

Merged
merged 4 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading