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

fix: Try to turn acir lambdas into brillig #7279

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
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
6 changes: 2 additions & 4 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,8 @@ impl DataFlowGraph {
existing_id: Option<InstructionId>,
) -> InsertInstructionResult {
if !self.is_handled_by_runtime(&instruction) {
// BUG: With panicking it fails to build the `token_contract`; see:
// https://github.com/AztecProtocol/aztec-packages/pull/11294#issuecomment-2624379102
// panic!("Attempted to insert instruction not handled by runtime: {instruction:?}");
return InsertInstructionResult::InstructionRemoved;
// It would be okay to simplify away these instructions here, but if they appear it's most likely a bug.
panic!("Attempted to insert instruction not handled by runtime: {instruction:?}");
}

match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) {
Expand Down
152 changes: 135 additions & 17 deletions compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,19 +226,22 @@ fn find_variants(ssa: &Ssa) -> Variants {
);
}

let mut signature_to_functions_as_value: BTreeMap<Signature, Vec<FunctionId>> = BTreeMap::new();
let mut signature_to_functions_as_value: Variants = BTreeMap::new();

for function_id in functions_as_values {
let signature = ssa.functions[&function_id].signature();
signature_to_functions_as_value.entry(signature).or_default().push(function_id);
let func = &ssa.functions[&function_id];
let signature = func.signature();
let runtime = func.runtime();
signature_to_functions_as_value.entry((signature, runtime)).or_default().push(function_id);
}

let mut variants: Variants = BTreeMap::new();

for (dispatch_signature, caller_runtime) in dynamic_dispatches {
let target_fns =
signature_to_functions_as_value.get(&dispatch_signature).cloned().unwrap_or_default();
variants.insert((dispatch_signature, caller_runtime), target_fns);
let key = (dispatch_signature, caller_runtime);
let target_fns = signature_to_functions_as_value.get(&key).cloned().unwrap_or_default();
Comment on lines -239 to +242
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this change we end up in a situation where if the lambda is created in constrained code, and subsequently is passed to unconstrained code, then this list is empty and the compiler crashes further down.


variants.insert(key, target_fns);
}

variants
Expand Down Expand Up @@ -530,36 +533,42 @@ mod tests {
let src = "
acir(inline) fn main f0 {
b0(v0: u32):
v3 = call f1(f2, v0) -> u32
v5 = add v0, u32 1
v6 = eq v3, v5
constrain v3 == v5
v9 = call f4(f3, v0) -> u32
v10 = add v0, u32 1
v11 = eq v9, v10
constrain v9 == v10
v13 = call f1(f3, v0) -> u32
v15 = call f1(f5, v0) -> u32
v24 = call f2(f4, v0) -> u32
v26 = call f2(f6, v0) -> u32
return
}
brillig(inline) fn wrapper f1 {
b0(v0: function, v1: u32):
v2 = call v0(v1) -> u32
return v2
}
acir(inline) fn wrapper_acir f4 {
acir(inline) fn wrapper_acir f2 {
b0(v0: function, v1: u32):
v2 = call v0(v1) -> u32
return v2
}
brillig(inline) fn increment f2 {
brillig(inline) fn increment f3 {
b0(v0: u32):
v2 = add v0, u32 1
return v2
}
acir(inline) fn increment_acir f3 {
acir(inline) fn increment_acir f4 {
b0(v0: u32):
v2 = add v0, u32 1
return v2
}
brillig(inline) fn decrement f5 {
b0(v0: u32):
v2 = sub v0, u32 1
return v2
}
acir(inline) fn decrement_acir f6 {
b0(v0: u32):
v2 = sub v0, u32 1
return v2
}
";

let ssa = Ssa::from_str(src).unwrap();
Expand All @@ -570,4 +579,113 @@ mod tests {
assert!(applies.iter().any(|f| f.runtime().is_acir()));
assert!(applies.iter().any(|f| f.runtime().is_brillig()));
}

#[test]
fn lambdas_separated_per_runtime() {
let src = "
acir(inline) fn main f0 {
b0(v0: u32):
v1 = call f1(v0) -> u32
v2 = call f2(v0) -> u32
return v1, v2
}
brillig(inline) fn via_brillig f1 {
b0(v0: u32):
v2 = call f6(v0) -> u32
return v2
}
acir(inline) fn via_acir f2 {
b0(v0: u32):
v2 = call f3(v0) -> u32
return v2
}
acir(inline) fn times100 f3 {
b0(v0: u32):
v3 = call f4(v0, f5) -> u32
return v3
}
acir(inline) fn apply_twice f4 {
b0(v0: u32, v1: function):
v2 = call v1(v0) -> u32
v3 = call v1(v2) -> u32
return v3
}
acir(inline) fn lambda f5 {
b0(v0: u32):
v2 = mul v0, u32 10
return v2
}
brillig(inline) fn times100 f6 {
b0(v0: u32):
v3 = call f7(v0, f8) -> u32
return v3
}
brillig(inline) fn apply_twice f7 {
b0(v0: u32, v1: function):
v2 = call v1(v0) -> u32
v3 = call v1(v2) -> u32
return v3
}
brillig(inline) fn lambda f8 {
b0(v0: u32):
v2 = mul v0, u32 10
return v2
}";

let ssa = Ssa::from_str(src).unwrap();
let ssa = ssa.defunctionalize();

// The lambdas called by the `apply` functions should all be the same runtime.
let expected = "
acir(inline) fn main f0 {
b0(v0: u32):
v2 = call f1(v0) -> u32
v4 = call f2(v0) -> u32
return v2, v4
}
brillig(inline) fn via_brillig f1 {
b0(v0: u32):
v2 = call f6(v0) -> u32
return v2
}
acir(inline) fn via_acir f2 {
b0(v0: u32):
v2 = call f3(v0) -> u32
return v2
}
acir(inline) fn times100 f3 {
b0(v0: u32):
v3 = call f4(v0, Field 5) -> u32
return v3
}
acir(inline) fn apply_twice f4 {
b0(v0: u32, v1: Field):
v3 = call f5(v0) -> u32
v4 = call f5(v3) -> u32
return v4
}
acir(inline) fn lambda f5 {
b0(v0: u32):
v2 = mul v0, u32 10
return v2
}
brillig(inline) fn times100 f6 {
b0(v0: u32):
v3 = call f7(v0, Field 8) -> u32
return v3
}
brillig(inline) fn apply_twice f7 {
b0(v0: u32, v1: Field):
v3 = call f8(v0) -> u32
v4 = call f8(v3) -> u32
return v4
}
brillig(inline) fn lambda f8 {
b0(v0: u32):
v2 = mul v0, u32 10
return v2
}
";
assert_normalized_ssa_equals(ssa, expected);
}
}
25 changes: 20 additions & 5 deletions compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,16 @@ impl<'interner> Monomorphizer<'interner> {
},
HirExpression::Literal(HirLiteral::Unit) => ast::Expression::Block(vec![]),
HirExpression::Block(block) => self.block(block.statements)?,
HirExpression::Unsafe(block) => self.block(block.statements)?,
HirExpression::Unsafe(block) => {
// TODO: Undo this change; not everything in an `unsafe` block is unconstrained, it just
// means we can call unconstrained functions. But it was a convenient way to change the
// the way lambdas made in an `unsafe` block are compiled and make corresponding tests pass.
let was_in_unconstrained_function = self.in_unconstrained_function;
self.in_unconstrained_function = true;
let res = self.block(block.statements);
self.in_unconstrained_function = was_in_unconstrained_function;
Comment on lines +508 to +510
Copy link
Member

Choose a reason for hiding this comment

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

This seems like we're treating the contents of an unsafe block as unconstrained. This isn't the case however as it's still constrained but the only difference is that we can call into unconstrained functions.

res?
}

HirExpression::Prefix(prefix) => {
let rhs = self.expr(prefix.rhs)?;
Expand Down Expand Up @@ -1822,15 +1831,16 @@ impl<'interner> Monomorphizer<'interner> {
inline_type: InlineType::default(),
func_sig: FunctionSignature::default(),
};
self.push_function(id, function);

let typ = ast::Type::Function(
parameter_types,
Box::new(ret_type),
Box::new(ast::Type::Unit),
false,
function.unconstrained,
);

self.push_function(id, function);

let name = lambda_name.to_owned();
Ok(ast::Expression::Ident(ast::Ident {
definition: Definition::Function(id),
Expand Down Expand Up @@ -1928,11 +1938,15 @@ impl<'interner> Monomorphizer<'interner> {
let body = self.expr(lambda.body)?;
self.lambda_envs_stack.pop();

// TODO: Ideally a lambda would inherit the runtime of its caller, but it could be passed as a
// variable (by function ID) to a function that uses it both as constrained and unconstrained.
let is_unconstrained = self.in_unconstrained_function;

let lambda_fn_typ: ast::Type = ast::Type::Function(
parameter_types,
Box::new(ret_type),
Box::new(env_typ.clone()),
false,
is_unconstrained,
);
let lambda_fn = ast::Expression::Ident(ast::Ident {
definition: Definition::Function(id),
Expand All @@ -1951,10 +1965,11 @@ impl<'interner> Monomorphizer<'interner> {
parameters,
body,
return_type,
unconstrained: self.in_unconstrained_function,
unconstrained: is_unconstrained,
inline_type: InlineType::default(),
func_sig: FunctionSignature::default(),
};

self.push_function(id, function);

let lambda_value =
Expand Down
7 changes: 7 additions & 0 deletions test_programs/execution_success/regression_7247/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_7247"
version = "0.1.0"
type = "bin"
authors = [""]

[dependencies]
3 changes: 3 additions & 0 deletions test_programs/execution_success/regression_7247/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
a = 5
b = 500
c = 5000
67 changes: 67 additions & 0 deletions test_programs/execution_success/regression_7247/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
fn main(
a: u32,
b: u32,
c: u32,
) {
/// Safety: Test
let vb = unsafe { via_brillig(a) };
let va = via_acir(a);
assert_eq(vb, b);
assert_eq(va, b);

/// Safety: Test
let bat = unsafe { brillig_apply_twice(a, times_ten) };
assert_eq(bat, b, "should be a*100");

let mat = mixed_apply_thrice(a, times_ten);
assert_eq(mat, c, "should be a*1000");
}

// The lambdas created within should be constrained.
fn via_acir(a: u32) -> u32 {
times100(a)
}

// The lambdas created within this should be unconstrained,
unconstrained fn via_brillig(a: u32) -> u32 {
times100(a)
}

// Example of a higher order function that will be transformed
// to call lambdas via `apply` functions. Those should only call
// one version of the `|x| x * 10` lambda, not both `acir` and `brillig`.
fn apply_twice(a: u32, f: fn(u32) -> u32) -> u32 {
f(f(a))
}

// Example of an unconstrained higher order function to which we pass
// a lambda defined in constrained code; we want it to call a `brillig` lambda.
unconstrained fn brillig_apply_twice(a: u32, f: fn(u32) -> u32) -> u32 {
f(f(a))
}

// Example of a constrained function that gets a constrained lambda,
// which it passes on to unconstrained code and also calls it locally,
// so the lambda is gets is called from both kinds of environments.
fn mixed_apply_thrice(a: u32, f: fn(u32) -> u32) -> u32 {
/// Safety: Test
let aa = unsafe { brillig_apply_twice(a, f) };
let aaa = f(aa);
aaa
}

// Example of creating a lambda and passing it on. Whether it's constrained or unconstrained
// depends on whether we call it from `via_acir` or `via_brillig`.
fn times100(a: u32) -> u32 {
apply_twice(a, |x| x * 10)
}

// Example of a constrained function we pass to both constrained and unconstrained by name.
fn times_ten(x: u32) -> u32 {
// Arbitrary `if` statement to trigger `EnableSideEffect` during flattening.
if x > 100000 {
x * 10 - 1
} else {
x * 10
}
}
Loading