Skip to content

Commit

Permalink
Merge branch 'master' into aztec-packages
Browse files Browse the repository at this point in the history
* master:
  feat: Add checks for bit size consistency on brillig gen (#4542)
  • Loading branch information
TomAFrench committed Mar 13, 2024
2 parents 7464bee + f3243b7 commit 1e97f6b
Show file tree
Hide file tree
Showing 9 changed files with 370 additions and 289 deletions.
208 changes: 105 additions & 103 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
use crate::{
brillig::brillig_ir::{
brillig_variable::{BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable},
BrilligContext, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
BrilligContext,
},
ssa::ir::{
basic_block::BasicBlockId,
Expand All @@ -13,7 +13,7 @@ use crate::{
},
};

use super::brillig_fn::FunctionContext;
use super::brillig_fn::{get_bit_size_from_ssa_type, FunctionContext};

#[derive(Debug, Default)]
pub(crate) struct BlockVariables {
Expand Down Expand Up @@ -189,21 +189,10 @@ pub(crate) fn allocate_value(
let typ = dfg.type_of_value(value_id);

match typ {
Type::Numeric(numeric_type) => BrilligVariable::SingleAddr(SingleAddrVariable {
address: brillig_context.allocate_register(),
bit_size: numeric_type.bit_size(),
}),
Type::Reference(_) => BrilligVariable::SingleAddr(SingleAddrVariable {
address: brillig_context.allocate_register(),
bit_size: BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
}),
Type::Function => {
// NB. function references are converted to a constant when
// translating from SSA to Brillig (to allow for debugger
// instrumentation to work properly)
Type::Numeric(_) | Type::Reference(_) | Type::Function => {
BrilligVariable::SingleAddr(SingleAddrVariable {
address: brillig_context.allocate_register(),
bit_size: 32,
bit_size: get_bit_size_from_ssa_type(&typ),
})
}
Type::Array(item_typ, elem_count) => {
Expand Down
14 changes: 7 additions & 7 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use acvm::FieldElement;
use iter_extended::vecmap;

use crate::{
Expand All @@ -11,7 +10,7 @@ use crate::{
basic_block::BasicBlockId,
function::{Function, FunctionId},
post_order::PostOrder,
types::{NumericType, Type},
types::Type,
value::ValueId,
},
};
Expand Down Expand Up @@ -116,11 +115,12 @@ impl FunctionContext {

pub(crate) fn get_bit_size_from_ssa_type(typ: &Type) -> u32 {
match typ {
Type::Numeric(num_type) => match num_type {
NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => *bit_size,
NumericType::NativeField => FieldElement::max_num_bits(),
},
Type::Numeric(num_type) => num_type.bit_size(),
Type::Reference(_) => BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
_ => unreachable!("ICE bitwise not on a non numeric type"),
// NB. function references are converted to a constant when
// translating from SSA to Brillig (to allow for debugger
// instrumentation to work properly)
Type::Function => 32,
_ => unreachable!("ICE bit size not on a non numeric type"),
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use acvm::brillig_vm::brillig::{BinaryIntOp, MemoryAddress};
use acvm::brillig_vm::brillig::BinaryIntOp;

use crate::brillig::brillig_ir::brillig_variable::{BrilligVariable, BrilligVector};
use crate::brillig::brillig_ir::brillig_variable::{
BrilligVariable, BrilligVector, SingleAddrVariable,
};

use super::brillig_block::BrilligBlock;

Expand All @@ -26,19 +28,19 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.copy_array_instruction(
source_vector.pointer,
target_vector.pointer,
source_vector.size,
SingleAddrVariable::new_usize(source_vector.size),
);

for (index, variable) in variables_to_insert.iter().enumerate() {
let target_index = self.brillig_context.make_usize_constant(index.into());
self.brillig_context.memory_op(
target_index,
target_index.address,
source_vector.size,
target_index,
target_index.address,
BinaryIntOp::Add,
);
self.store_variable_in_array(target_vector.pointer, target_index, *variable);
self.brillig_context.deallocate_register(target_index);
self.brillig_context.deallocate_single_addr(target_index);
}
}

Expand Down Expand Up @@ -72,14 +74,14 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.copy_array_instruction(
source_vector.pointer,
destination_copy_pointer,
source_vector.size,
SingleAddrVariable::new_usize(source_vector.size),
);

// Then we write the items to insert at the start
for (index, variable) in variables_to_insert.iter().enumerate() {
let target_index = self.brillig_context.make_usize_constant(index.into());
self.store_variable_in_array(target_vector.pointer, target_index, *variable);
self.brillig_context.deallocate_register(target_index);
self.brillig_context.deallocate_single_addr(target_index);
}

self.brillig_context.deallocate_register(destination_copy_pointer);
Expand Down Expand Up @@ -115,13 +117,13 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.copy_array_instruction(
source_copy_pointer,
target_vector.pointer,
target_vector.size,
SingleAddrVariable::new_usize(target_vector.size),
);

for (index, variable) in removed_items.iter().enumerate() {
let target_index = self.brillig_context.make_usize_constant(index.into());
self.retrieve_variable_from_array(source_vector.pointer, target_index, *variable);
self.brillig_context.deallocate_register(target_index);
self.brillig_context.deallocate_single_addr(target_index);
}

self.brillig_context.deallocate_register(source_copy_pointer);
Expand All @@ -148,27 +150,27 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.copy_array_instruction(
source_vector.pointer,
target_vector.pointer,
target_vector.size,
SingleAddrVariable::new_usize(target_vector.size),
);

for (index, variable) in removed_items.iter().enumerate() {
let target_index = self.brillig_context.make_usize_constant(index.into());
self.brillig_context.memory_op(
target_index,
target_index.address,
target_vector.size,
target_index,
target_index.address,
BinaryIntOp::Add,
);
self.retrieve_variable_from_array(source_vector.pointer, target_index, *variable);
self.brillig_context.deallocate_register(target_index);
self.brillig_context.deallocate_single_addr(target_index);
}
}

pub(crate) fn slice_insert_operation(
&mut self,
target_vector: BrilligVector,
source_vector: BrilligVector,
index: MemoryAddress,
index: SingleAddrVariable,
items: &[BrilligVariable],
) {
// First we need to allocate the target vector incrementing the size by items.len()
Expand All @@ -193,7 +195,7 @@ impl<'block> BrilligBlock<'block> {
let source_pointer_at_index = self.brillig_context.allocate_register();
self.brillig_context.memory_op(
source_vector.pointer,
index,
index.address,
source_pointer_at_index,
BinaryIntOp::Add,
);
Expand All @@ -202,7 +204,7 @@ impl<'block> BrilligBlock<'block> {
let target_pointer_after_index = self.brillig_context.allocate_register();
self.brillig_context.memory_op(
target_vector.pointer,
index,
index.address,
target_pointer_after_index,
BinaryIntOp::Add,
);
Expand All @@ -214,21 +216,31 @@ impl<'block> BrilligBlock<'block> {

// Compute the number of elements to the right of the index
let item_count = self.brillig_context.allocate_register();
self.brillig_context.memory_op(source_vector.size, index, item_count, BinaryIntOp::Sub);
self.brillig_context.memory_op(
source_vector.size,
index.address,
item_count,
BinaryIntOp::Sub,
);

// Copy the elements to the right of the index
self.brillig_context.copy_array_instruction(
source_pointer_at_index,
target_pointer_after_index,
item_count,
SingleAddrVariable::new_usize(item_count),
);

// Write the items to insert starting at the index
for (subitem_index, variable) in items.iter().enumerate() {
let target_index = self.brillig_context.make_usize_constant(subitem_index.into());
self.brillig_context.memory_op(target_index, index, target_index, BinaryIntOp::Add);
self.brillig_context.memory_op(
target_index.address,
index.address,
target_index.address,
BinaryIntOp::Add,
);
self.store_variable_in_array(target_vector.pointer, target_index, *variable);
self.brillig_context.deallocate_register(target_index);
self.brillig_context.deallocate_single_addr(target_index);
}

self.brillig_context.deallocate_register(source_pointer_at_index);
Expand All @@ -240,7 +252,7 @@ impl<'block> BrilligBlock<'block> {
&mut self,
target_vector: BrilligVector,
source_vector: BrilligVector,
index: MemoryAddress,
index: SingleAddrVariable,
removed_items: &[BrilligVariable],
) {
// First we need to allocate the target vector decrementing the size by removed_items.len()
Expand All @@ -265,7 +277,7 @@ impl<'block> BrilligBlock<'block> {
let source_pointer_after_index = self.brillig_context.allocate_register();
self.brillig_context.memory_op(
source_vector.pointer,
index,
index.address,
source_pointer_after_index,
BinaryIntOp::Add,
);
Expand All @@ -279,29 +291,39 @@ impl<'block> BrilligBlock<'block> {
let target_pointer_at_index = self.brillig_context.allocate_register();
self.brillig_context.memory_op(
target_vector.pointer,
index,
index.address,
target_pointer_at_index,
BinaryIntOp::Add,
);

// Compute the number of elements to the right of the index
let item_count = self.brillig_context.allocate_register();
self.brillig_context.memory_op(source_vector.size, index, item_count, BinaryIntOp::Sub);
self.brillig_context.memory_op(
source_vector.size,
index.address,
item_count,
BinaryIntOp::Sub,
);
self.brillig_context.usize_op_in_place(item_count, BinaryIntOp::Sub, removed_items.len());

// Copy the elements to the right of the index
self.brillig_context.copy_array_instruction(
source_pointer_after_index,
target_pointer_at_index,
item_count,
SingleAddrVariable::new_usize(item_count),
);

// Get the removed items
for (subitem_index, variable) in removed_items.iter().enumerate() {
let target_index = self.brillig_context.make_usize_constant(subitem_index.into());
self.brillig_context.memory_op(target_index, index, target_index, BinaryIntOp::Add);
self.brillig_context.memory_op(
target_index.address,
index.address,
target_index.address,
BinaryIntOp::Add,
);
self.retrieve_variable_from_array(source_vector.pointer, target_index, *variable);
self.brillig_context.deallocate_register(target_index);
self.brillig_context.deallocate_single_addr(target_index);
}

self.brillig_context.deallocate_register(source_pointer_after_index);
Expand Down Expand Up @@ -592,7 +614,10 @@ mod tests {
address: context.allocate_register(),
bit_size: BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
};
let index_to_insert = context.allocate_register();
let index_to_insert = SingleAddrVariable::new(
context.allocate_register(),
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
);

// Cast the source array to a vector
let source_vector = context.array_to_vector(&array_variable);
Expand Down Expand Up @@ -710,7 +735,10 @@ mod tests {
size: array.len(),
rc: context.allocate_register(),
};
let index_to_insert = context.allocate_register();
let index_to_insert = SingleAddrVariable::new(
context.allocate_register(),
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
);

// Cast the source array to a vector
let source_vector = context.array_to_vector(&array_variable);
Expand Down
Loading

0 comments on commit 1e97f6b

Please sign in to comment.