From a19afcb23874ee1fcae7dc9f2aca95992ee11630 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 4 Aug 2025 16:35:41 +0000 Subject: [PATCH 1/5] add various unit tests to inlining, inline info and call graph --- .../noirc_evaluator/src/ssa/ir/call_graph.rs | 168 ++++++++++++++++++ .../noirc_evaluator/src/ssa/opt/inlining.rs | 115 ++++++++++++ .../src/ssa/opt/inlining/inline_info.rs | 80 +++++++++ .../noirc_evaluator/src/ssa/validation/mod.rs | 22 +++ 4 files changed, 385 insertions(+) diff --git a/compiler/noirc_evaluator/src/ssa/ir/call_graph.rs b/compiler/noirc_evaluator/src/ssa/ir/call_graph.rs index 093dac3a452..2b73a233526 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/call_graph.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/call_graph.rs @@ -345,6 +345,76 @@ mod tests { assert!(recursive_functions.contains(&Id::test_new(3))); } + #[test] + fn mark_multiple_independent_recursion_cycles() { + // This test is an expanded version of `mark_mutually_recursive_functions` where we have multiple recursive cycles. + let src = " + acir(inline) fn main f0 { + b0(): + call f1() + call f4() + return + } + // First recursive cycle: f1 -> f2 -> f3 -> f1 + brillig(inline) fn starter f1 { + b0(): + call f2() + return + } + brillig(inline) fn ping f2 { + b0(): + call f3() + return + } + brillig(inline) fn pong f3 { + b0(): + call f1() + return + } + // Second recursive cycle: f4 <-> f5 + brillig(inline) fn foo f4 { + b0(): + call f5() + return + } + brillig(inline) fn bar f5 { + b0(): + call f4() + return + } + // Non-recursive leaf function + brillig(inline) fn baz f6 { + b0(): + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let call_graph = CallGraph::from_ssa_weighted(&ssa); + let recursive_functions = call_graph.get_recursive_functions(); + + // There should be 5 recursive functions: f1, f2, f3 (cycle 1), and f4, f5 (cycle 2) + let expected_recursive_ids = [1, 2, 3, 4, 5].map(Id::test_new).to_vec(); + + assert_eq!( + recursive_functions.len(), + expected_recursive_ids.len(), + "Expected {} recursive functions", + expected_recursive_ids.len() + ); + + for func_id in expected_recursive_ids { + assert!( + recursive_functions.contains(&func_id), + "Function {:?} should be marked recursive", + func_id + ); + } + + // f6 should not be marked recursive + assert!(!recursive_functions.contains(&Id::test_new(6))); + } + #[test] fn mark_self_recursive_function() { let src = " @@ -368,6 +438,82 @@ mod tests { assert!(recursive_functions.contains(&Id::test_new(1))); } + #[test] + fn self_recursive_and_calls_others() { + let src = " + acir(inline) fn main f0 { + b0(): + call f1() + return + } + brillig(inline) fn self_recur f1 { + b0(): + call f1() + call f2() + return + } + brillig(inline) fn foo f2 { + b0(): + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let call_graph = CallGraph::from_ssa_weighted(&ssa); + + let f0 = Id::test_new(0); + let f1 = Id::test_new(1); + let f2 = Id::test_new(2); + + let recursive = call_graph.get_recursive_functions(); + assert!(recursive.contains(&f1)); + assert!(!recursive.contains(&f0)); + assert!(!recursive.contains(&f2)); + + let callees = call_graph.callees(); + let f1_callees = callees.get(&f1).unwrap(); + assert_eq!(f1_callees.len(), 2); + assert_eq!(*f1_callees.get(&f1).unwrap(), 1, "f1 should call itself once"); + assert_eq!(*f1_callees.get(&f2).unwrap(), 1, "f1 should call f2 once"); + + let callers = call_graph.callers(); + let f1_callers = callers.get(&f1).unwrap(); + assert_eq!(f1_callers.len(), 2); + assert_eq!(*f1_callers.get(&f0).unwrap(), 1, "f0 calls f1 once"); + assert_eq!(*f1_callers.get(&f1).unwrap(), 1, "f1 calls itself once"); + + let f2_callers = callers.get(&f2).unwrap(); + assert_eq!(f2_callers.len(), 1); + assert_eq!(*f2_callers.get(&f1).unwrap(), 1, "f1 calls f2 once"); + + let f2_callees = callees.get(&f2).unwrap(); + assert!(f2_callees.is_empty(), "f2 should not call any functions"); + + let f0_callees = callees.get(&f0).unwrap(); + assert_eq!(f0_callees.len(), 1); + assert_eq!(*f0_callees.get(&f1).unwrap(), 1); + } + + #[test] + fn pure_self_recursive_function() { + let src = " + brillig(inline) fn self_recur f0 { + b0(): + call f0() + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let call_graph = CallGraph::from_ssa_weighted(&ssa); + + let recursive = call_graph.get_recursive_functions(); + assert!(recursive.contains(&Id::test_new(0))); + + let callers = call_graph.callers(); + let f0_callers = callers.get(&Id::test_new(0)).unwrap(); + assert_eq!(f0_callers.len(), 1); + assert_eq!(*f0_callers.get(&Id::test_new(0)).unwrap(), 1); + } + fn callers_and_callees_src() -> &'static str { r#" acir(inline) fn main f0 { @@ -506,4 +652,26 @@ mod tests { *times_called.get(&Id::test_new(4)).expect(" Should have times called"); assert_eq!(times_f4_called, 2); } + + #[test] + fn dead_function_not_called() { + let src = " + acir(inline) fn main f0 { + b0(): + return + } + brillig(inline) fn dead_code f1 { + b0(): + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let call_graph = CallGraph::from_ssa_weighted(&ssa); + + // f1 is never called, but it should still be tracked. + let times_called = call_graph.times_called(); + assert_eq!(*times_called.get(&Id::test_new(1)).unwrap(), 0); + assert!(call_graph.callers().get(&Id::test_new(1)).unwrap().is_empty()); + assert!(call_graph.callees().get(&Id::test_new(1)).unwrap().is_empty()); + } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index c923f38048b..89b7e456794 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -799,6 +799,27 @@ mod test { "); } + #[test] + fn basic_inlining_brillig_not_inlined_into_acir() { + // This test matches the `basic_inlining` test exactly except that f1 is marked as a Brillig runtime. + // We expect that Brillig entry points (e.g., Brillig functions called from ACIR) should never be inlined. + let src = " + acir(inline) fn foo f0 { + b0(): + v1 = call f1() -> Field + return v1 + } + + brillig(inline) fn bar f1 { + b0(): + return Field 72 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + assert_normalized_ssa_equals(ssa, src); + } + #[test] fn complex_inlining() { // This SSA is from issue #1327 which previously failed to inline properly @@ -1030,4 +1051,98 @@ mod test { } "); } + + #[test] + fn static_assertions_to_always_be_inlined() { + let src = " + brillig(inline) fn main f0 { + b0(): + call f1(Field 1) + return + } + brillig(inline) fn foo f1 { + b0(v0: Field): + call assert_constant(v0) + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + assert_ssa_snapshot!(ssa, @r" + brillig(inline) fn main f0 { + b0(): + return + } + "); + } + + #[test] + fn no_predicates_flag_inactive() { + let src = " + acir(inline) fn main f0 { + b0(): + call f1() + return + } + acir(no_predicates) fn no_predicates f1 { + b0(): + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + assert_normalized_ssa_equals(ssa, src); + } + + #[test] + fn no_predicates_flag_active() { + let src = " + acir(inline) fn main f0 { + b0(): + call f1() + return + } + acir(no_predicates) fn no_predicates f1 { + b0(): + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions_with_no_predicates(i64::MAX).unwrap(); + + assert_ssa_snapshot!(ssa, @r" + acir(inline) fn main f0 { + b0(): + return + } + "); + } + + #[test] + fn inline_always_function() { + let src = " + brillig(inline) fn main f0 { + b0(): + call f1() + return + } + + brillig(inline_always) fn always_inline f1 { + b0(): + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + assert_ssa_snapshot!(ssa, @r" + brillig(inline) fn main f0 { + b0(): + return + } + "); + } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining/inline_info.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining/inline_info.rs index 40cc9078521..fa49139fb3d 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining/inline_info.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining/inline_info.rs @@ -453,4 +453,84 @@ mod tests { ); assert!(tws[3] > std::cmp::max(tws[1], tws[2]), "ideally 'main' has the most weight"); } + + #[test] + fn mark_static_assertions_to_always_be_inlined() { + let src = " + brillig(inline) fn main f0 { + b0(): + call f1(Field 1) + return + } + brillig(inline) fn foo f1 { + b0(v0: Field): + call assert_constant(v0) + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let call_graph = CallGraph::from_ssa_weighted(&ssa); + let infos = compute_inline_infos(&ssa, &call_graph, false, 0); + + let f1 = infos.get(&Id::test_new(1)).expect("f1 should be analyzed"); + assert!( + f1.contains_static_assertion, + "f1 should be marked as containing a static assertion" + ); + assert!(f1.should_inline, "f1 should be inlined due to static assertion"); + } + + #[test] + fn no_predicates() { + let src = " + acir(inline) fn main f0 { + b0(): + call f1() + return + } + acir(no_predicates) fn no_predicates f1 { + b0(): + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let call_graph = CallGraph::from_ssa_weighted(&ssa); + let infos = compute_inline_infos(&ssa, &call_graph, false, 0); + + let f1 = infos.get(&Id::test_new(1)).expect("Should analyze f1"); + assert!( + !f1.should_inline, + "no_predicates functions should NOT be inlined if the flag is false" + ); + + let infos = compute_inline_infos(&ssa, &call_graph, true, 0); + + let f1 = infos.get(&Id::test_new(1)).expect("Should analyze f1"); + assert!(f1.should_inline, "no_predicates functions should be inlined if the flag is true"); + } + + #[test] + fn inline_always_functions_are_inlined() { + let src = " + brillig(inline) fn main f0 { + b0(): + call f1() + return + } + + brillig(inline_always) fn always_inline f1 { + b0(): + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let call_graph = CallGraph::from_ssa_weighted(&ssa); + let infos = compute_inline_infos(&ssa, &call_graph, false, 0); + + let f1 = infos.get(&Id::test_new(1)).expect("Should analyze f1"); + assert!(f1.should_inline, "inline_always functions should be inlined"); + } } diff --git a/compiler/noirc_evaluator/src/ssa/validation/mod.rs b/compiler/noirc_evaluator/src/ssa/validation/mod.rs index cc826f9f594..cb9b5d678b5 100644 --- a/compiler/noirc_evaluator/src/ssa/validation/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/validation/mod.rs @@ -665,6 +665,28 @@ mod tests { let _ = Ssa::from_str(src).unwrap(); } + #[test] + #[should_panic(expected = "Function f1 has multiple return blocks")] + fn multiple_return_blocks() { + let src = " + acir(inline) fn main f0 { + b0(): + v1 = call f1(u1 1) -> Field + return v1 + } + + acir(inline) fn f1 f1 { + b0(v0: u1): + jmpif v0 then: b1, else: b2 + b1(): + return Field 1 + b2(): + return Field 2 + } + "; + let _ = Ssa::from_str(src).unwrap(); + } + #[test] #[should_panic( expected = "MakeArray returns an array of flattened length 2, but it has 3 elements" From 9220bdc973fe11a8a1c152e242dbee5516bbbe8e Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 4 Aug 2025 16:44:44 +0000 Subject: [PATCH 2/5] cleanup tests --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 1 - compiler/noirc_evaluator/src/ssa/validation/mod.rs | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 89b7e456794..2a085973213 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -809,7 +809,6 @@ mod test { v1 = call f1() -> Field return v1 } - brillig(inline) fn bar f1 { b0(): return Field 72 diff --git a/compiler/noirc_evaluator/src/ssa/validation/mod.rs b/compiler/noirc_evaluator/src/ssa/validation/mod.rs index cb9b5d678b5..7c4cb67637d 100644 --- a/compiler/noirc_evaluator/src/ssa/validation/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/validation/mod.rs @@ -670,17 +670,17 @@ mod tests { fn multiple_return_blocks() { let src = " acir(inline) fn main f0 { - b0(): + b0(): v1 = call f1(u1 1) -> Field return v1 } acir(inline) fn f1 f1 { - b0(v0: u1): + b0(v0: u1): jmpif v0 then: b1, else: b2 - b1(): + b1(): return Field 1 - b2(): + b2(): return Field 2 } "; From 6c971acb10d5142d55aad26ccbd80fcfe9cb3a71 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 4 Aug 2025 16:47:31 +0000 Subject: [PATCH 3/5] clippy --- compiler/noirc_evaluator/src/ssa/ir/call_graph.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/call_graph.rs b/compiler/noirc_evaluator/src/ssa/ir/call_graph.rs index 2b73a233526..56b4ca4d8b7 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/call_graph.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/call_graph.rs @@ -406,8 +406,7 @@ mod tests { for func_id in expected_recursive_ids { assert!( recursive_functions.contains(&func_id), - "Function {:?} should be marked recursive", - func_id + "Function {func_id} should be marked recursive", ); } From 66b62fb98b74ec41f6b810366e040d64f22701ab Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 5 Aug 2025 09:17:19 -0400 Subject: [PATCH 4/5] Update compiler/noirc_evaluator/src/ssa/opt/inlining.rs Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- 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 2a085973213..c46500e213a 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -1135,7 +1135,7 @@ mod test { } "; let ssa = Ssa::from_str(src).unwrap(); - let ssa = ssa.inline_functions(i64::MAX).unwrap(); + let ssa = ssa.inline_functions(i64::MIN).unwrap(); assert_ssa_snapshot!(ssa, @r" brillig(inline) fn main f0 { From c50821e999fe16a001ad84b25e6eab9a2d93acfb Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 5 Aug 2025 14:33:39 +0000 Subject: [PATCH 5/5] update inline_always unit test --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index c46500e213a..f3e1969a8e0 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -1136,12 +1136,18 @@ mod test { "; let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.inline_functions(i64::MIN).unwrap(); - assert_ssa_snapshot!(ssa, @r" brillig(inline) fn main f0 { b0(): return } "); + + // Check that with a minimum inliner aggressiveness we do not inline a function + // not marked with `inline_always` + let no_inline_always_src = &src.replace("inline_always", "inline"); + let ssa = Ssa::from_str(no_inline_always_src).unwrap(); + let ssa = ssa.inline_functions(i64::MIN).unwrap(); + assert_normalized_ssa_equals(ssa, no_inline_always_src); } }