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: Sync from noir #6086

Merged
merged 14 commits into from
May 1, 2024
Merged
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1ec9cdc7013e867db3672d27e3a6104e4b7e7eef
8aad2e45acbe08afc3902db95a83324f822c35eb
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,12 @@ contract AvmTest {

#[aztec(public-vm)]
fn set_opcode_u32() -> pub u32 {
1 << 30 as u32
1 << 30 as u8
}

#[aztec(public-vm)]
fn set_opcode_u64() -> pub u64 {
1 << 60 as u64
1 << 60 as u8
}

#[aztec(public-vm)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub fn insert_subtree_to_snapshot_tree<N>(
) -> AppendOnlyTreeSnapshot {
// TODO(Lasse): Sanity check len of sibling_path > height of subtree
// TODO(Lasse): Ensure height of subtree is correct (eg 3 for commitments, 1 for contracts)
let leaf_index_at_depth = snapshot.next_available_leaf_index >> (subtree_depth as u32);
let leaf_index_at_depth = snapshot.next_available_leaf_index >> (subtree_depth as u8);

// Check that the current root is correct and that there is an empty subtree at the insertion location
assert_check_membership(
Expand All @@ -30,7 +30,7 @@ pub fn insert_subtree_to_snapshot_tree<N>(
);

// 2^subtree_depth is the number of leaves added. 2^x = 1 << x
let new_next_available_leaf_index = (snapshot.next_available_leaf_index as u64) + (1 << (subtree_depth as u64));
let new_next_available_leaf_index = (snapshot.next_available_leaf_index as u64) + (1 << (subtree_depth as u8));

AppendOnlyTreeSnapshot { root: new_root, next_available_leaf_index: new_next_available_leaf_index as u32 }
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub fn batch_insert<Value, Leaf, SubtreeWidth, SiblingPathLength, SubtreeHeight,
}

let empty_subtree_root = calculate_empty_tree_root(SubtreeHeight);
let leaf_index_subtree_depth = start_insertion_index >> (SubtreeHeight as u32);
let leaf_index_subtree_depth = start_insertion_index >> (SubtreeHeight as u8);

assert_check_membership(
empty_subtree_root,
Expand All @@ -78,7 +78,7 @@ pub fn batch_insert<Value, Leaf, SubtreeWidth, SiblingPathLength, SubtreeHeight,

// Calculate the new root
// We are inserting a subtree rather than a full tree here
let subtree_index = start_insertion_index >> (SubtreeHeight as u32);
let subtree_index = start_insertion_index >> (SubtreeHeight as u8);
let new_root = root_from_sibling_path(subtree_root, subtree_index as Field, new_subtree_sibling_path);

AppendOnlyTreeSnapshot { root: new_root, next_available_leaf_index: start_insertion_index + (values_to_insert.len() as u32) }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use crate::{
merkle_tree::{MerkleTree, calculate_empty_tree_root},
traits::Empty
};
use crate::{merkle_tree::{MerkleTree, calculate_empty_tree_root}, traits::Empty};

pub fn compute_zero_hashes<N>(mut hashes: [Field; N]) -> [Field; N] {
hashes[0] = dep::std::hash::pedersen_hash([0, 0]);
Expand Down Expand Up @@ -111,7 +108,7 @@ impl<SUBTREE_ITEMS, TREE_HEIGHT, SUPERTREE_HEIGHT, SUBTREE_HEIGHT> NonEmptyMerkl
left_supertree_branch[0] = dep::std::hash::pedersen_hash([subtree.get_root(), zero_hashes[SUBTREE_HEIGHT-1]]);
for i in 1..left_supertree_branch.len() {
// TODO(md): far right of this yuck
left_supertree_branch[i] = dep::std::hash::pedersen_hash([left_supertree_branch[i-1], zero_hashes[(SUBTREE_HEIGHT-1 as u32) + i as u32]]);
left_supertree_branch[i] = dep::std::hash::pedersen_hash([left_supertree_branch[i-1], zero_hashes[(SUBTREE_HEIGHT as u64 -1) + i as u64]]);
}

NonEmptyMerkleTree { subtree, zero_hashes, left_supertree_branch, _phantom_subtree_height: _subtree_height }
Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/.github/workflows/publish-nargo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ permissions:

jobs:
build-apple-darwin:
runs-on: macos-latest
runs-on: macos-12
env:
CROSS_CONFIG: ${{ github.workspace }}/.github/Cross.toml
NIGHTLY_RELEASE: ${{ inputs.tag == '' }}
Expand Down
4 changes: 3 additions & 1 deletion noir/noir-repo/acvm-repo/brillig_vm/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ pub(crate) fn evaluate_binary_int_op(
}
}
})?;
let rhs = rhs.expect_integer_with_bit_size(bit_size).map_err(|err| match err {
let rhs_bit_size =
if op == &BinaryIntOp::Shl || op == &BinaryIntOp::Shr { 8 } else { bit_size };
let rhs = rhs.expect_integer_with_bit_size(rhs_bit_size).map_err(|err| match err {
MemoryTypeError::MismatchedBitSize { value_bit_size, expected_bit_size } => {
BrilligArithmeticError::MismatchedRhsBitSize {
rhs_bit_size: value_bit_size,
Expand Down
1 change: 1 addition & 0 deletions noir/noir-repo/aztec_macros/src/utils/hir_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ pub fn inject_global(
module_id,
file_id,
global.attributes.clone(),
false,
);

// Add the statement to the scope so its path can be looked up later
Expand Down
8 changes: 8 additions & 0 deletions noir/noir-repo/compiler/noirc_arena/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

use std::fmt;

#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)]
pub struct Index(usize);

Expand All @@ -25,6 +27,12 @@ impl Index {
}
}

impl fmt::Display for Index {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.0.fmt(f)
}
}

#[derive(Clone, Debug)]
pub struct Arena<T> {
pub vec: Vec<T>,
Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ pub fn check_crate(
let mut errors = vec![];
let diagnostics = CrateDefMap::collect_defs(crate_id, context, macros);
errors.extend(diagnostics.into_iter().map(|(error, file_id)| {
let diagnostic: CustomDiagnostic = error.into();
let diagnostic = CustomDiagnostic::from(&error);
diagnostic.in_file(file_id)
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1267,8 +1267,11 @@ impl<'block> BrilligBlock<'block> {
dfg: &DataFlowGraph,
result_variable: SingleAddrVariable,
) {
let binary_type =
type_of_binary_operation(dfg[binary.lhs].get_type(), dfg[binary.rhs].get_type());
let binary_type = type_of_binary_operation(
dfg[binary.lhs].get_type(),
dfg[binary.rhs].get_type(),
binary.operator,
);

let left = self.convert_ssa_single_addr_value(binary.lhs, dfg);
let right = self.convert_ssa_single_addr_value(binary.rhs, dfg);
Expand Down Expand Up @@ -1754,7 +1757,7 @@ impl<'block> BrilligBlock<'block> {
}

/// Returns the type of the operation considering the types of the operands
pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type {
pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type, op: BinaryOp) -> Type {
match (lhs_type, rhs_type) {
(_, Type::Function) | (Type::Function, _) => {
unreachable!("Functions are invalid in binary operations")
Expand All @@ -1770,12 +1773,15 @@ pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type
}
// If both sides are numeric type, then we expect their types to be
// the same.
(Type::Numeric(lhs_type), Type::Numeric(rhs_type)) => {
(Type::Numeric(lhs_type), Type::Numeric(rhs_type))
if op != BinaryOp::Shl && op != BinaryOp::Shr =>
{
assert_eq!(
lhs_type, rhs_type,
"lhs and rhs types in a binary operation are always the same but got {lhs_type} and {rhs_type}"
);
Type::Numeric(*lhs_type)
}
_ => lhs_type.clone(),
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,6 @@ impl BrilligContext {
result: SingleAddrVariable,
operation: BrilligBinaryOp,
) {
assert!(
lhs.bit_size == rhs.bit_size,
"Not equal bit size for lhs and rhs: lhs {}, rhs {}",
lhs.bit_size,
rhs.bit_size
);
let is_field_op = lhs.bit_size == FieldElement::max_num_bits();
let expected_result_bit_size =
BrilligContext::binary_result_bit_size(operation, lhs.bit_size);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use noirc_frontend::monomorphization::ast::InlineType;
use crate::ssa::ir::{
basic_block::BasicBlockId,
function::{Function, FunctionId},
instruction::{Binary, BinaryOp, Instruction, TerminatorInstruction},
instruction::{Binary, BinaryOp, ErrorSelector, Instruction, TerminatorInstruction},
types::Type,
value::{Value, ValueId},
};
Expand All @@ -19,7 +19,7 @@ use super::{
basic_block::BasicBlock,
dfg::{CallStack, InsertInstructionResult},
function::RuntimeType,
instruction::{ConstrainError, ErrorSelector, ErrorType, InstructionId, Intrinsic},
instruction::{ConstrainError, ErrorType, InstructionId, Intrinsic},
},
ssa_gen::Ssa,
};
Expand Down Expand Up @@ -232,7 +232,12 @@ impl FunctionBuilder {
) -> ValueId {
let lhs_type = self.type_of_value(lhs);
let rhs_type = self.type_of_value(rhs);
assert_eq!(lhs_type, rhs_type, "ICE - Binary instruction operands must have the same type");
if operator != BinaryOp::Shl && operator != BinaryOp::Shr {
assert_eq!(
lhs_type, rhs_type,
"ICE - Binary instruction operands must have the same type"
);
}
let instruction = Instruction::Binary(Binary { lhs, rhs, operator });
self.insert_instruction(instruction, None).first()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl Context<'_> {
} else {
// we use a predicate to nullify the result in case of overflow
let bit_size_var =
self.numeric_constant(FieldElement::from(bit_size as u128), typ.clone());
self.numeric_constant(FieldElement::from(bit_size as u128), Type::unsigned(8));
let overflow = self.insert_binary(rhs, BinaryOp::Lt, bit_size_var);
let predicate = self.insert_cast(overflow, typ.clone());
// we can safely cast to unsigned because overflow_checks prevent bit-shift with a negative value
Expand Down
13 changes: 10 additions & 3 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
//!
//! Note that this pass also often creates superfluous jmp instructions in the
//! program that will need to be removed by a later simplify cfg pass.
//! Note also that unrolling is skipped for Brillig runtime and as a result
//! we remove reference count instructions because they are only used by Brillig bytecode
use std::collections::HashSet;

use crate::{
Expand All @@ -24,7 +26,7 @@ use crate::{
dom::DominatorTree,
function::{Function, RuntimeType},
function_inserter::FunctionInserter,
instruction::TerminatorInstruction,
instruction::{Instruction, TerminatorInstruction},
post_order::PostOrder,
value::ValueId,
},
Expand Down Expand Up @@ -465,9 +467,14 @@ impl<'f> LoopIteration<'f> {
// instances of the induction variable or any values that were changed as a result
// of the new induction variable value.
for instruction in instructions {
self.inserter.push_instruction(instruction, self.insert_block);
// Skip reference count instructions since they are only used for brillig, and brillig code is not unrolled
if !matches!(
self.dfg()[instruction],
Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. }
) {
self.inserter.push_instruction(instruction, self.insert_block);
}
}

let mut terminator = self.dfg()[self.source_block]
.unwrap_terminator()
.clone()
Expand Down
30 changes: 5 additions & 25 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ impl<'a> FunctionContext<'a> {
self.insert_safe_cast(result, result_type, location)
}
BinaryOpKind::ShiftLeft | BinaryOpKind::ShiftRight => {
self.check_shift_overflow(result, rhs, bit_size, location, true)
self.check_shift_overflow(result, rhs, bit_size, location)
}
_ => unreachable!("operator {} should not overflow", operator),
}
Expand Down Expand Up @@ -408,7 +408,7 @@ impl<'a> FunctionContext<'a> {
}
}

self.check_shift_overflow(result, rhs, bit_size, location, false);
self.check_shift_overflow(result, rhs, bit_size, location);
}

_ => unreachable!("operator {} should not overflow", operator),
Expand All @@ -430,32 +430,12 @@ impl<'a> FunctionContext<'a> {
rhs: ValueId,
bit_size: u32,
location: Location,
is_signed: bool,
) -> ValueId {
let one = self.builder.numeric_constant(FieldElement::one(), Type::bool());
let rhs = if is_signed {
self.insert_safe_cast(rhs, Type::unsigned(bit_size), location)
} else {
rhs
};
// Bit-shift with a negative number is an overflow
if is_signed {
// We compute the sign of rhs.
let half_width = self.builder.numeric_constant(
FieldElement::from(2_i128.pow(bit_size - 1)),
Type::unsigned(bit_size),
);
let sign = self.builder.insert_binary(rhs, BinaryOp::Lt, half_width);
self.builder.set_location(location).insert_constrain(
sign,
one,
Some("attempt to bit-shift with overflow".to_owned().into()),
);
}
assert!(self.builder.current_function.dfg.type_of_value(rhs) == Type::unsigned(8));

let max = self
.builder
.numeric_constant(FieldElement::from(bit_size as i128), Type::unsigned(bit_size));
let max =
self.builder.numeric_constant(FieldElement::from(bit_size as i128), Type::unsigned(8));
let overflow = self.builder.insert_binary(rhs, BinaryOp::Lt, max);
self.builder.set_location(location).insert_constrain(
overflow,
Expand Down
7 changes: 4 additions & 3 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub enum StatementKind {
Break,
Continue,
/// This statement should be executed at compile-time
Comptime(Box<StatementKind>),
Comptime(Box<Statement>),
// This is an expression with a trailing semi-colon
Semi(Expression),
// This statement is the result of a recovered parse error.
Expand Down Expand Up @@ -486,7 +486,8 @@ impl Pattern {
pub fn name_ident(&self) -> &Ident {
match self {
Pattern::Identifier(name_ident) => name_ident,
_ => panic!("only the identifier pattern can return a name"),
Pattern::Mutable(pattern, ..) => pattern.name_ident(),
_ => panic!("Only the Identifier or Mutable patterns can return a name"),
}
}

Expand Down Expand Up @@ -685,7 +686,7 @@ impl Display for StatementKind {
StatementKind::For(for_loop) => for_loop.fmt(f),
StatementKind::Break => write!(f, "break"),
StatementKind::Continue => write!(f, "continue"),
StatementKind::Comptime(statement) => write!(f, "comptime {statement}"),
StatementKind::Comptime(statement) => write!(f, "comptime {}", statement.kind),
StatementKind::Semi(semi) => write!(f, "{semi};"),
StatementKind::Error => write!(f, "Error"),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use super::value::Value;
/// The possible errors that can halt the interpreter.
#[derive(Debug, Clone)]
pub enum InterpreterError {
ArgumentCountMismatch { expected: usize, actual: usize, call_location: Location },
ArgumentCountMismatch { expected: usize, actual: usize, location: Location },
TypeMismatch { expected: Type, value: Value, location: Location },
NonComptimeVarReferenced { name: String, location: Location },
IntegerOutOfRangeForType { value: FieldElement, typ: Type, location: Location },
Expand Down Expand Up @@ -60,7 +60,7 @@ impl InterpreterError {

pub fn get_location(&self) -> Location {
match self {
InterpreterError::ArgumentCountMismatch { call_location: location, .. }
InterpreterError::ArgumentCountMismatch { location, .. }
| InterpreterError::TypeMismatch { location, .. }
| InterpreterError::NonComptimeVarReferenced { location, .. }
| InterpreterError::IntegerOutOfRangeForType { location, .. }
Expand Down Expand Up @@ -98,20 +98,20 @@ impl InterpreterError {
}
}

impl From<InterpreterError> for CustomDiagnostic {
fn from(error: InterpreterError) -> Self {
impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
fn from(error: &'a InterpreterError) -> Self {
match error {
InterpreterError::ArgumentCountMismatch { expected, actual, call_location } => {
InterpreterError::ArgumentCountMismatch { expected, actual, location } => {
let only = if expected > actual { "only " } else { "" };
let plural = if expected == 1 { "" } else { "s" };
let was_were = if actual == 1 { "was" } else { "were" };
let plural = if *expected == 1 { "" } else { "s" };
let was_were = if *actual == 1 { "was" } else { "were" };
let msg = format!(
"Expected {expected} argument{plural}, but {only}{actual} {was_were} provided"
);

let few_many = if actual < expected { "few" } else { "many" };
let secondary = format!("Too {few_many} arguments");
CustomDiagnostic::simple_error(msg, secondary, call_location.span)
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
InterpreterError::TypeMismatch { expected, value, location } => {
let typ = value.get_type();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl StmtId {
HirStatement::Semi(expr) => StatementKind::Semi(expr.to_ast(interner)),
HirStatement::Error => StatementKind::Error,
HirStatement::Comptime(statement) => {
StatementKind::Comptime(Box::new(statement.to_ast(interner).kind))
StatementKind::Comptime(Box::new(statement.to_ast(interner)))
}
};

Expand Down
Loading
Loading