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(avm): fix some Brillig problems #5091

Merged
merged 3 commits into from
Mar 11, 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
8 changes: 8 additions & 0 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
lhs,
rhs,
} => {
assert!(is_integral_bit_size(*bit_size), "BinaryIntOp::{:?} bit_size must be integral, got {:?}", op, bit_size);
let avm_opcode = match op {
BinaryIntOp::Add => AvmOpcode::ADD,
BinaryIntOp::Sub => AvmOpcode::SUB,
Expand Down Expand Up @@ -913,6 +914,13 @@ fn map_brillig_pcs_to_avm_pcs(initial_offset: usize, brillig: &Brillig) -> Vec<u
pc_map
}

fn is_integral_bit_size(bit_size: u32) -> bool {
match bit_size {
1 | 8 | 16 | 32 | 64 | 128 => true,
_ => false,
}
}

fn tag_from_bit_size(bit_size: u32) -> AvmTypeTag {
match bit_size {
1 => AvmTypeTag::UINT8, // temp workaround
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ contract AppSubscription {

global SUBSCRIPTION_DURATION_IN_BLOCKS = 5;
global SUBSCRIPTION_TXS = 5;
global CANONICAL_GAS_TOKEN_ADDRESS = AztecAddress::from_field(0x22a29a9544c0cbdbc35bb21caa78b6f08903be4b390a65319e33afd7443f3499);
global CANONICAL_GAS_TOKEN_ADDRESS = AztecAddress::from_field(10518537853519909204957666322334442672584410385979059968848104222965977783517);

#[aztec(private)]
fn entrypoint(payload: pub DAppPayload, user_address: pub AztecAddress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ contract FPC {
gas_token_address: SharedImmutable<AztecAddress>,
}

global CANONICAL_GAS_TOKEN_ADDRESS = AztecAddress::from_field(0x22a29a9544c0cbdbc35bb21caa78b6f08903be4b390a65319e33afd7443f3499);
global CANONICAL_GAS_TOKEN_ADDRESS = AztecAddress::from_field(10518537853519909204957666322334442672584410385979059968848104222965977783517);

#[aztec(public)]
#[aztec(initializer)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -831,16 +831,10 @@ impl<'block> BrilligBlock<'block> {
_ => unreachable!("ICE: array set on non-array"),
};

// Here we want to compare the reference count against 1.
let one = self.brillig_context.make_usize_constant(1_usize.into());
let condition = self.brillig_context.allocate_register();

self.brillig_context.binary_instruction(
reference_count,
one,
condition,
BrilligBinaryOp::Field { op: BinaryFieldOp::Equals },
);

self.brillig_context.memory_op(reference_count, one, condition, BinaryIntOp::Equals);
self.brillig_context.branch_instruction(condition, |ctx, cond| {
if cond {
// Reference count is 1, we can mutate the array directly
Expand Down
36 changes: 13 additions & 23 deletions noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use acvm::{
FieldElement,
};
use debug_show::DebugShow;
use num_bigint::BigUint;

/// The Brillig VM does not apply a limit to the memory address space,
/// As a convention, we take use 64 bits. This means that we assume that
Expand Down Expand Up @@ -213,13 +212,7 @@ impl BrilligContext {
self.debug_show.array_get(array_ptr, index, result);
// Computes array_ptr + index, ie array[index]
let index_of_element_in_memory = self.allocate_register();
self.binary_instruction(
array_ptr,
index,
index_of_element_in_memory,
BrilligBinaryOp::Field { op: BinaryFieldOp::Add },
);

self.memory_op(array_ptr, index, index_of_element_in_memory, BinaryIntOp::Add);
self.load_instruction(result, index_of_element_in_memory);
// Free up temporary register
self.deallocate_register(index_of_element_in_memory);
Expand All @@ -239,7 +232,10 @@ impl BrilligContext {
array_ptr,
index,
index_of_element_in_memory,
BrilligBinaryOp::Field { op: BinaryFieldOp::Add },
BrilligBinaryOp::Integer {
op: BinaryIntOp::Add,
bit_size: BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
},
Comment on lines 232 to +238
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Re: lines 212 to 238]

what exactly is this change?

See this comment inline on Graphite.

);

self.store_instruction(index_of_element_in_memory, value);
Expand Down Expand Up @@ -726,6 +722,8 @@ impl BrilligContext {
/// Instead truncation instructions are emitted as to when a
/// truncation should be done.
/// For Brillig, all integer operations will overflow as its cheap.
/// We currently use cast to truncate: we cast to the required bit size
/// and back to the original bit size.
pub(crate) fn truncate_instruction(
&mut self,
destination_of_truncated_value: SingleAddrVariable,
Expand All @@ -744,20 +742,12 @@ impl BrilligContext {
value_to_truncate.bit_size
fcarreiro marked this conversation as resolved.
Show resolved Hide resolved
);

let mask = BigUint::from(2_u32).pow(bit_size) - BigUint::from(1_u32);
let mask_constant = self.make_constant(
FieldElement::from_be_bytes_reduce(&mask.to_bytes_be()).into(),
value_to_truncate.bit_size,
);

self.binary_instruction(
value_to_truncate.address,
mask_constant,
destination_of_truncated_value.address,
BrilligBinaryOp::Integer { op: BinaryIntOp::And, bit_size: value_to_truncate.bit_size },
);

self.deallocate_register(mask_constant);
// We cast back and forth to ensure that the value is truncated.
let intermediate_register =
SingleAddrVariable { address: self.allocate_register(), bit_size };
self.cast_instruction(intermediate_register, value_to_truncate);
self.cast_instruction(destination_of_truncated_value, intermediate_register);
self.deallocate_register(intermediate_register.address);
}

/// Emits a stop instruction
Expand Down
Loading
Loading