From 64b33401af90e6752358894bdf3bb96f8d33d323 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 29 Apr 2024 18:20:05 +0000 Subject: [PATCH 01/14] implement switching off inline_never after passes which we want to make more granular for inlining --- compiler/noirc_evaluator/src/ssa.rs | 5 ++- .../ssa/acir_gen/acir_ir/generated_acir.rs | 3 ++ .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 14 +++++++- .../noirc_evaluator/src/ssa/ir/function.rs | 10 ++++++ .../src/ssa/opt/flatten_cfg.rs | 3 ++ .../noirc_evaluator/src/ssa/opt/inlining.rs | 26 ++++++++++----- .../src/ssa/ssa_gen/program.rs | 5 +++ .../src/monomorphization/ast.rs | 2 +- .../Nargo.toml | 7 ++++ .../Prover.toml | 2 ++ .../src/main.nr | 33 +++++++++++++++++++ 11 files changed, 98 insertions(+), 12 deletions(-) create mode 100644 test_programs/execution_success/inline_numeric_generic_poseidon/Nargo.toml create mode 100644 test_programs/execution_success/inline_numeric_generic_poseidon/Prover.toml create mode 100644 test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index f9fd5f41f77..4eeb62e997d 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -58,6 +58,9 @@ pub(crate) fn optimize_into_acir( .try_run_pass(Ssa::unroll_loops_iteratively, "After Unrolling:")? .run_pass(Ssa::simplify_cfg, "After Simplifying:") .run_pass(Ssa::flatten_cfg, "After Flattening:") + // Run the inlining pass again as certain codegen attributes will now be disabled after flattening, + // such as treating functions marked with the `InlineType::Never` as an entry point. + .run_pass(Ssa::inline_functions, "After Inlining:") .run_pass(Ssa::remove_bit_shifts, "After Removing Bit Shifts:") // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores .run_pass(Ssa::mem2reg, "After Mem2Reg:") @@ -159,7 +162,7 @@ pub fn create_program( // For setting up the ABI we need separately specify main's input and return witnesses let mut is_main = true; for (acir, func_sig) in generated_acirs.into_iter().zip(func_sigs) { - let circuit_artifact = convert_generated_acir_into_circuit( + let mut circuit_artifact = convert_generated_acir_into_circuit( acir, func_sig, recursive, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 9f305a28c25..5bac60d10f2 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -22,6 +22,7 @@ use acvm::{ FieldElement, }; use iter_extended::vecmap; +use noirc_frontend::monomorphization::ast::InlineType; use num_bigint::BigUint; /// Brillig calls such as for the Brillig std lib are resolved only after code generation is finished. @@ -73,6 +74,8 @@ pub(crate) struct GeneratedAcir { /// As to avoid passing the ACIR gen shared context into each individual ACIR /// we can instead keep this map and resolve the Brillig calls at the end of code generation. pub(crate) brillig_stdlib_func_locations: BTreeMap, + + pub(crate) inline_type: InlineType, } #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 63d1c1b564f..ce35bd8a81d 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -320,6 +320,11 @@ impl Ssa { } } + generated_acir.inline_type = match function.runtime() { + RuntimeType::Acir(inline_type) => inline_type, + // TODO: switch to real error + _ => panic!("unexpected brillig runtime for a generated acir"), + }; generated_acir.name = function.name().to_owned(); acirs.push(generated_acir); } @@ -390,7 +395,10 @@ impl<'a> Context<'a> { panic!("ACIR function should have been inlined earlier if not marked otherwise"); } } - InlineType::Fold | InlineType::Never => {} + InlineType::Never => { + panic!("All ACIR functions marked with #[inline(never)] should be inlined before ACIR gen. This is an SSA exclusive codegen attribute"); + } + InlineType::Fold => {} } // 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)?)) @@ -2653,7 +2661,11 @@ mod test { basic_call_with_outputs_assert(InlineType::Fold); call_output_as_next_call_input(InlineType::Fold); basic_nested_call(InlineType::Fold); + } + #[test] + #[should_panic] + fn basic_calls_inline_never() { call_output_as_next_call_input(InlineType::Never); basic_nested_call(InlineType::Never); basic_call_with_outputs_assert(InlineType::Never); diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 057786bf5ec..7d6d4be4a17 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -85,6 +85,16 @@ impl Function { self.runtime = runtime; } + pub(crate) fn is_inline_never(&self) -> bool { + match self.runtime() { + RuntimeType::Acir(inline_type) => match inline_type { + InlineType::Never => true, + _ => false, + }, + RuntimeType::Brillig => false, + } + } + /// Retrieves the entry block of a function. /// /// A function's entry block contains the instructions diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 07771397ce8..6789bfc14ab 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -170,6 +170,9 @@ impl Ssa { for function in self.functions.values_mut() { flatten_function_cfg(function); } + // Now that flattening has been completed we can have SSA generation inline any + // calls to functions marked with `#[inline(never)]` + self.use_inline_never = false; self } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 0b78d47fbb1..36aa973559c 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -5,6 +5,7 @@ use std::collections::{BTreeSet, HashSet}; use iter_extended::{btree_map, vecmap}; +use noirc_frontend::monomorphization::ast::InlineType; use crate::ssa::{ function_builder::FunctionBuilder, @@ -38,10 +39,11 @@ impl Ssa { /// as well save the work for later instead of performing it twice. #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn inline_functions(mut self) -> Ssa { - self.functions = btree_map(get_entry_point_functions(&self), |entry_point| { - let new_function = InlineContext::new(&self, entry_point).inline_all(&self); - (entry_point, new_function) - }); + self.functions = + btree_map(get_entry_point_functions(&self, self.use_inline_never), |entry_point| { + let new_function = InlineContext::new(&self, entry_point).inline_all(&self); + (entry_point, new_function) + }); self } @@ -97,10 +99,13 @@ struct PerFunctionContext<'function> { /// should be left in the final program. /// This is the `main` function, any Acir functions with a [fold inline type][InlineType::Fold], /// and any brillig functions used. -fn get_entry_point_functions(ssa: &Ssa) -> BTreeSet { +fn get_entry_point_functions(ssa: &Ssa, use_inline_never: bool) -> BTreeSet { let functions = ssa.functions.iter(); let mut entry_points = functions - .filter(|(_, function)| function.runtime().is_entry_point()) + .filter(|(_, function)| { + let inline_never = if use_inline_never { function.is_inline_never() } else { false }; + function.runtime().is_entry_point() || inline_never + }) .map(|(id, _)| *id) .collect::>(); @@ -351,11 +356,14 @@ impl<'function> PerFunctionContext<'function> { for id in block.instructions() { match &self.source_function.dfg[*id] { Instruction::Call { func, arguments } => match self.get_function(*func) { - Some(function) => { - if ssa.functions[&function].runtime().is_entry_point() { + Some(func_id) => { + let function = &ssa.functions[&func_id]; + let inline_never = + if ssa.use_inline_never { function.is_inline_never() } else { false }; + if function.runtime().is_entry_point() || inline_never { self.push_instruction(*id); } else { - self.inline_function(ssa, *id, function, arguments); + self.inline_function(ssa, *id, func_id, arguments); } } None => self.push_instruction(*id), diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index b05a2cbc741..40927dab36d 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -17,6 +17,10 @@ pub(crate) struct Ssa { /// This mapping is necessary to use the correct function pointer for an ACIR call, /// as the final program artifact will be a list of only entry point functions. pub(crate) entry_point_to_generated_index: BTreeMap, + /// A flag to indicate how we should perform codegen in between passes. + /// This is currently defaulted to `true` and should be set to false by a pass at some point + /// before ACIR generation. + pub(crate) use_inline_never: bool, } impl Ssa { @@ -50,6 +54,7 @@ impl Ssa { main_id, next_id: AtomicCounter::starting_after(max_id), entry_point_to_generated_index, + use_inline_never: true, } } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index dc43f53e038..3961ad13043 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -242,7 +242,7 @@ impl InlineType { match self { InlineType::Inline => false, InlineType::Fold => true, - InlineType::Never => true, + InlineType::Never => false, } } } diff --git a/test_programs/execution_success/inline_numeric_generic_poseidon/Nargo.toml b/test_programs/execution_success/inline_numeric_generic_poseidon/Nargo.toml new file mode 100644 index 00000000000..af74e941d2b --- /dev/null +++ b/test_programs/execution_success/inline_numeric_generic_poseidon/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "inline_numeric_generic_poseidon" +type = "bin" +authors = [""] +compiler_version = ">=0.28.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/inline_numeric_generic_poseidon/Prover.toml b/test_programs/execution_success/inline_numeric_generic_poseidon/Prover.toml new file mode 100644 index 00000000000..00e821cf89d --- /dev/null +++ b/test_programs/execution_success/inline_numeric_generic_poseidon/Prover.toml @@ -0,0 +1,2 @@ +enable = [true, false] +to_hash = [[0, 0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0]] diff --git a/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr b/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr new file mode 100644 index 00000000000..cd9c06d484f --- /dev/null +++ b/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr @@ -0,0 +1,33 @@ +use dep::std::hash::{pedersen_hash_with_separator, poseidon2::Poseidon2}; + +global NUM_HASHES = 2; +global HASH_LENGTH = 10; + +#[inline(never)] +pub fn poseidon_hash(inputs: [Field; N]) -> Field { + Poseidon2::hash(inputs, inputs.len()) +} + +fn main( + to_hash: [[Field; HASH_LENGTH]; NUM_HASHES], + enable: [bool; NUM_HASHES] +) -> pub [Field; NUM_HASHES + 1] { + let mut result = [0; NUM_HASHES + 1]; + for i in 0..NUM_HASHES { + let enable = enable[i]; + let to_hash = to_hash[i]; + if enable { + result[i] = poseidon_hash(to_hash); + } + } + + // We want to make sure that the foldable function with a numeric generic + // is monomorphized correctly. + let mut double_preimage = [0; 20]; + for i in 0..HASH_LENGTH * 2 { + double_preimage[i] = to_hash[0][i % HASH_LENGTH]; + } + result[NUM_HASHES] = poseidon_hash(double_preimage); + + result +} From 78113a6bf7778eabed4d57123a87d33e39aea1b5 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 29 Apr 2024 18:23:32 +0000 Subject: [PATCH 02/14] cleanup --- compiler/noirc_evaluator/src/ssa.rs | 2 +- compiler/noirc_evaluator/src/ssa/ir/function.rs | 5 +---- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 1 - 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 4eeb62e997d..db52a95b15f 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -162,7 +162,7 @@ pub fn create_program( // For setting up the ABI we need separately specify main's input and return witnesses let mut is_main = true; for (acir, func_sig) in generated_acirs.into_iter().zip(func_sigs) { - let mut circuit_artifact = convert_generated_acir_into_circuit( + let circuit_artifact = convert_generated_acir_into_circuit( acir, func_sig, recursive, diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 7d6d4be4a17..d1befb2cb2e 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -87,10 +87,7 @@ impl Function { pub(crate) fn is_inline_never(&self) -> bool { match self.runtime() { - RuntimeType::Acir(inline_type) => match inline_type { - InlineType::Never => true, - _ => false, - }, + RuntimeType::Acir(inline_type) => matches!(inline_type, InlineType::Never), RuntimeType::Brillig => false, } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 36aa973559c..b4d4b331677 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -5,7 +5,6 @@ use std::collections::{BTreeSet, HashSet}; use iter_extended::{btree_map, vecmap}; -use noirc_frontend::monomorphization::ast::InlineType; use crate::ssa::{ function_builder::FunctionBuilder, From 6df184a67fce8e5fdd538e7b0eb622b0844557d7 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 29 Apr 2024 18:29:27 +0000 Subject: [PATCH 03/14] remove old tech debt --- .../src/ssa/acir_gen/acir_ir/generated_acir.rs | 3 --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 5 ----- 2 files changed, 8 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 5bac60d10f2..9f305a28c25 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -22,7 +22,6 @@ use acvm::{ FieldElement, }; use iter_extended::vecmap; -use noirc_frontend::monomorphization::ast::InlineType; use num_bigint::BigUint; /// Brillig calls such as for the Brillig std lib are resolved only after code generation is finished. @@ -74,8 +73,6 @@ pub(crate) struct GeneratedAcir { /// As to avoid passing the ACIR gen shared context into each individual ACIR /// we can instead keep this map and resolve the Brillig calls at the end of code generation. pub(crate) brillig_stdlib_func_locations: BTreeMap, - - pub(crate) inline_type: InlineType, } #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index ce35bd8a81d..72e290898eb 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -320,11 +320,6 @@ impl Ssa { } } - generated_acir.inline_type = match function.runtime() { - RuntimeType::Acir(inline_type) => inline_type, - // TODO: switch to real error - _ => panic!("unexpected brillig runtime for a generated acir"), - }; generated_acir.name = function.name().to_owned(); acirs.push(generated_acir); } From 8995293122aa6f225fc4ffd5764796797b049988 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 29 Apr 2024 18:30:53 +0000 Subject: [PATCH 04/14] cleanup comment --- .../inline_numeric_generic_poseidon/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr b/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr index cd9c06d484f..42d3342c114 100644 --- a/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr +++ b/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr @@ -21,7 +21,7 @@ fn main( } } - // We want to make sure that the foldable function with a numeric generic + // We want to make sure that the function marked with `#[inline(never)]` with a numeric generic // is monomorphized correctly. let mut double_preimage = [0; 20]; for i in 0..HASH_LENGTH * 2 { From a63f54f6baa0f9911c54cafb78d953d19cab1a69 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 29 Apr 2024 21:16:10 +0000 Subject: [PATCH 05/14] move inline after mem2reg --- compiler/noirc_evaluator/src/ssa.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index db52a95b15f..5ebf149874e 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -58,12 +58,12 @@ pub(crate) fn optimize_into_acir( .try_run_pass(Ssa::unroll_loops_iteratively, "After Unrolling:")? .run_pass(Ssa::simplify_cfg, "After Simplifying:") .run_pass(Ssa::flatten_cfg, "After Flattening:") - // Run the inlining pass again as certain codegen attributes will now be disabled after flattening, - // such as treating functions marked with the `InlineType::Never` as an entry point. - .run_pass(Ssa::inline_functions, "After Inlining:") .run_pass(Ssa::remove_bit_shifts, "After Removing Bit Shifts:") // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores .run_pass(Ssa::mem2reg, "After Mem2Reg:") + // Run the inlining pass again as certain codegen attributes will now be disabled after flattening, + // such as treating functions marked with the `InlineType::Never` as an entry point. + .run_pass(Ssa::inline_functions, "After Inlining:") .run_pass(Ssa::fold_constants, "After Constant Folding:") .run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffects removal:") .run_pass(Ssa::fold_constants_using_constraints, "After Constraint Folding:") From e49e9cf32ca19413cad83820c9ff8048dd47f99b Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 29 Apr 2024 21:25:22 +0000 Subject: [PATCH 06/14] inline_numeric_generic_poseidon ignore for debugger --- tooling/debugger/ignored-tests.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index 5e6ce18be54..0c8624631df 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -18,4 +18,5 @@ fold_call_witness_condition fold_after_inlined_calls fold_numeric_generic_poseidon inline_never_basic +inline_numeric_generic_poseidon regression_4709 From dc479fb5cf872bf27b5f428e848cb3f376a20a0e Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 29 Apr 2024 21:26:52 +0000 Subject: [PATCH 07/14] typo fix --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 72e290898eb..4230d1a69fa 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -2802,7 +2802,7 @@ mod test { let (acir_functions, _) = ssa .into_acir(&Brillig::default(), noirc_frontend::ast::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); - // The expected result should look very similar to the abvoe test expect that the input witnesses of the `Call` + // The expected result should look very similar to the above test expect that the input witnesses of the `Call` // opcodes will be different. The changes can discerned from the checks below. let main_acir = &acir_functions[0]; From c4f8acff6e80c0b8183b324438de05a08113e20b Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 30 Apr 2024 01:50:43 +0000 Subject: [PATCH 08/14] use && --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index b4d4b331677..d9c4551d5df 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -102,7 +102,7 @@ fn get_entry_point_functions(ssa: &Ssa, use_inline_never: bool) -> BTreeSet PerFunctionContext<'function> { Instruction::Call { func, arguments } => match self.get_function(*func) { Some(func_id) => { let function = &ssa.functions[&func_id]; - let inline_never = - if ssa.use_inline_never { function.is_inline_never() } else { false }; + let inline_never = ssa.use_inline_never && function.is_inline_never(); if function.runtime().is_entry_point() || inline_never { self.push_instruction(*id); } else { From bb7eb8ed5c0bb922cc94229dfe109433d3ae9f5c Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 30 Apr 2024 18:18:35 +0000 Subject: [PATCH 09/14] rename inline attribute to no_predicates --- compiler/noirc_evaluator/src/ssa.rs | 2 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 12 ++++++------ .../noirc_evaluator/src/ssa/ir/function.rs | 4 ++-- .../src/ssa/opt/flatten_cfg.rs | 4 ++-- .../noirc_evaluator/src/ssa/opt/inlining.rs | 17 +++++++++++------ .../src/ssa/ssa_gen/program.rs | 10 +++++----- compiler/noirc_frontend/src/ast/function.rs | 2 +- .../src/hir/resolution/errors.rs | 12 ++++++------ .../src/hir/resolution/resolver.rs | 12 ++++++------ .../noirc_frontend/src/hir/type_check/mod.rs | 4 ++-- .../noirc_frontend/src/hir_def/function.rs | 5 +++-- compiler/noirc_frontend/src/lexer/token.rs | 19 ++++++++----------- .../src/monomorphization/ast.rs | 19 +++++++------------ compiler/noirc_frontend/src/tests.rs | 7 +++++-- .../Nargo.toml | 2 +- .../Prover.toml | 0 .../src/main.nr | 2 +- .../Nargo.toml | 2 +- .../Prover.toml | 0 .../src/main.nr | 4 ++-- 20 files changed, 70 insertions(+), 69 deletions(-) rename test_programs/execution_success/{inline_never_basic => no_predicates_basic}/Nargo.toml (74%) rename test_programs/execution_success/{inline_never_basic => no_predicates_basic}/Prover.toml (100%) rename test_programs/execution_success/{inline_never_basic => no_predicates_basic}/src/main.nr (83%) rename test_programs/execution_success/{inline_numeric_generic_poseidon => no_predicates_numeric_generic_poseidon}/Nargo.toml (63%) rename test_programs/execution_success/{inline_numeric_generic_poseidon => no_predicates_numeric_generic_poseidon}/Prover.toml (100%) rename test_programs/execution_success/{inline_numeric_generic_poseidon => no_predicates_numeric_generic_poseidon}/src/main.nr (87%) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 5ebf149874e..6e0e31c0ae5 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -62,7 +62,7 @@ pub(crate) fn optimize_into_acir( // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores .run_pass(Ssa::mem2reg, "After Mem2Reg:") // Run the inlining pass again as certain codegen attributes will now be disabled after flattening, - // such as treating functions marked with the `InlineType::Never` as an entry point. + // such as treating functions marked with the `InlineType::NoPredicates` as an entry point. .run_pass(Ssa::inline_functions, "After Inlining:") .run_pass(Ssa::fold_constants, "After Constant Folding:") .run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffects removal:") diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 4230d1a69fa..16680c2b7eb 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -390,8 +390,8 @@ impl<'a> Context<'a> { panic!("ACIR function should have been inlined earlier if not marked otherwise"); } } - InlineType::Never => { - panic!("All ACIR functions marked with #[inline(never)] should be inlined before ACIR gen. This is an SSA exclusive codegen attribute"); + InlineType::NoPredicates => { + panic!("All ACIR functions marked with #[no_predicates] should be inlined before ACIR gen. This is an SSA exclusive codegen attribute"); } InlineType::Fold => {} } @@ -2660,10 +2660,10 @@ mod test { #[test] #[should_panic] - fn basic_calls_inline_never() { - call_output_as_next_call_input(InlineType::Never); - basic_nested_call(InlineType::Never); - basic_call_with_outputs_assert(InlineType::Never); + fn basic_calls_no_predicates() { + call_output_as_next_call_input(InlineType::NoPredicates); + basic_nested_call(InlineType::NoPredicates); + basic_call_with_outputs_assert(InlineType::NoPredicates); } #[test] diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index d1befb2cb2e..a49e02b0380 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -85,9 +85,9 @@ impl Function { self.runtime = runtime; } - pub(crate) fn is_inline_never(&self) -> bool { + pub(crate) fn is_no_predicates(&self) -> bool { match self.runtime() { - RuntimeType::Acir(inline_type) => matches!(inline_type, InlineType::Never), + RuntimeType::Acir(inline_type) => matches!(inline_type, InlineType::NoPredicates), RuntimeType::Brillig => false, } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 6789bfc14ab..bb4cd5e75ee 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -171,8 +171,8 @@ impl Ssa { flatten_function_cfg(function); } // Now that flattening has been completed we can have SSA generation inline any - // calls to functions marked with `#[inline(never)]` - self.use_inline_never = false; + // calls to functions marked with `#[no_predicates]` + self.past_flattening_pass = true; self } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index d9c4551d5df..e9277d6b1ba 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -39,7 +39,7 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn inline_functions(mut self) -> Ssa { self.functions = - btree_map(get_entry_point_functions(&self, self.use_inline_never), |entry_point| { + btree_map(get_entry_point_functions(&self, self.past_flattening_pass), |entry_point| { let new_function = InlineContext::new(&self, entry_point).inline_all(&self); (entry_point, new_function) }); @@ -98,12 +98,14 @@ struct PerFunctionContext<'function> { /// should be left in the final program. /// This is the `main` function, any Acir functions with a [fold inline type][InlineType::Fold], /// and any brillig functions used. -fn get_entry_point_functions(ssa: &Ssa, use_inline_never: bool) -> BTreeSet { +fn get_entry_point_functions(ssa: &Ssa, past_flattening_pass: bool) -> BTreeSet { let functions = ssa.functions.iter(); let mut entry_points = functions .filter(|(_, function)| { - let inline_never = use_inline_never && function.is_inline_never(); - function.runtime().is_entry_point() || inline_never + // If we have not already finished the flattening pass, functions marked + // to not have predicates should be marked as entry points. + let no_predicates_is_entry_point = !past_flattening_pass && function.is_no_predicates(); + function.runtime().is_entry_point() || no_predicates_is_entry_point }) .map(|(id, _)| *id) .collect::>(); @@ -357,8 +359,11 @@ impl<'function> PerFunctionContext<'function> { Instruction::Call { func, arguments } => match self.get_function(*func) { Some(func_id) => { let function = &ssa.functions[&func_id]; - let inline_never = ssa.use_inline_never && function.is_inline_never(); - if function.runtime().is_entry_point() || inline_never { + // If we have not already finished the flattening pass, functions marked + // to not have predicates should be marked as entry points. + let no_predicates_is_entry_point = + !ssa.past_flattening_pass && function.is_no_predicates(); + if function.runtime().is_entry_point() || no_predicates_is_entry_point { self.push_instruction(*id); } else { self.inline_function(ssa, *id, func_id, arguments); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index 40927dab36d..bb03b218616 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -17,10 +17,10 @@ pub(crate) struct Ssa { /// This mapping is necessary to use the correct function pointer for an ACIR call, /// as the final program artifact will be a list of only entry point functions. pub(crate) entry_point_to_generated_index: BTreeMap, - /// A flag to indicate how we should perform codegen in between passes. - /// This is currently defaulted to `true` and should be set to false by a pass at some point - /// before ACIR generation. - pub(crate) use_inline_never: bool, + /// Flag that determines when to inline functions marked to not use predicates. + /// This is currently defaulted to `false` and should be set to true by after completion + /// of the flattening pass which sets side effects. + pub(crate) past_flattening_pass: bool, } impl Ssa { @@ -54,7 +54,7 @@ impl Ssa { main_id, next_id: AtomicCounter::starting_after(max_id), entry_point_to_generated_index, - use_inline_never: true, + past_flattening_pass: false, } } diff --git a/compiler/noirc_frontend/src/ast/function.rs b/compiler/noirc_frontend/src/ast/function.rs index 9115178671e..dc426a4642a 100644 --- a/compiler/noirc_frontend/src/ast/function.rs +++ b/compiler/noirc_frontend/src/ast/function.rs @@ -109,7 +109,7 @@ impl From for NoirFunction { Some(FunctionAttribute::Oracle(_)) => FunctionKind::Oracle, Some(FunctionAttribute::Recursive) => FunctionKind::Recursive, Some(FunctionAttribute::Fold) => FunctionKind::Normal, - Some(FunctionAttribute::Inline(_)) => FunctionKind::Normal, + Some(FunctionAttribute::NoPredicates) => FunctionKind::Normal, None => FunctionKind::Normal, }; diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 1727471c34f..953edc7c7d9 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -90,8 +90,8 @@ pub enum ResolverError { MutableGlobal { span: Span }, #[error("Self-referential structs are not supported")] SelfReferentialStruct { span: Span }, - #[error("#[inline(tag)] attribute is only allowed on constrained functions")] - InlineAttributeOnUnconstrained { ident: Ident }, + #[error("#[no_predicates] attribute is only allowed on constrained functions")] + NoPredicatesAttributeOnUnconstrained { ident: Ident }, #[error("#[fold] attribute is only allowed on constrained functions")] FoldAttributeOnUnconstrained { ident: Ident }, } @@ -362,16 +362,16 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) }, - ResolverError::InlineAttributeOnUnconstrained { ident } => { + ResolverError::NoPredicatesAttributeOnUnconstrained { 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(), + format!("misplaced #[no_predicates] attribute on unconstrained function {name}. Only allowed on constrained functions"), + "misplaced #[no_predicates] 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.add_note("The `#[no_predicates]` attribute specifies to the compiler whether it should diverge from auto-inlining constrained functions".to_owned()); diag } ResolverError::FoldAttributeOnUnconstrained { ident } => { diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index bef0ebdaacc..14a45a8788f 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1000,11 +1000,11 @@ impl<'a> Resolver<'a> { let name_ident = HirIdent::non_trait_method(id, location); let attributes = func.attributes().clone(); - let has_inline_attribute = attributes.is_inline(); + let has_no_predicates_attribute = attributes.is_no_predicates(); let should_fold = attributes.is_foldable(); if !self.inline_attribute_allowed(func) { - if has_inline_attribute { - self.push_err(ResolverError::InlineAttributeOnUnconstrained { + if has_no_predicates_attribute { + self.push_err(ResolverError::NoPredicatesAttributeOnUnconstrained { ident: func.name_ident().clone(), }); } else if should_fold { @@ -1013,10 +1013,10 @@ impl<'a> Resolver<'a> { }); } } - // Both the #[fold] and #[inline(tag)] alter a function's inline type and code generation in similar ways. + // Both the #[fold] and #[no_predicates] alter a function's inline type and code generation in similar ways. // In certain cases such as type checking (for which the following flag will be used) both attributes // indicate we should code generate in the same way. Thus, we unify the attributes into one flag here. - let has_inline_or_fold_attribute = has_inline_attribute || should_fold; + let has_inline_attribute = has_no_predicates_attribute || should_fold; let mut generics = vecmap(&self.generics, |(_, typevar, _)| typevar.clone()); let mut parameters = vec![]; @@ -1111,7 +1111,7 @@ impl<'a> Resolver<'a> { has_body: !func.def.body.is_empty(), trait_constraints: self.resolve_trait_constraints(&func.def.where_clause), is_entry_point: self.is_entry_point_function(func), - has_inline_or_fold_attribute, + has_inline_attribute, } } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 6235fe3848d..ef5692abdd3 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -173,7 +173,7 @@ fn check_if_type_is_valid_for_program_input( ) { let meta = type_checker.interner.function_meta(&func_id); if (meta.is_entry_point && !param.1.is_valid_for_program_input()) - || (meta.has_inline_or_fold_attribute && !param.1.is_valid_non_inlined_function_input()) + || (meta.has_inline_attribute && !param.1.is_valid_non_inlined_function_input()) { let span = param.0.span(); errors.push(TypeCheckError::InvalidTypeForEntryPoint { span }); @@ -546,7 +546,7 @@ pub mod test { trait_constraints: Vec::new(), direct_generics: Vec::new(), is_entry_point: true, - has_inline_or_fold_attribute: false, + has_inline_attribute: false, }; interner.push_fn_meta(func_meta, func_id); diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index 57d3038a135..db54a40c8e7 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -126,8 +126,9 @@ pub struct FuncMeta { pub is_entry_point: bool, /// True if this function is marked with an attribute - /// that indicates it should not be inlined, such as `fold` or `inline(never)` - pub has_inline_or_fold_attribute: bool, + /// that indicates it should be inlined differently than the default (inline everything). + /// For example, such as `fold` (never inlined) or `no_predicates` (inlined after flattening) + pub has_inline_attribute: bool, } impl FuncMeta { diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 82e17ac3912..ebbc7fc9813 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -582,8 +582,8 @@ impl Attributes { self.function.as_ref().map_or(false, |func_attribute| func_attribute.is_foldable()) } - pub fn is_inline(&self) -> bool { - self.function.as_ref().map_or(false, |func_attribute| func_attribute.is_inline()) + pub fn is_no_predicates(&self) -> bool { + self.function.as_ref().map_or(false, |func_attribute| func_attribute.is_no_predicates()) } } @@ -645,10 +645,7 @@ impl Attribute { ["test"] => Attribute::Function(FunctionAttribute::Test(TestScope::None)), ["recursive"] => Attribute::Function(FunctionAttribute::Recursive), ["fold"] => Attribute::Function(FunctionAttribute::Fold), - ["inline", tag] => { - validate(tag)?; - Attribute::Function(FunctionAttribute::Inline(tag.to_string())) - } + ["no_predicates"] => Attribute::Function(FunctionAttribute::NoPredicates), ["test", name] => { validate(name)?; let malformed_scope = @@ -701,7 +698,7 @@ pub enum FunctionAttribute { Test(TestScope), Recursive, Fold, - Inline(String), + NoPredicates, } impl FunctionAttribute { @@ -738,8 +735,8 @@ impl FunctionAttribute { /// Check whether we have an `inline` attribute /// Although we also do not want to inline foldable functions, /// we keep the two attributes distinct for clarity. - pub fn is_inline(&self) -> bool { - matches!(self, FunctionAttribute::Inline(_)) + pub fn is_no_predicates(&self) -> bool { + matches!(self, FunctionAttribute::NoPredicates) } } @@ -752,7 +749,7 @@ impl fmt::Display for FunctionAttribute { FunctionAttribute::Oracle(ref k) => write!(f, "#[oracle({k})]"), FunctionAttribute::Recursive => write!(f, "#[recursive]"), FunctionAttribute::Fold => write!(f, "#[fold]"), - FunctionAttribute::Inline(ref k) => write!(f, "#[inline({k})]"), + FunctionAttribute::NoPredicates => write!(f, "#[no_predicates]"), } } } @@ -798,7 +795,7 @@ impl AsRef for FunctionAttribute { FunctionAttribute::Test { .. } => "", FunctionAttribute::Recursive => "", FunctionAttribute::Fold => "", - FunctionAttribute::Inline(string) => string, + FunctionAttribute::NoPredicates => "", } } } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 3961ad13043..34a8b27a242 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -213,10 +213,11 @@ pub enum InlineType { Inline, /// Functions marked as foldable will not be inlined and compiled separately into ACIR Fold, - /// Similar to `Fold`, these functions will not be inlined and compile separately into ACIR. - /// They are different from `Fold` though as they are expected to be inlined into the program + /// Functions marked to have no predicates will not be inlined in the default inlining pass + /// and will be separately inlined after the flattening pass. + /// They are different from `Fold` as they are expected to be inlined into the program /// entry point before being used in the backend. - Never, + NoPredicates, } impl From<&Attributes> for InlineType { @@ -224,13 +225,7 @@ impl From<&Attributes> for InlineType { attributes.function.as_ref().map_or(InlineType::default(), |func_attribute| { match func_attribute { FunctionAttribute::Fold => InlineType::Fold, - FunctionAttribute::Inline(tag) => { - if tag == "never" { - InlineType::Never - } else { - InlineType::default() - } - } + FunctionAttribute::NoPredicates => InlineType::NoPredicates, _ => InlineType::default(), } }) @@ -242,7 +237,7 @@ impl InlineType { match self { InlineType::Inline => false, InlineType::Fold => true, - InlineType::Never => false, + InlineType::NoPredicates => false, } } } @@ -252,7 +247,7 @@ impl std::fmt::Display for InlineType { match self { InlineType::Inline => write!(f, "inline"), InlineType::Fold => write!(f, "fold"), - InlineType::Never => write!(f, "inline(never)"), + InlineType::NoPredicates => write!(f, "no_predicates"), } } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index b2cc7eee9f8..5f99e9e347a 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1378,9 +1378,10 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { assert_eq!(get_program_errors(src).len(), 1); } + #[test] fn deny_inline_attribute_on_unconstrained() { let src = r#" - #[inline(never)] + #[no_predicates] unconstrained fn foo(x: Field, y: Field) { assert(x != y); } @@ -1389,7 +1390,9 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { assert_eq!(errors.len(), 1); assert!(matches!( errors[0].0, - CompilationError::ResolverError(ResolverError::InlineAttributeOnUnconstrained { .. }) + CompilationError::ResolverError( + ResolverError::NoPredicatesAttributeOnUnconstrained { .. } + ) )); } diff --git a/test_programs/execution_success/inline_never_basic/Nargo.toml b/test_programs/execution_success/no_predicates_basic/Nargo.toml similarity index 74% rename from test_programs/execution_success/inline_never_basic/Nargo.toml rename to test_programs/execution_success/no_predicates_basic/Nargo.toml index 16691770d76..bcefd550fb0 100644 --- a/test_programs/execution_success/inline_never_basic/Nargo.toml +++ b/test_programs/execution_success/no_predicates_basic/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "inline_never_basic" +name = "no_predicates_basic" type = "bin" authors = [""] compiler_version = ">=0.27.0" diff --git a/test_programs/execution_success/inline_never_basic/Prover.toml b/test_programs/execution_success/no_predicates_basic/Prover.toml similarity index 100% rename from test_programs/execution_success/inline_never_basic/Prover.toml rename to test_programs/execution_success/no_predicates_basic/Prover.toml diff --git a/test_programs/execution_success/inline_never_basic/src/main.nr b/test_programs/execution_success/no_predicates_basic/src/main.nr similarity index 83% rename from test_programs/execution_success/inline_never_basic/src/main.nr rename to test_programs/execution_success/no_predicates_basic/src/main.nr index 1922aaedb6c..05b886fca77 100644 --- a/test_programs/execution_success/inline_never_basic/src/main.nr +++ b/test_programs/execution_success/no_predicates_basic/src/main.nr @@ -2,7 +2,7 @@ fn main(x: Field, y: pub Field) { basic_check(x, y); } -#[inline(never)] +#[no_predicates_basic] fn basic_check(x: Field, y: Field) { assert(x != y); } diff --git a/test_programs/execution_success/inline_numeric_generic_poseidon/Nargo.toml b/test_programs/execution_success/no_predicates_numeric_generic_poseidon/Nargo.toml similarity index 63% rename from test_programs/execution_success/inline_numeric_generic_poseidon/Nargo.toml rename to test_programs/execution_success/no_predicates_numeric_generic_poseidon/Nargo.toml index af74e941d2b..1ce13c24287 100644 --- a/test_programs/execution_success/inline_numeric_generic_poseidon/Nargo.toml +++ b/test_programs/execution_success/no_predicates_numeric_generic_poseidon/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "inline_numeric_generic_poseidon" +name = "no_predicates_numeric_generic_poseidon" type = "bin" authors = [""] compiler_version = ">=0.28.0" diff --git a/test_programs/execution_success/inline_numeric_generic_poseidon/Prover.toml b/test_programs/execution_success/no_predicates_numeric_generic_poseidon/Prover.toml similarity index 100% rename from test_programs/execution_success/inline_numeric_generic_poseidon/Prover.toml rename to test_programs/execution_success/no_predicates_numeric_generic_poseidon/Prover.toml diff --git a/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr b/test_programs/execution_success/no_predicates_numeric_generic_poseidon/src/main.nr similarity index 87% rename from test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr rename to test_programs/execution_success/no_predicates_numeric_generic_poseidon/src/main.nr index 42d3342c114..dcb5e6bc5ca 100644 --- a/test_programs/execution_success/inline_numeric_generic_poseidon/src/main.nr +++ b/test_programs/execution_success/no_predicates_numeric_generic_poseidon/src/main.nr @@ -3,7 +3,7 @@ use dep::std::hash::{pedersen_hash_with_separator, poseidon2::Poseidon2}; global NUM_HASHES = 2; global HASH_LENGTH = 10; -#[inline(never)] +#[no_predicates] pub fn poseidon_hash(inputs: [Field; N]) -> Field { Poseidon2::hash(inputs, inputs.len()) } @@ -21,7 +21,7 @@ fn main( } } - // We want to make sure that the function marked with `#[inline(never)]` with a numeric generic + // We want to make sure that the function marked with `#[no_predicates]` with a numeric generic // is monomorphized correctly. let mut double_preimage = [0; 20]; for i in 0..HASH_LENGTH * 2 { From 313d6d000adea315c1c682e0babab28be405051b Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 30 Apr 2024 18:31:17 +0000 Subject: [PATCH 10/14] fix ignored tests and attribute name --- .../execution_success/no_predicates_basic/src/main.nr | 2 +- tooling/debugger/ignored-tests.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test_programs/execution_success/no_predicates_basic/src/main.nr b/test_programs/execution_success/no_predicates_basic/src/main.nr index 05b886fca77..d6037c2ab26 100644 --- a/test_programs/execution_success/no_predicates_basic/src/main.nr +++ b/test_programs/execution_success/no_predicates_basic/src/main.nr @@ -2,7 +2,7 @@ fn main(x: Field, y: pub Field) { basic_check(x, y); } -#[no_predicates_basic] +#[no_predicates] fn basic_check(x: Field, y: Field) { assert(x != y); } diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index 0c8624631df..f214be5e613 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -17,6 +17,6 @@ fold_basic_nested_call fold_call_witness_condition fold_after_inlined_calls fold_numeric_generic_poseidon -inline_never_basic -inline_numeric_generic_poseidon +no_predicates_basic +no_predicates_numeric_generic_poseidon regression_4709 From 648f9a1146c7f165d4ac2675d9611669375fa436 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 30 Apr 2024 19:17:16 +0000 Subject: [PATCH 11/14] switch to use separate inline entry point --- compiler/noirc_evaluator/src/ssa.rs | 6 +-- .../src/ssa/opt/flatten_cfg.rs | 3 -- .../noirc_evaluator/src/ssa/opt/inlining.rs | 45 ++++++++++++++----- .../src/ssa/ssa_gen/program.rs | 5 --- .../src/monomorphization/ast.rs | 2 + 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 6e0e31c0ae5..edc32fddeaf 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -61,9 +61,9 @@ pub(crate) fn optimize_into_acir( .run_pass(Ssa::remove_bit_shifts, "After Removing Bit Shifts:") // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores .run_pass(Ssa::mem2reg, "After Mem2Reg:") - // Run the inlining pass again as certain codegen attributes will now be disabled after flattening, - // such as treating functions marked with the `InlineType::NoPredicates` as an entry point. - .run_pass(Ssa::inline_functions, "After Inlining:") + // Run the inlining pass again to handle functions with `InlineType::NoPredicates`. + // Before flattening is run, we treat functions marked with the `InlineType::NoPredicates` as an entry point. + .run_pass(Ssa::inline_functions_with_no_predicates, "After Inlining:") .run_pass(Ssa::fold_constants, "After Constant Folding:") .run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffects removal:") .run_pass(Ssa::fold_constants_using_constraints, "After Constraint Folding:") diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index bb4cd5e75ee..07771397ce8 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -170,9 +170,6 @@ impl Ssa { for function in self.functions.values_mut() { flatten_function_cfg(function); } - // Now that flattening has been completed we can have SSA generation inline any - // calls to functions marked with `#[no_predicates]` - self.past_flattening_pass = true; self } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index e9277d6b1ba..cbe90c0cc16 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -38,14 +38,22 @@ impl Ssa { /// as well save the work for later instead of performing it twice. #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn inline_functions(mut self) -> Ssa { - self.functions = - btree_map(get_entry_point_functions(&self, self.past_flattening_pass), |entry_point| { - let new_function = InlineContext::new(&self, entry_point).inline_all(&self); - (entry_point, new_function) - }); + self.functions = btree_map(get_entry_point_functions(&self, true), |entry_point| { + let new_function = InlineContext::new(&self, entry_point, true).inline_all(&self); + (entry_point, new_function) + }); self } + + // Run the inlining pass where functions marked with `InlineType::NoPredicates` as not entry points + pub(crate) fn inline_functions_with_no_predicates(mut self) -> Ssa { + self.functions = btree_map(get_entry_point_functions(&self, false), |entry_point| { + let new_function = InlineContext::new(&self, entry_point, false).inline_all(&self); + (entry_point, new_function) + }); + self + } } /// The context for the function inlining pass. @@ -61,6 +69,8 @@ struct InlineContext { // The FunctionId of the entry point function we're inlining into in the old, unmodified Ssa. entry_point: FunctionId, + + no_predicates_is_entry_point: bool, } /// The per-function inlining context contains information that is only valid for one function. @@ -98,13 +108,17 @@ struct PerFunctionContext<'function> { /// should be left in the final program. /// This is the `main` function, any Acir functions with a [fold inline type][InlineType::Fold], /// and any brillig functions used. -fn get_entry_point_functions(ssa: &Ssa, past_flattening_pass: bool) -> BTreeSet { +fn get_entry_point_functions( + ssa: &Ssa, + no_predicates_is_entry_point: bool, +) -> BTreeSet { let functions = ssa.functions.iter(); let mut entry_points = functions .filter(|(_, function)| { // If we have not already finished the flattening pass, functions marked // to not have predicates should be marked as entry points. - let no_predicates_is_entry_point = !past_flattening_pass && function.is_no_predicates(); + let no_predicates_is_entry_point = + no_predicates_is_entry_point && function.is_no_predicates(); function.runtime().is_entry_point() || no_predicates_is_entry_point }) .map(|(id, _)| *id) @@ -120,11 +134,21 @@ impl InlineContext { /// The function being inlined into will always be the main function, although it is /// actually a copy that is created in case the original main is still needed from a function /// that could not be inlined calling it. - fn new(ssa: &Ssa, entry_point: FunctionId) -> InlineContext { + fn new( + ssa: &Ssa, + entry_point: FunctionId, + no_predicates_is_entry_point: bool, + ) -> InlineContext { let source = &ssa.functions[&entry_point]; let mut builder = FunctionBuilder::new(source.name().to_owned(), entry_point); builder.set_runtime(source.runtime()); - Self { builder, recursion_level: 0, entry_point, call_stack: CallStack::new() } + Self { + builder, + recursion_level: 0, + entry_point, + call_stack: CallStack::new(), + no_predicates_is_entry_point, + } } /// Start inlining the entry point function and all functions reachable from it. @@ -362,7 +386,8 @@ impl<'function> PerFunctionContext<'function> { // If we have not already finished the flattening pass, functions marked // to not have predicates should be marked as entry points. let no_predicates_is_entry_point = - !ssa.past_flattening_pass && function.is_no_predicates(); + self.context.no_predicates_is_entry_point + && function.is_no_predicates(); if function.runtime().is_entry_point() || no_predicates_is_entry_point { self.push_instruction(*id); } else { diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index bb03b218616..b05a2cbc741 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -17,10 +17,6 @@ pub(crate) struct Ssa { /// This mapping is necessary to use the correct function pointer for an ACIR call, /// as the final program artifact will be a list of only entry point functions. pub(crate) entry_point_to_generated_index: BTreeMap, - /// Flag that determines when to inline functions marked to not use predicates. - /// This is currently defaulted to `false` and should be set to true by after completion - /// of the flattening pass which sets side effects. - pub(crate) past_flattening_pass: bool, } impl Ssa { @@ -54,7 +50,6 @@ impl Ssa { main_id, next_id: AtomicCounter::starting_after(max_id), entry_point_to_generated_index, - past_flattening_pass: false, } } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 34a8b27a242..721c7d5f0b1 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -217,6 +217,8 @@ pub enum InlineType { /// and will be separately inlined after the flattening pass. /// They are different from `Fold` as they are expected to be inlined into the program /// entry point before being used in the backend. + /// This attribute is unsafe and can cause a function whose logic relies on predicates from + /// the flattening pass to fail. NoPredicates, } From daf5beb4b0835dcff043fa097d516bd88cba1c5b Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 30 Apr 2024 19:19:27 +0000 Subject: [PATCH 12/14] DRY for inline entry points --- .../noirc_evaluator/src/ssa/opt/inlining.rs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index cbe90c0cc16..1de07638fe4 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -36,22 +36,28 @@ impl Ssa { /// changes. This is because if the function's id later becomes known by a later /// pass, we would need to re-run all of inlining anyway to inline it, so we might /// as well save the work for later instead of performing it twice. + /// + /// There are some attributes that allow inlining a function at a different step of codegen. + /// Currently this is just `InlineType::NoPredicates` for which we have a flag indicating + /// whether treating that inline functions. The default is to treat these functions as entry points. #[tracing::instrument(level = "trace", skip(self))] - pub(crate) fn inline_functions(mut self) -> Ssa { - self.functions = btree_map(get_entry_point_functions(&self, true), |entry_point| { - let new_function = InlineContext::new(&self, entry_point, true).inline_all(&self); - (entry_point, new_function) - }); - - self + pub(crate) fn inline_functions(self) -> Ssa { + Self::inline_functions_inner(self, true) } // Run the inlining pass where functions marked with `InlineType::NoPredicates` as not entry points - pub(crate) fn inline_functions_with_no_predicates(mut self) -> Ssa { - self.functions = btree_map(get_entry_point_functions(&self, false), |entry_point| { - let new_function = InlineContext::new(&self, entry_point, false).inline_all(&self); - (entry_point, new_function) - }); + pub(crate) fn inline_functions_with_no_predicates(self) -> Ssa { + Self::inline_functions_inner(self, false) + } + + fn inline_functions_inner(mut self, no_predicates_is_entry_point: bool) -> Ssa { + self.functions = btree_map( + get_entry_point_functions(&self, no_predicates_is_entry_point), + |entry_point| { + let new_function = InlineContext::new(&self, entry_point, false).inline_all(&self); + (entry_point, new_function) + }, + ); self } } From 7b1ed2e3366566666d7802342a1fe30833d9f17f Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 30 Apr 2024 19:27:07 +0000 Subject: [PATCH 13/14] use no_predicates_is_entry_point flag --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 1de07638fe4..61507ddf287 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -54,7 +54,7 @@ impl Ssa { self.functions = btree_map( get_entry_point_functions(&self, no_predicates_is_entry_point), |entry_point| { - let new_function = InlineContext::new(&self, entry_point, false).inline_all(&self); + let new_function = InlineContext::new(&self, entry_point, no_predicates_is_entry_point).inline_all(&self); (entry_point, new_function) }, ); From 38a13713330f08cffc36bd95a3c331673fac9af2 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 30 Apr 2024 19:30:10 +0000 Subject: [PATCH 14/14] cargo fmt --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 61507ddf287..bddfb25f26c 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -54,7 +54,9 @@ impl Ssa { self.functions = btree_map( get_entry_point_functions(&self, no_predicates_is_entry_point), |entry_point| { - let new_function = InlineContext::new(&self, entry_point, no_predicates_is_entry_point).inline_all(&self); + let new_function = + InlineContext::new(&self, entry_point, no_predicates_is_entry_point) + .inline_all(&self); (entry_point, new_function) }, );