diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index aa517d43104..94546905cad 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -309,10 +309,8 @@ impl DataFlowGraph { existing_id: Option, ) -> 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) { diff --git a/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs b/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs index 524805238b9..dc0c8f9fa76 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs @@ -226,19 +226,22 @@ fn find_variants(ssa: &Ssa) -> Variants { ); } - let mut signature_to_functions_as_value: BTreeMap> = 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(); + + variants.insert(key, target_fns); } variants @@ -530,14 +533,10 @@ 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 { @@ -545,21 +544,31 @@ mod tests { 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(); @@ -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); + } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 7ad703523d4..13c2a460bdb 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -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; + res? + } HirExpression::Prefix(prefix) => { let rhs = self.expr(prefix.rhs)?; @@ -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), @@ -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), @@ -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 = diff --git a/test_programs/execution_success/regression_7247/Nargo.toml b/test_programs/execution_success/regression_7247/Nargo.toml new file mode 100644 index 00000000000..6ad9397f415 --- /dev/null +++ b/test_programs/execution_success/regression_7247/Nargo.toml @@ -0,0 +1,7 @@ +[package] + name = "regression_7247" + version = "0.1.0" + type = "bin" + authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_7247/Prover.toml b/test_programs/execution_success/regression_7247/Prover.toml new file mode 100644 index 00000000000..02e613b3b61 --- /dev/null +++ b/test_programs/execution_success/regression_7247/Prover.toml @@ -0,0 +1,3 @@ +a = 5 +b = 500 +c = 5000 diff --git a/test_programs/execution_success/regression_7247/src/main.nr b/test_programs/execution_success/regression_7247/src/main.nr new file mode 100644 index 00000000000..0b465a549d3 --- /dev/null +++ b/test_programs/execution_success/regression_7247/src/main.nr @@ -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 + } +}