Skip to content

Commit

Permalink
fix: prevent panic within remove_possibly_mutated_cached_make_arrays (
Browse files Browse the repository at this point in the history
noir-lang/noir#7264)

chore: early check type equality in try_unify (noir-lang/noir#7263)
feat(LSP): suggest enum variants without parameters (noir-lang/noir#7261)
feat(experimental): Parse match expressions (noir-lang/noir#7243)
feat(experimental): Implement zeroed for enums (noir-lang/noir#7252)
fix(ssa): Only attempt to inline constant Brillig calls for entry points (noir-lang/noir#7260)
fix: Add missing `is_empty` check for enums (noir-lang/noir#7257)
feat(experimental): Implement enum tag constants (noir-lang/noir#7183)
fix(unrolling): Fetch original bytecode size from the original function (noir-lang/noir#7253)
fix(ssa): Use number of SSA instructions for the Brillig unrolling bytecode size limit (noir-lang/noir#7242)
feat: Sync from aztec-packages (noir-lang/noir#7241)
chore: bump gates diff (noir-lang/noir#7245)
feat: simplify subtraction from self to return zero (noir-lang/noir#7189)
feat: allow specifying multiple patterns in nargo test (noir-lang/noir#7186)
fix: Avoid type error when calling something with a type alias of a function (noir-lang/noir#7239)
feat: Allow resolved types in constructors (noir-lang/noir#7223)
chore: clarify to_radix docs examples (noir-lang/noir#7230)
chore: fix struct example (noir-lang/noir#7198)
feat(optimization): Add purity analysis to SSA (noir-lang/noir#7197)
chore: start tracking time to run critical library tests (noir-lang/noir#7221)
chore: Rework defunctionalize pass to not rely on DFG bugs (noir-lang/noir#7222)
fix(brillig): Globals entry point reachability analysis  (noir-lang/noir#7188)
chore: update docs to use devcontainer feature (noir-lang/noir#7206)
chore(ci): Add test for global vars entry points regression (noir-lang/noir#7209)
chore(ssa): Flip the SSA Brillig constraint check to off by default (noir-lang/noir#7211)
chore(docs): moving references to noir-starter to awesome-noir (noir-lang/noir#7203)
chore: build docs in the merge queue (noir-lang/noir#7218)
fix: correct reversed callstacks (noir-lang/noir#7212)
chore: exclude dependency fetching time from benchmarks (noir-lang/noir#7210)
feat(experimental): Support enums in comptime code (noir-lang/noir#7194)
  • Loading branch information
AztecBot committed Feb 4, 2025
2 parents 6a98665 + 4d5f9e3 commit e903406
Show file tree
Hide file tree
Showing 21 changed files with 583 additions and 141 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
a9e985064303b0843cbf68fb5a9d41f9ade1e30d
130d99125a09110a3ee3e877d88d83b5aa37f369
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ pub(crate) fn gen_brillig_for(
brillig: &Brillig,
) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
// Create the entry point artifact
let globals_memory_size = brillig.globals_memory_size.get(&func.id()).copied().unwrap_or(0);
let globals_memory_size = brillig
.globals_memory_size
.get(&func.id())
.copied()
.expect("Should have the globals memory size specified for an entry point");
let mut entry_point = BrilligContext::new_entry_point_artifact(
arguments,
FunctionContext::return_values(func),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,20 +182,11 @@ impl BrilligGlobals {
&self,
brillig_function_id: FunctionId,
) -> SsaToBrilligGlobals {
let mut globals_allocations = HashMap::default();

// First check whether the `brillig_function_id` is itself an entry point,
// if so we can fetch the global allocations directly from `self.entry_point_globals_map`.
if let Some(globals) = self.entry_point_globals_map.get(&brillig_function_id) {
globals_allocations.extend(globals);
return globals_allocations;
}

// If the Brillig function we are compiling is not an entry point, we should search
// for the entry point which triggers the given function.
let entry_points = self.inner_call_to_entry_point.get(&brillig_function_id);

let mut globals_allocations = HashMap::default();
if let Some(entry_points) = entry_points {
// A Brillig function can be used by multiple entry points. Fetch both globals allocations
// A Brillig function is used by multiple entry points. Fetch both globals allocations
// in case one is used by the internal call.
let entry_point_allocations = entry_points
.iter()
Expand All @@ -204,6 +195,11 @@ impl BrilligGlobals {
for map in entry_point_allocations {
globals_allocations.extend(map);
}
} else if let Some(globals) = self.entry_point_globals_map.get(&brillig_function_id) {
// If there is no mapping from an inner call to an entry point, that means `brillig_function_id`
// is itself an entry point and we can fetch the global allocations directly from `self.entry_point_globals_map`.
// vec![globals]
globals_allocations.extend(globals);
} else {
unreachable!(
"ICE: Expected global allocation to be set for function {brillig_function_id}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,49 @@ impl Ssa {
function.constant_fold(false, brillig_info);
}

// It could happen that we inlined all calls to a given brillig function.
// In that case it's unused so we can remove it. This is what we check next.
self.remove_unused_brillig_functions(brillig_functions)
}

fn remove_unused_brillig_functions(
mut self,
mut brillig_functions: BTreeMap<FunctionId, Function>,
) -> Ssa {
// Remove from the above map functions that are called
for function in self.functions.values() {
for block_id in function.reachable_blocks() {
for instruction_id in function.dfg[block_id].instructions() {
let instruction = &function.dfg[*instruction_id];
let Instruction::Call { func: func_id, arguments: _ } = instruction else {
continue;
};

let func_value = &function.dfg[*func_id];
let Value::Function(func_id) = func_value else { continue };

if function.runtime().is_acir() {
brillig_functions.remove(func_id);
}
}
}
}

// The ones that remain are never called: let's remove them.
for (func_id, func) in &brillig_functions {
// We never want to remove the main function (it could be `unconstrained` or it
// could have been turned into brillig if `--force-brillig` was given).
// We also don't want to remove entry points.
let runtime = func.runtime();
if self.main_id == *func_id
|| (runtime.is_entry_point() && matches!(runtime, RuntimeType::Acir(_)))
{
continue;
}

self.functions.remove(func_id);
}

self
}
}
Expand Down Expand Up @@ -682,6 +725,11 @@ impl<'brillig> Context<'brillig> {

// Should we consider calls to slice_push_back and similar to be mutating operations as well?
if let Store { value: array, .. } | ArraySet { array, .. } = instruction {
if function.dfg.is_global(*array) {
// Early return as we expect globals to be immutable.
return;
};

let instruction = match &function.dfg[*array] {
Value::Instruction { instruction, .. } => &function.dfg[*instruction],
_ => return,
Expand Down Expand Up @@ -1533,6 +1581,82 @@ mod test {
assert_normalized_ssa_equals(ssa, expected);
}

#[test]
fn inlines_brillig_call_with_entry_point_globals() {
let src = "
g0 = Field 2
acir(inline) fn main f0 {
b0():
v1 = call f1() -> Field
return v1
}
brillig(inline) fn one f1 {
b0():
v1 = add g0, Field 3
return v1
}
";
let ssa = Ssa::from_str(src).unwrap();
let mut ssa = ssa.dead_instruction_elimination();
let used_globals_map = std::mem::take(&mut ssa.used_globals);
let brillig = ssa.to_brillig_with_globals(false, used_globals_map);

let expected = "
g0 = Field 2
acir(inline) fn main f0 {
b0():
return Field 5
}
";

let ssa = ssa.fold_constants_with_brillig(&brillig);
assert_normalized_ssa_equals(ssa, expected);
}

#[test]
fn inlines_brillig_call_with_non_entry_point_globals() {
let src = "
g0 = Field 2
acir(inline) fn main f0 {
b0():
v1 = call f1() -> Field
return v1
}
brillig(inline) fn entry_point f1 {
b0():
v1 = call f2() -> Field
return v1
}
brillig(inline) fn one f2 {
b0():
v1 = add g0, Field 3
return v1
}
";
let ssa = Ssa::from_str(src).unwrap();
let mut ssa = ssa.dead_instruction_elimination();
let used_globals_map = std::mem::take(&mut ssa.used_globals);
let brillig = ssa.to_brillig_with_globals(false, used_globals_map);

let expected = "
g0 = Field 2
acir(inline) fn main f0 {
b0():
return Field 5
}
";

let ssa = ssa.fold_constants_with_brillig(&brillig);
assert_normalized_ssa_equals(ssa, expected);
}

#[test]
fn does_not_use_cached_constrain_in_block_that_is_not_dominated() {
let src = "
Expand Down
18 changes: 18 additions & 0 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub enum ExpressionKind {
Cast(Box<CastExpression>),
Infix(Box<InfixExpression>),
If(Box<IfExpression>),
Match(Box<MatchExpression>),
Variable(Path),
Tuple(Vec<Expression>),
Lambda(Box<Lambda>),
Expand Down Expand Up @@ -465,6 +466,12 @@ pub struct IfExpression {
pub alternative: Option<Expression>,
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub struct MatchExpression {
pub expression: Expression,
pub rules: Vec<(/*pattern*/ Expression, /*branch*/ Expression)>,
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub struct Lambda {
pub parameters: Vec<(Pattern, UnresolvedType)>,
Expand Down Expand Up @@ -612,6 +619,7 @@ impl Display for ExpressionKind {
Cast(cast) => cast.fmt(f),
Infix(infix) => infix.fmt(f),
If(if_expr) => if_expr.fmt(f),
Match(match_expr) => match_expr.fmt(f),
Variable(path) => path.fmt(f),
Constructor(constructor) => constructor.fmt(f),
MemberAccess(access) => access.fmt(f),
Expand Down Expand Up @@ -790,6 +798,16 @@ impl Display for IfExpression {
}
}

impl Display for MatchExpression {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "match {} {{", self.expression)?;
for (pattern, branch) in &self.rules {
writeln!(f, " {pattern} -> {branch},")?;
}
write!(f, "}}")
}
}

impl Display for Lambda {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let parameters = vecmap(&self.parameters, |(name, r#type)| format!("{name}: {type}"));
Expand Down
25 changes: 24 additions & 1 deletion noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{

use super::{
ForBounds, FunctionReturnType, GenericTypeArgs, IntegerBitSize, ItemVisibility,
NoirEnumeration, Pattern, Signedness, TraitBound, TraitImplItemKind, TypePath,
MatchExpression, NoirEnumeration, Pattern, Signedness, TraitBound, TraitImplItemKind, TypePath,
UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
UnresolvedTypeExpression,
};
Expand Down Expand Up @@ -222,6 +222,10 @@ pub trait Visitor {
true
}

fn visit_match_expression(&mut self, _: &MatchExpression, _: Span) -> bool {
true
}

fn visit_tuple(&mut self, _: &[Expression], _: Span) -> bool {
true
}
Expand Down Expand Up @@ -866,6 +870,9 @@ impl Expression {
ExpressionKind::If(if_expression) => {
if_expression.accept(self.span, visitor);
}
ExpressionKind::Match(match_expression) => {
match_expression.accept(self.span, visitor);
}
ExpressionKind::Tuple(expressions) => {
if visitor.visit_tuple(expressions, self.span) {
visit_expressions(expressions, visitor);
Expand Down Expand Up @@ -1073,6 +1080,22 @@ impl IfExpression {
}
}

impl MatchExpression {
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
if visitor.visit_match_expression(self, span) {
self.accept_children(visitor);
}
}

pub fn accept_children(&self, visitor: &mut impl Visitor) {
self.expression.accept(visitor);
for (pattern, branch) in &self.rules {
pattern.accept(visitor);
branch.accept(visitor);
}
}
}

impl Lambda {
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
if visitor.visit_lambda(self, span) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ use crate::{
ast::{
ArrayLiteral, BlockExpression, CallExpression, CastExpression, ConstructorExpression,
Expression, ExpressionKind, Ident, IfExpression, IndexExpression, InfixExpression,
ItemVisibility, Lambda, Literal, MemberAccessExpression, MethodCallExpression, Path,
PathSegment, PrefixExpression, StatementKind, UnaryOp, UnresolvedTypeData,
UnresolvedTypeExpression,
ItemVisibility, Lambda, Literal, MatchExpression, MemberAccessExpression,
MethodCallExpression, Path, PathSegment, PrefixExpression, StatementKind, UnaryOp,
UnresolvedTypeData, UnresolvedTypeExpression,
},
hir::{
comptime::{self, InterpreterError},
Expand Down Expand Up @@ -51,6 +51,7 @@ impl<'context> Elaborator<'context> {
ExpressionKind::Cast(cast) => self.elaborate_cast(*cast, expr.span),
ExpressionKind::Infix(infix) => return self.elaborate_infix(*infix, expr.span),
ExpressionKind::If(if_) => self.elaborate_if(*if_),
ExpressionKind::Match(match_) => self.elaborate_match(*match_),
ExpressionKind::Variable(variable) => return self.elaborate_variable(variable),
ExpressionKind::Tuple(tuple) => self.elaborate_tuple(tuple),
ExpressionKind::Lambda(lambda) => self.elaborate_lambda(*lambda, None),
Expand Down Expand Up @@ -926,6 +927,10 @@ impl<'context> Elaborator<'context> {
(HirExpression::If(if_expr), ret_type)
}

fn elaborate_match(&mut self, _match_expr: MatchExpression) -> (HirExpression, Type) {
(HirExpression::Error, Type::Error)
}

fn elaborate_tuple(&mut self, tuple: Vec<Expression>) -> (HirExpression, Type) {
let mut element_ids = Vec::with_capacity(tuple.len());
let mut element_types = Vec::with_capacity(tuple.len());
Expand Down
15 changes: 12 additions & 3 deletions noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use crate::{
ArrayLiteral, AsTraitPath, AssignStatement, BlockExpression, CallExpression,
CastExpression, ConstrainStatement, ConstructorExpression, Expression, ExpressionKind,
ForBounds, ForLoopStatement, ForRange, GenericTypeArgs, IfExpression, IndexExpression,
InfixExpression, LValue, Lambda, LetStatement, Literal, MemberAccessExpression,
MethodCallExpression, Pattern, PrefixExpression, Statement, StatementKind, UnresolvedType,
UnresolvedTypeData,
InfixExpression, LValue, Lambda, LetStatement, Literal, MatchExpression,
MemberAccessExpression, MethodCallExpression, Pattern, PrefixExpression, Statement,
StatementKind, UnresolvedType, UnresolvedTypeData,
},
hir_def::traits::TraitConstraint,
node_interner::{InternedStatementKind, NodeInterner},
Expand Down Expand Up @@ -241,6 +241,7 @@ impl<'interner> TokenPrettyPrinter<'interner> {
| Token::GreaterEqual
| Token::Equal
| Token::NotEqual
| Token::FatArrow
| Token::Arrow => write!(f, " {token} "),
Token::Assign => {
if last_was_op {
Expand Down Expand Up @@ -602,6 +603,14 @@ fn remove_interned_in_expression_kind(
.alternative
.map(|alternative| remove_interned_in_expression(interner, alternative)),
})),
ExpressionKind::Match(match_expr) => ExpressionKind::Match(Box::new(MatchExpression {
expression: remove_interned_in_expression(interner, match_expr.expression),
rules: vecmap(match_expr.rules, |(pattern, branch)| {
let pattern = remove_interned_in_expression(interner, pattern);
let branch = remove_interned_in_expression(interner, branch);
(pattern, branch)
}),
})),
ExpressionKind::Variable(_) => expr,
ExpressionKind::Tuple(expressions) => ExpressionKind::Tuple(vecmap(expressions, |expr| {
remove_interned_in_expression(interner, expr)
Expand Down
Loading

0 comments on commit e903406

Please sign in to comment.