Skip to content

Commit

Permalink
feat(perf): Allow array set last uses optimization in return block of…
Browse files Browse the repository at this point in the history
… Brillig functions (noir-lang/noir#6119)

feat: represent assertions more similarly to function calls (noir-lang/noir#6103)
feat: pretty print Quoted token stream (noir-lang/noir#6111)
feat: LSP autocompletion for `TypePath` (noir-lang/noir#6117)
fix: disambiguate field or int static trait method call (noir-lang/noir#6112)
chore: delete duplicated test (noir-lang/noir#6113)
feat: (LSP) suggest $vars inside `quote { ... }` (noir-lang/noir#6114)
chore: ec addition for non-zero points (noir-lang/noir#5858)
chore(docs): removing old versions (noir-lang/noir#6075)
fix(mem2reg): Remove possibility of underflow (noir-lang/noir#6107)
fix: decode databus return values (noir-lang/noir#6095)
  • Loading branch information
AztecBot committed Sep 21, 2024
2 parents c03730a + bd72a30 commit 40623e2
Show file tree
Hide file tree
Showing 1,578 changed files with 1,543 additions and 127,154 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
aea5cc789ccf4a4d16b1d238d99474f37920b37e
5598059576c6cbc72474aff4b18bc5e4bb9f08e1
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,15 @@ pub enum BlackBoxFunc {
/// ultimately fail.
RecursiveAggregation,

/// Addition over the embedded curve on which the witness is defined.
/// Addition over the embedded curve on which the witness is defined
/// The opcode makes the following assumptions but does not enforce them because
/// it is more efficient to do it only when required. For instance, adding two
/// points that are on the curve it guarantee to give a point on the curve.
///
/// It assumes that the points are on the curve.
/// If the inputs are the same witnesses index, it will perform a doubling,
/// If not, it assumes that the points' x-coordinates are not equal.
/// It also assumes neither point is the infinity point.
EmbeddedCurveAdd,

/// BigInt addition
Expand Down
44 changes: 25 additions & 19 deletions noir/noir-repo/aztec_macros/src/transforms/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,14 +316,17 @@ fn create_static_check(fname: &str, is_private: bool) -> Statement {
.iter()
.fold(variable("context"), |acc, member| member_access(acc, member))
};
make_statement(StatementKind::Constrain(ConstrainStatement(
make_eq(is_static_call_expr, expression(ExpressionKind::Literal(Literal::Bool(true)))),
Some(expression(ExpressionKind::Literal(Literal::Str(format!(
"Function {} can only be called statically",
fname
))))),
ConstrainKind::Assert,
)))
make_statement(StatementKind::Constrain(ConstrainStatement {
kind: ConstrainKind::Assert,
arguments: vec![
make_eq(is_static_call_expr, expression(ExpressionKind::Literal(Literal::Bool(true)))),
expression(ExpressionKind::Literal(Literal::Str(format!(
"Function {} can only be called statically",
fname
)))),
],
span: Default::default(),
}))
}

/// Creates a check for internal functions ensuring that the caller is self.
Expand All @@ -332,17 +335,20 @@ fn create_static_check(fname: &str, is_private: bool) -> Statement {
/// assert(context.msg_sender() == context.this_address(), "Function can only be called internally");
/// ```
fn create_internal_check(fname: &str) -> Statement {
make_statement(StatementKind::Constrain(ConstrainStatement(
make_eq(
method_call(variable("context"), "msg_sender", vec![]),
method_call(variable("context"), "this_address", vec![]),
),
Some(expression(ExpressionKind::Literal(Literal::Str(format!(
"Function {} can only be called internally",
fname
))))),
ConstrainKind::Assert,
)))
make_statement(StatementKind::Constrain(ConstrainStatement {
kind: ConstrainKind::Assert,
arguments: vec![
make_eq(
method_call(variable("context"), "msg_sender", vec![]),
method_call(variable("context"), "this_address", vec![]),
),
expression(ExpressionKind::Literal(Literal::Str(format!(
"Function {} can only be called internally",
fname
)))),
],
span: Default::default(),
}))
}

/// Creates a call to assert_initialization_matches_address_preimage to be inserted
Expand Down
5 changes: 1 addition & 4 deletions noir/noir-repo/aztec_macros/src/utils/parse_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,7 @@ fn empty_statement(statement: &mut Statement) {
}

fn empty_constrain_statement(constrain_statement: &mut ConstrainStatement) {
empty_expression(&mut constrain_statement.0);
if let Some(expression) = &mut constrain_statement.1 {
empty_expression(expression);
}
empty_expressions(&mut constrain_statement.arguments);
}

fn empty_expressions(expressions: &mut [Expression]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ impl<'block> BrilligBlock<'block> {

self.brillig_context.deallocate_register(items_pointer);
}
Instruction::ArraySet { array, index, value, mutable: _ } => {
Instruction::ArraySet { array, index, value, mutable } => {
let source_variable = self.convert_ssa_value(*array, dfg);
let index_register = self.convert_ssa_single_addr_value(*index, dfg);
let value_variable = self.convert_ssa_value(*value, dfg);
Expand All @@ -688,6 +688,7 @@ impl<'block> BrilligBlock<'block> {
destination_variable,
index_register,
value_variable,
*mutable,
);
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
Expand Down Expand Up @@ -868,34 +869,49 @@ impl<'block> BrilligBlock<'block> {
destination_variable: BrilligVariable,
index_register: SingleAddrVariable,
value_variable: BrilligVariable,
mutable: bool,
) {
assert!(index_register.bit_size == BRILLIG_MEMORY_ADDRESSING_BIT_SIZE);
match (source_variable, destination_variable) {
(
BrilligVariable::BrilligArray(source_array),
BrilligVariable::BrilligArray(destination_array),
) => {
self.brillig_context.call_array_copy_procedure(source_array, destination_array);
if !mutable {
self.brillig_context.call_array_copy_procedure(source_array, destination_array);
}
}
(
BrilligVariable::BrilligVector(source_vector),
BrilligVariable::BrilligVector(destination_vector),
) => {
self.brillig_context.call_vector_copy_procedure(source_vector, destination_vector);
if !mutable {
self.brillig_context
.call_vector_copy_procedure(source_vector, destination_vector);
}
}
_ => unreachable!("ICE: array set on non-array"),
}

let destination_for_store = if mutable { source_variable } else { destination_variable };
// Then set the value in the newly created array
let items_pointer =
self.brillig_context.codegen_make_array_or_vector_items_pointer(destination_variable);
self.brillig_context.codegen_make_array_or_vector_items_pointer(destination_for_store);

self.brillig_context.codegen_store_with_offset(
items_pointer,
index_register,
value_variable.extract_register(),
);

// If we mutated the source array we want instructions that use the destination array to point to the source array
if mutable {
self.brillig_context.mov_instruction(
destination_variable.extract_register(),
source_variable.extract_register(),
);
}

self.brillig_context.deallocate_register(items_pointer);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1471,6 +1471,7 @@ impl<F: AcirField> AcirContext<F> {
| BlackBoxFunc::AND
| BlackBoxFunc::XOR
| BlackBoxFunc::AES128Encrypt
| BlackBoxFunc::EmbeddedCurveAdd
);
// Convert `AcirVar` to `FunctionInput`
let inputs = self.prepare_inputs_for_black_box_func_call(inputs, allow_constant_inputs)?;
Expand Down
11 changes: 11 additions & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,17 @@ impl Function {
let returns = vecmap(self.returns(), |ret| self.dfg.type_of_value(*ret));
Signature { params, returns }
}

/// Finds the block of the function with the Return instruction
pub(crate) fn find_last_block(&self) -> BasicBlockId {
for block in self.reachable_blocks() {
if matches!(self.dfg[block].terminator(), Some(TerminatorInstruction::Return { .. })) {
return block;
}
}

unreachable!("SSA Function {} has no reachable return instruction!", self.id())
}
}

impl std::fmt::Display for RuntimeType {
Expand Down
15 changes: 9 additions & 6 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/array_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ impl Ssa {
#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn array_set_optimization(mut self) -> Self {
for func in self.functions.values_mut() {
if !func.runtime().is_entry_point() {
let mut reachable_blocks = func.reachable_blocks();
let mut reachable_blocks = func.reachable_blocks();
let block = if !func.runtime().is_entry_point() {
assert_eq!(reachable_blocks.len(), 1, "Expected there to be 1 block remaining in Acir function for array_set optimization");
reachable_blocks.pop_first().unwrap()
} else {
// We only apply the array set optimization in the return block of Brillig functions
func.find_last_block()
};

let block = reachable_blocks.pop_first().unwrap();
let instructions_to_update = analyze_last_uses(&func.dfg, block);
make_mutable(&mut func.dfg, block, instructions_to_update);
}
let instructions_to_update = analyze_last_uses(&func.dfg, block);
make_mutable(&mut func.dfg, block, instructions_to_update);
}
self
}
Expand Down
19 changes: 2 additions & 17 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use std::collections::{HashMap, HashSet};

use crate::ssa::{
ir::{
basic_block::BasicBlockId,
function::Function,
instruction::{Instruction, InstructionId, TerminatorInstruction},
instruction::{Instruction, InstructionId},
types::Type,
value::ValueId,
},
Expand Down Expand Up @@ -107,7 +106,7 @@ impl Context {
/// Find each dec_rc instruction and if the most recent inc_rc instruction for the same value
/// is not possibly mutated, then we can remove them both. Returns each such pair.
fn find_rcs_to_remove(&mut self, function: &Function) -> HashSet<InstructionId> {
let last_block = Self::find_last_block(function);
let last_block = function.find_last_block();
let mut to_remove = HashSet::new();

for instruction in function.dfg[last_block].instructions() {
Expand All @@ -124,20 +123,6 @@ impl Context {
to_remove
}

/// Finds the block of the function with the Return instruction
fn find_last_block(function: &Function) -> BasicBlockId {
for block in function.reachable_blocks() {
if matches!(
function.dfg[block].terminator(),
Some(TerminatorInstruction::Return { .. })
) {
return block;
}
}

unreachable!("SSA Function {} has no reachable return instruction!", function.id())
}

/// Finds and pops the IncRc for the given array value if possible.
fn pop_rc_for(&mut self, value: ValueId, function: &Function) -> Option<IncRc> {
let typ = function.dfg.type_of_value(value);
Expand Down
47 changes: 40 additions & 7 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,27 @@ pub enum LValue {
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub struct ConstrainStatement(pub Expression, pub Option<Expression>, pub ConstrainKind);
pub struct ConstrainStatement {
pub kind: ConstrainKind,
pub arguments: Vec<Expression>,
pub span: Span,
}

impl Display for ConstrainStatement {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self.kind {
ConstrainKind::Assert | ConstrainKind::AssertEq => write!(
f,
"{}({})",
self.kind,
vecmap(&self.arguments, |arg| arg.to_string()).join(", ")
),
ConstrainKind::Constrain => {
write!(f, "constrain {}", &self.arguments[0])
}
}
}
}

#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub enum ConstrainKind {
Expand All @@ -571,6 +591,25 @@ pub enum ConstrainKind {
Constrain,
}

impl ConstrainKind {
pub fn required_arguments_count(&self) -> usize {
match self {
ConstrainKind::Assert | ConstrainKind::Constrain => 1,
ConstrainKind::AssertEq => 2,
}
}
}

impl Display for ConstrainKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ConstrainKind::Assert => write!(f, "assert"),
ConstrainKind::AssertEq => write!(f, "assert_eq"),
ConstrainKind::Constrain => write!(f, "constrain"),
}
}
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub enum Pattern {
Identifier(Ident),
Expand Down Expand Up @@ -885,12 +924,6 @@ impl Display for LetStatement {
}
}

impl Display for ConstrainStatement {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "constrain {}", self.0)
}
}

impl Display for AssignStatement {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{} = {}", self.lvalue, self.expression)
Expand Down
6 changes: 1 addition & 5 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1117,11 +1117,7 @@ impl ConstrainStatement {
}

pub fn accept_children(&self, visitor: &mut impl Visitor) {
self.0.accept(visitor);

if let Some(exp) = &self.1 {
exp.accept(visitor);
}
visit_expressions(&self.arguments, visitor);
}
}

Expand Down
Loading

0 comments on commit 40623e2

Please sign in to comment.