Skip to content

Commit

Permalink
feat: Add #[inline(tag)] attribute and codegen (#4913)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #4910 

## Summary\*

We add a new attribute `Inline(String)`. Currently we only support one
`InlineType` variant of `Never`. This PR also moves `InlineType` into
the monomorphization ast as its functionality is expected to be shared
across various frontend passes as well as the SSA/ACIR gen.

## Additional Context

I know there is some plans to split the AST off into its own crate as
per (#4852). This change
shouldn't affect make the split much more dififcult as the evaluator
already depends on the AST as the issue mentions and this `InlineType`
is pretty isolated in its usage.

## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [X] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
  • Loading branch information
vezenovm and TomAFrench committed Apr 25, 2024
1 parent 7ea0e91 commit 1ec9cdc
Show file tree
Hide file tree
Showing 19 changed files with 240 additions and 77 deletions.
66 changes: 40 additions & 26 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use crate::brillig::brillig_ir::artifact::{BrilligParameter, GeneratedBrillig};
use crate::brillig::brillig_ir::BrilligContext;
use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
use crate::ssa::ir::function::InlineType;
pub(crate) use acir_ir::generated_acir::GeneratedAcir;
use noirc_frontend::monomorphization::ast::InlineType;

use acvm::acir::circuit::brillig::BrilligBytecode;
use acvm::acir::circuit::OpcodeLocation;
Expand Down Expand Up @@ -385,12 +385,12 @@ impl<'a> Context<'a> {
match function.runtime() {
RuntimeType::Acir(inline_type) => {
match inline_type {
InlineType::Fold => {}
InlineType::Inline => {
if function.id() != ssa.main_id {
panic!("ACIR function should have been inlined earlier if not marked otherwise");
}
}
InlineType::Fold | InlineType::Never => {}
}
// We only want to convert entry point functions. This being `main` and those marked with `InlineType::Fold`
Ok(Some(self.convert_acir_main(function, ssa, brillig)?))
Expand Down Expand Up @@ -2610,36 +2610,33 @@ mod test {
},
FieldElement,
};
use noirc_frontend::monomorphization::ast::InlineType;

use crate::{
brillig::Brillig,
ssa::{
acir_gen::acir_ir::generated_acir::BrilligStdlibFunc,
function_builder::FunctionBuilder,
ir::{
function::{FunctionId, InlineType},
instruction::BinaryOp,
map::Id,
types::Type,
},
ir::{function::FunctionId, instruction::BinaryOp, map::Id, types::Type},
},
};

fn build_basic_foo_with_return(
builder: &mut FunctionBuilder,
foo_id: FunctionId,
is_brillig_func: bool,
// `InlineType` can only exist on ACIR functions, so if the option is `None` we should generate a Brillig function
inline_type: Option<InlineType>,
) {
// fn foo f1 {
// b0(v0: Field, v1: Field):
// v2 = eq v0, v1
// constrain v2 == u1 0
// return v0
// }
if is_brillig_func {
builder.new_brillig_function("foo".into(), foo_id);
if let Some(inline_type) = inline_type {
builder.new_function("foo".into(), foo_id, inline_type);
} else {
builder.new_function("foo".into(), foo_id, InlineType::Fold);
builder.new_brillig_function("foo".into(), foo_id);
}
let foo_v0 = builder.add_parameter(Type::field());
let foo_v1 = builder.add_parameter(Type::field());
Expand All @@ -2650,8 +2647,25 @@ mod test {
builder.terminate_with_return(vec![foo_v0]);
}

/// Check that each `InlineType` which prevents inlining functions generates code in the same manner
#[test]
fn basic_call_with_outputs_assert() {
fn basic_calls_fold() {
basic_call_with_outputs_assert(InlineType::Fold);
call_output_as_next_call_input(InlineType::Fold);
basic_nested_call(InlineType::Fold);

call_output_as_next_call_input(InlineType::Never);
basic_nested_call(InlineType::Never);
basic_call_with_outputs_assert(InlineType::Never);
}

#[test]
#[should_panic]
fn call_without_inline_attribute() {
basic_call_with_outputs_assert(InlineType::Inline);
}

fn basic_call_with_outputs_assert(inline_type: InlineType) {
// acir(inline) fn main f0 {
// b0(v0: Field, v1: Field):
// v2 = call f1(v0, v1)
Expand Down Expand Up @@ -2679,7 +2693,7 @@ mod test {
builder.insert_constrain(main_call1_results[0], main_call2_results[0], None);
builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, false);
build_basic_foo_with_return(&mut builder, foo_id, Some(inline_type));

let ssa = builder.finish();

Expand Down Expand Up @@ -2745,8 +2759,7 @@ mod test {
}
}

#[test]
fn call_output_as_next_call_input() {
fn call_output_as_next_call_input(inline_type: InlineType) {
// acir(inline) fn main f0 {
// b0(v0: Field, v1: Field):
// v3 = call f1(v0, v1)
Expand Down Expand Up @@ -2775,7 +2788,7 @@ mod test {
builder.insert_constrain(main_call1_results[0], main_call2_results[0], None);
builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, false);
build_basic_foo_with_return(&mut builder, foo_id, Some(inline_type));

let ssa = builder.finish();

Expand All @@ -2794,8 +2807,7 @@ mod test {
check_call_opcode(&main_opcodes[1], 1, vec![Witness(2), Witness(1)], vec![Witness(3)]);
}

#[test]
fn basic_nested_call() {
fn basic_nested_call(inline_type: InlineType) {
// SSA for the following Noir program:
// fn main(x: Field, y: pub Field) {
// let z = func_with_nested_foo_call(x, y);
Expand Down Expand Up @@ -2851,7 +2863,7 @@ mod test {
builder.new_function(
"func_with_nested_foo_call".into(),
func_with_nested_foo_call_id,
InlineType::Fold,
inline_type,
);
let func_with_nested_call_v0 = builder.add_parameter(Type::field());
let func_with_nested_call_v1 = builder.add_parameter(Type::field());
Expand All @@ -2866,7 +2878,7 @@ mod test {
.to_vec();
builder.terminate_with_return(vec![foo_call[0]]);

build_basic_foo_with_return(&mut builder, foo_id, false);
build_basic_foo_with_return(&mut builder, foo_id, Some(inline_type));

let ssa = builder.finish();

Expand Down Expand Up @@ -2977,8 +2989,8 @@ mod test {
builder.insert_call(bar, vec![main_v0, main_v1], vec![Type::field()]).to_vec();
builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, true);
build_basic_foo_with_return(&mut builder, bar_id, true);
build_basic_foo_with_return(&mut builder, foo_id, None);
build_basic_foo_with_return(&mut builder, bar_id, None);

let ssa = builder.finish();
let brillig = ssa.to_brillig(false);
Expand Down Expand Up @@ -3106,7 +3118,7 @@ mod test {

builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, true);
build_basic_foo_with_return(&mut builder, foo_id, None);

let ssa = builder.finish();
// We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass.
Expand Down Expand Up @@ -3192,8 +3204,10 @@ mod test {

builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, true);
build_basic_foo_with_return(&mut builder, bar_id, false);
// Build a Brillig function
build_basic_foo_with_return(&mut builder, foo_id, None);
// Build an ACIR function which has the same logic as the Brillig function above
build_basic_foo_with_return(&mut builder, bar_id, Some(InlineType::Fold));

let ssa = builder.finish();
// We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass.
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::{borrow::Cow, rc::Rc};

use acvm::FieldElement;
use noirc_errors::Location;
use noirc_frontend::monomorphization::ast::InlineType;

use crate::ssa::ir::{
basic_block::BasicBlockId,
Expand All @@ -17,7 +18,7 @@ use super::{
ir::{
basic_block::BasicBlock,
dfg::{CallStack, InsertInstructionResult},
function::{InlineType, RuntimeType},
function::RuntimeType,
instruction::{ConstrainError, InstructionId, Intrinsic},
},
ssa_gen::Ssa,
Expand Down
27 changes: 2 additions & 25 deletions compiler/noirc_evaluator/src/ssa/ir/function.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::collections::BTreeSet;

use iter_extended::vecmap;
use noirc_frontend::monomorphization::ast::InlineType;

use super::basic_block::BasicBlockId;
use super::dfg::DataFlowGraph;
Expand All @@ -17,29 +18,14 @@ pub(crate) enum RuntimeType {
Brillig,
}

/// Represents how a RuntimeType::Acir function should be inlined.
/// This type is only relevant for ACIR functions as we do not inline any Brillig functions
#[derive(Default, Clone, Copy, PartialEq, Eq, Debug, Hash)]
pub(crate) enum InlineType {
/// The most basic entry point can expect all its functions to be inlined.
/// All function calls are expected to be inlined into a single ACIR.
#[default]
Inline,
/// Functions marked as foldable will not be inlined and compiled separately into ACIR
Fold,
}

impl RuntimeType {
/// Returns whether the runtime type represents an entry point.
/// We return `false` for InlineType::Inline on default, which is true
/// in all cases except for main. `main` should be supported with special
/// handling in any places where this function determines logic.
pub(crate) fn is_entry_point(&self) -> bool {
match self {
RuntimeType::Acir(inline_type) => match inline_type {
InlineType::Inline => false,
InlineType::Fold => true,
},
RuntimeType::Acir(inline_type) => inline_type.is_entry_point(),
RuntimeType::Brillig => true,
}
}
Expand Down Expand Up @@ -163,15 +149,6 @@ impl std::fmt::Display for RuntimeType {
}
}

impl std::fmt::Display for InlineType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
InlineType::Inline => write!(f, "inline"),
InlineType::Fold => write!(f, "fold"),
}
}
}

/// FunctionId is a reference for a function
///
/// This Id is how each function refers to other functions
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,12 +517,12 @@ impl<'function> PerFunctionContext<'function> {
#[cfg(test)]
mod test {
use acvm::FieldElement;
use noirc_frontend::monomorphization::ast::InlineType;

use crate::ssa::{
function_builder::FunctionBuilder,
ir::{
basic_block::BasicBlockId,
function::InlineType,
instruction::{BinaryOp, Intrinsic, TerminatorInstruction},
map::Id,
types::Type,
Expand Down
5 changes: 2 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use noirc_frontend::monomorphization::ast::{FuncId, Program};
use crate::errors::RuntimeError;
use crate::ssa::function_builder::FunctionBuilder;
use crate::ssa::ir::basic_block::BasicBlockId;
use crate::ssa::ir::function::FunctionId as IrFunctionId;
use crate::ssa::ir::function::{Function, RuntimeType};
use crate::ssa::ir::function::{FunctionId as IrFunctionId, InlineType};
use crate::ssa::ir::instruction::BinaryOp;
use crate::ssa::ir::instruction::Instruction;
use crate::ssa::ir::map::AtomicCounter;
Expand Down Expand Up @@ -125,8 +125,7 @@ impl<'a> FunctionContext<'a> {
if func.unconstrained {
self.builder.new_brillig_function(func.name.clone(), id);
} else {
let inline_type = if func.should_fold { InlineType::Fold } else { InlineType::Inline };
self.builder.new_function(func.name.clone(), id, inline_type);
self.builder.new_function(func.name.clone(), id, func.inline_type);
}
self.add_parameters_to_scope(&func.parameters);
}
Expand Down
9 changes: 2 additions & 7 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ use noirc_frontend::monomorphization::ast::{self, Expression, Program};

use crate::{
errors::{InternalError, RuntimeError},
ssa::{
function_builder::data_bus::DataBusBuilder,
ir::{function::InlineType, instruction::Intrinsic},
},
ssa::{function_builder::data_bus::DataBusBuilder, ir::instruction::Intrinsic},
};

use self::{
Expand Down Expand Up @@ -61,9 +58,7 @@ pub(crate) fn generate_ssa(
if force_brillig_runtime || main.unconstrained {
RuntimeType::Brillig
} else {
let main_inline_type =
if main.should_fold { InlineType::Fold } else { InlineType::Inline };
RuntimeType::Acir(main_inline_type)
RuntimeType::Acir(main.inline_type)
},
&context,
);
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/ast/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ impl From<FunctionDefinition> for NoirFunction {
Some(FunctionAttribute::Oracle(_)) => FunctionKind::Oracle,
Some(FunctionAttribute::Recursive) => FunctionKind::Recursive,
Some(FunctionAttribute::Fold) => FunctionKind::Normal,
Some(FunctionAttribute::Inline(_)) => FunctionKind::Normal,
None => FunctionKind::Normal,
};

Expand Down
28 changes: 28 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ pub enum ResolverError {
JumpInConstrainedFn { is_break: bool, span: Span },
#[error("break/continue are only allowed within loops")]
JumpOutsideLoop { is_break: bool, span: Span },
#[error("#[inline(tag)] attribute is only allowed on constrained functions")]
InlineAttributeOnUnconstrained { ident: Ident },
#[error("#[fold] attribute is only allowed on constrained functions")]
FoldAttributeOnUnconstrained { ident: Ident },
}

impl ResolverError {
Expand Down Expand Up @@ -340,6 +344,30 @@ impl From<ResolverError> for Diagnostic {
span,
)
},
ResolverError::InlineAttributeOnUnconstrained { ident } => {
let name = &ident.0.contents;

let mut diag = Diagnostic::simple_error(
format!("misplaced #[inline(tag)] attribute on unconstrained function {name}. Only allowed on constrained functions"),
"misplaced #[inline(tag)] attribute".to_string(),
ident.0.span(),
);

diag.add_note("The `#[inline(tag)]` attribute specifies to the compiler whether it should diverge from auto-inlining constrained functions".to_owned());
diag
}
ResolverError::FoldAttributeOnUnconstrained { ident } => {
let name = &ident.0.contents;

let mut diag = Diagnostic::simple_error(
format!("misplaced #[fold] attribute on unconstrained function {name}. Only allowed on constrained functions"),
"misplaced #[fold] attribute".to_string(),
ident.0.span(),
);

diag.add_note("The `#[fold]` attribute specifies whether a constrained function should be treated as a separate circuit rather than inlined into the program entry point".to_owned());
diag
}
}
}
}

0 comments on commit 1ec9cdc

Please sign in to comment.