From 20cc3852988df64552367d1cc81ea1e9078a00e8 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 5 Aug 2025 20:26:49 +0000 Subject: [PATCH 1/7] more unit tests --- .../noirc_evaluator/src/ssa/opt/inlining.rs | 236 +++++++++++++++++- .../src/ssa/opt/inlining/inline_info.rs | 28 +++ 2 files changed, 259 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index c923f38048b..41393ad87ca 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -436,10 +436,8 @@ impl<'function> PerFunctionContext<'function> { } let translated_block_id = self.translate_block(source_block_id, &mut block_queue); self.context.builder.switch_to_block(translated_block_id); - seen_blocks.insert(source_block_id); self.inline_block_instructions(ssa, source_block_id, should_inline_call)?; - if let Some((block, values)) = self.handle_terminator_instruction(source_block_id, &mut block_queue) { @@ -509,9 +507,9 @@ impl<'function> PerFunctionContext<'function> { // can pollute the function they're being inlined into with `Instruction::EnabledSideEffects`, // resulting in predicates not being applied properly. // - // Note that this doesn't cover the case in which there exists an `Instruction::EnabledSideEffects` + // Note that this doesn't cover the case in which there exists an `Instruction::EnableSideEffectsIf` // within the function being inlined whilst the source function has not encountered one yet. - // In practice this isn't an issue as the last `Instruction::EnabledSideEffects` in the + // In practice this isn't an issue as the last `Instruction::EnableSideEffectsIf` in the // function being inlined will be to turn off predicates rather than to create one. if let Some(condition) = side_effects_enabled { self.context.builder.insert_enable_side_effects_if(condition); @@ -772,7 +770,7 @@ impl<'function> PerFunctionContext<'function> { mod test { use crate::{ assert_ssa_snapshot, - ssa::{Ssa, opt::assert_normalized_ssa_equals}, + ssa::{Ssa, ir::instruction::TerminatorInstruction, opt::assert_normalized_ssa_equals}, }; #[test] @@ -1030,4 +1028,232 @@ mod test { } "); } + + #[test] + fn conditional_inlining_const_from_param_and_direct_constant() { + let src = " + brillig(inline) fn foo f0 { + b0(): + v1 = call f1() -> Field + v2 = call f2(u1 1) -> Field + v3 = call f2(u1 0) -> Field + return v1, v2, v3 + } + + brillig(inline) fn bar f1 { + b0(): + jmpif u1 1 then: b1, else: b2 + b1(): + jmp b3(Field 1) + b2(): + jmp b3(Field 2) + b3(v3: Field): + return v3 + } + + brillig(inline) fn baz f2 { + b0(v0: u1): + jmpif v0 then: b1, else: b2 + b1(): + jmp b3(Field 1) + b2(): + jmp b3(Field 2) + b3(v3: Field): + return v3 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + // We expect a block from all calls to f1 and f2 to be pruned and that the constant argument to the f2 call + // is propagated to the jmpif conditional in b0. + // Field 1 to be returned from the first call to f2 and Field 2 should be returned from the second call to f2. + assert_ssa_snapshot!(ssa, @r" + brillig(inline) fn foo f0 { + b0(): + jmp b1() + b1(): + jmp b2(Field 1) + b2(v0: Field): + jmp b3() + b3(): + jmp b4(Field 1) + b4(v2: Field): + jmp b5() + b5(): + jmp b6(Field 2) + b6(v3: Field): + return v0, v2, v3 + } + "); + } + + #[test] + fn acir_global_arrays_are_inlined_with_new_value_ids() { + let src = " + g0 = Field 1 + g1 = Field 2 + g2 = make_array [Field 1, Field 2] : [Field; 2] + + acir(inline) fn main f0 { + b0(): + v0 = call f1() -> [Field; 2] + // v1 = array_get g2, index u32 1 -> Field + return v0 + } + acir(inline) fn create_array f1 { + b0(): + return g2 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + assert_ssa_snapshot!(ssa, @r" + g0 = Field 1 + g1 = Field 2 + g2 = make_array [Field 1, Field 2] : [Field; 2] + + acir(inline) fn main f0 { + b0(): + v3 = make_array [Field 1, Field 2] : [Field; 2] + return v3 + } + "); + } + + #[test] + fn brillig_global_arrays_keep_same_value_ids() { + let src = " + g0 = Field 1 + g1 = Field 2 + g2 = make_array [Field 1, Field 2] : [Field; 2] + + brillig(inline) fn main f0 { + b0(): + v0 = call f1() -> [Field; 2] + // v1 = array_get g2, index u32 1 -> Field + return v0 + } + brillig(inline) fn create_array f1 { + b0(): + return g2 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + assert_ssa_snapshot!(ssa, @r" + g0 = Field 1 + g1 = Field 2 + g2 = make_array [Field 1, Field 2] : [Field; 2] + + brillig(inline) fn main f0 { + b0(): + return g2 + } + "); + } + + #[test] + fn acir_global_constants_are_inlined_with_new_value_ids() { + let src = " + g0 = Field 1 + + acir(inline) fn main f0 { + b0(): + v0 = call f1() -> Field + return v0 + } + acir(inline) fn get_constant f1 { + b0(): + return g0 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + // The string output of global constants resolve to their inner values, so we need to check whether they are globals explicitly. + let main = ssa.main(); + let entry_block = main.entry_block(); + let terminator = main.dfg[entry_block].unwrap_terminator(); + let TerminatorInstruction::Return { return_values, .. } = terminator else { + panic!("Expected return"); + }; + assert_eq!(return_values.len(), 1); + // TODO(https://github.com/noir-lang/noir/issues/9408) + // assert!(!main.dfg.is_global(return_values[0])); + + assert_ssa_snapshot!(ssa, @r" + g0 = Field 1 + + acir(inline) fn main f0 { + b0(): + return Field 1 + } + "); + } + + #[test] + fn brillig_global_constants_keep_same_value_ids() { + let src = " + g0 = Field 1 + + brillig(inline) fn main f0 { + b0(): + v0 = call f1() -> Field + return v0 + } + brillig(inline) fn get_constant f1 { + b0(): + return g0 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + // The string output of global constants resolve to their inner values, so we need to check whether they are globals explicitly. + let main = ssa.main(); + let entry_block = main.entry_block(); + let terminator = main.dfg[entry_block].unwrap_terminator(); + let TerminatorInstruction::Return { return_values, .. } = terminator else { + panic!("Expected return"); + }; + assert_eq!(return_values.len(), 1); + assert!(main.dfg.is_global(return_values[0])); + + assert_ssa_snapshot!(ssa, @r" + g0 = Field 1 + + brillig(inline) fn main f0 { + b0(): + return Field 1 + } + "); + } + + #[test] + #[should_panic( + expected = "Unreachable terminator instruction should not exist during inlining" + )] + fn inlining_unreachable_block() { + let src = " + acir(inline) fn foo f0 { + b0(): + v1 = call f1() -> Field + return v1 + } + + acir(inline) fn bar f1 { + b0(): + unreachable + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let _ = ssa.inline_functions(i64::MAX).unwrap(); + } } 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..85f1d408c63 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,32 @@ mod tests { ); assert!(tws[3] > std::cmp::max(tws[1], tws[2]), "ideally 'main' has the most weight"); } + + #[test] + fn no_predicates_marked_as_acir_entry_point() { + let src = " + acir(inline) fn main f0 { + b0(v0: u32): + v1 = call f1(v0) -> u32 + return v1 + } + acir(no_predicates) fn no_predicates f1 { + b0(v0: u32): + return v0 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let call_graph = CallGraph::from_ssa_weighted(&ssa); + let inline_infos_do_not_inline_no_pred = + compute_inline_infos(&ssa, &call_graph, false, i64::MAX); + let f1 = Id::test_new(1); + + let f1_info = + inline_infos_do_not_inline_no_pred.get(&f1).expect("Should have f1 inline info"); + assert!(f1_info.is_acir_entry_point); + + let inline_infos_inline_no_pred = compute_inline_infos(&ssa, &call_graph, true, i64::MAX); + let f1_info = inline_infos_inline_no_pred.get(&f1).expect("Should have f1 inline info"); + assert!(!f1_info.is_acir_entry_point); + } } From 23d33e0ca40e402bb008251b9ddbf9c2f6cef8ca Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 5 Aug 2025 20:28:35 +0000 Subject: [PATCH 2/7] reduce diff --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 41393ad87ca..ebf59f60f7f 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -436,8 +436,10 @@ impl<'function> PerFunctionContext<'function> { } let translated_block_id = self.translate_block(source_block_id, &mut block_queue); self.context.builder.switch_to_block(translated_block_id); + seen_blocks.insert(source_block_id); self.inline_block_instructions(ssa, source_block_id, should_inline_call)?; + if let Some((block, values)) = self.handle_terminator_instruction(source_block_id, &mut block_queue) { From 280824f40fae58b022e4c0ac2d8c7fa98437fbfc Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 5 Aug 2025 20:31:01 +0000 Subject: [PATCH 3/7] conflicts --- .../noirc_evaluator/src/ssa/opt/inlining.rs | 218 ++++++++++++++++++ 1 file changed, 218 insertions(+) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index f815e034573..591750fb732 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -1051,6 +1051,63 @@ mod test { "); } + #[test] + fn conditional_inlining_const_from_param_and_direct_constant() { + let src = " + brillig(inline) fn foo f0 { + b0(): + v1 = call f1() -> Field + v2 = call f2(u1 1) -> Field + v3 = call f2(u1 0) -> Field + return v1, v2, v3 + } + brillig(inline) fn bar f1 { + b0(): + jmpif u1 1 then: b1, else: b2 + b1(): + jmp b3(Field 1) + b2(): + jmp b3(Field 2) + b3(v3: Field): + return v3 + } + brillig(inline) fn baz f2 { + b0(v0: u1): + jmpif v0 then: b1, else: b2 + b1(): + jmp b3(Field 1) + b2(): + jmp b3(Field 2) + b3(v3: Field): + return v3 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + // We expect a block from all calls to f1 and f2 to be pruned and that the constant argument to the f2 call + // is propagated to the jmpif conditional in b0. + // Field 1 to be returned from the first call to f2 and Field 2 should be returned from the second call to f2. + assert_ssa_snapshot!(ssa, @r" + brillig(inline) fn foo f0 { + b0(): + jmp b1() + b1(): + jmp b2(Field 1) + b2(v0: Field): + jmp b3() + b3(): + jmp b4(Field 1) + b4(v2: Field): + jmp b5() + b5(): + jmp b6(Field 2) + b6(v3: Field): + return v0, v2, v3 + } + "); + } + #[test] fn static_assertions_to_always_be_inlined() { let src = " @@ -1150,4 +1207,165 @@ mod test { let ssa = ssa.inline_functions(i64::MIN).unwrap(); assert_normalized_ssa_equals(ssa, no_inline_always_src); } + + #[test] + fn acir_global_arrays_are_inlined_with_new_value_ids() { + let src = " + g0 = Field 1 + g1 = Field 2 + g2 = make_array [Field 1, Field 2] : [Field; 2] + acir(inline) fn main f0 { + b0(): + v0 = call f1() -> [Field; 2] + // v1 = array_get g2, index u32 1 -> Field + return v0 + } + acir(inline) fn create_array f1 { + b0(): + return g2 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + assert_ssa_snapshot!(ssa, @r" + g0 = Field 1 + g1 = Field 2 + g2 = make_array [Field 1, Field 2] : [Field; 2] + acir(inline) fn main f0 { + b0(): + v3 = make_array [Field 1, Field 2] : [Field; 2] + return v3 + } + "); + } + + #[test] + fn brillig_global_arrays_keep_same_value_ids() { + let src = " + g0 = Field 1 + g1 = Field 2 + g2 = make_array [Field 1, Field 2] : [Field; 2] + brillig(inline) fn main f0 { + b0(): + v0 = call f1() -> [Field; 2] + // v1 = array_get g2, index u32 1 -> Field + return v0 + } + brillig(inline) fn create_array f1 { + b0(): + return g2 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + assert_ssa_snapshot!(ssa, @r" + g0 = Field 1 + g1 = Field 2 + g2 = make_array [Field 1, Field 2] : [Field; 2] + brillig(inline) fn main f0 { + b0(): + return g2 + } + "); + } + + #[test] + fn acir_global_constants_are_inlined_with_new_value_ids() { + let src = " + g0 = Field 1 + acir(inline) fn main f0 { + b0(): + v0 = call f1() -> Field + return v0 + } + acir(inline) fn get_constant f1 { + b0(): + return g0 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + // The string output of global constants resolve to their inner values, so we need to check whether they are globals explicitly. + let main = ssa.main(); + let entry_block = main.entry_block(); + let terminator = main.dfg[entry_block].unwrap_terminator(); + let TerminatorInstruction::Return { return_values, .. } = terminator else { + panic!("Expected return"); + }; + assert_eq!(return_values.len(), 1); + // TODO(https://github.com/noir-lang/noir/issues/9408) + // assert!(!main.dfg.is_global(return_values[0])); + + assert_ssa_snapshot!(ssa, @r" + g0 = Field 1 + acir(inline) fn main f0 { + b0(): + return Field 1 + } + "); + } + + #[test] + fn brillig_global_constants_keep_same_value_ids() { + let src = " + g0 = Field 1 + + brillig(inline) fn main f0 { + b0(): + v0 = call f1() -> Field + return v0 + } + brillig(inline) fn get_constant f1 { + b0(): + return g0 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.inline_functions(i64::MAX).unwrap(); + + // The string output of global constants resolve to their inner values, so we need to check whether they are globals explicitly. + let main = ssa.main(); + let entry_block = main.entry_block(); + let terminator = main.dfg[entry_block].unwrap_terminator(); + let TerminatorInstruction::Return { return_values, .. } = terminator else { + panic!("Expected return"); + }; + assert_eq!(return_values.len(), 1); + assert!(main.dfg.is_global(return_values[0])); + + assert_ssa_snapshot!(ssa, @r" + g0 = Field 1 + brillig(inline) fn main f0 { + b0(): + return Field 1 + } + "); + } + + #[test] + #[should_panic( + expected = "Unreachable terminator instruction should not exist during inlining" + )] + fn inlining_unreachable_block() { + let src = " + acir(inline) fn foo f0 { + b0(): + v1 = call f1() -> Field + return v1 + } + acir(inline) fn bar f1 { + b0(): + unreachable + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let _ = ssa.inline_functions(i64::MAX).unwrap(); + } } From e71436bad81b93da3f8ca3fb3d271bcb14ed0823 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 5 Aug 2025 20:31:42 +0000 Subject: [PATCH 4/7] fmt --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 591750fb732..4e2d3ea5653 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -436,7 +436,7 @@ impl<'function> PerFunctionContext<'function> { } let translated_block_id = self.translate_block(source_block_id, &mut block_queue); self.context.builder.switch_to_block(translated_block_id); - + seen_blocks.insert(source_block_id); self.inline_block_instructions(ssa, source_block_id, should_inline_call)?; @@ -1051,7 +1051,7 @@ mod test { "); } - #[test] + #[test] fn conditional_inlining_const_from_param_and_direct_constant() { let src = " brillig(inline) fn foo f0 { @@ -1208,7 +1208,7 @@ mod test { assert_normalized_ssa_equals(ssa, no_inline_always_src); } - #[test] + #[test] fn acir_global_arrays_are_inlined_with_new_value_ids() { let src = " g0 = Field 1 From d220d723e87a2e40d7e0455179ca09faf1f09148 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 6 Aug 2025 15:36:01 +0000 Subject: [PATCH 5/7] fixup tests --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 4e2d3ea5653..6d9a338188e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -1098,12 +1098,12 @@ mod test { jmp b3() b3(): jmp b4(Field 1) - b4(v2: Field): + b4(v1: Field): jmp b5() b5(): jmp b6(Field 2) - b6(v3: Field): - return v0, v2, v3 + b6(v2: Field): + return v0, v1, v2 } "); } @@ -1214,6 +1214,7 @@ mod test { g0 = Field 1 g1 = Field 2 g2 = make_array [Field 1, Field 2] : [Field; 2] + acir(inline) fn main f0 { b0(): v0 = call f1() -> [Field; 2] @@ -1233,6 +1234,7 @@ mod test { g0 = Field 1 g1 = Field 2 g2 = make_array [Field 1, Field 2] : [Field; 2] + acir(inline) fn main f0 { b0(): v3 = make_array [Field 1, Field 2] : [Field; 2] @@ -1247,6 +1249,7 @@ mod test { g0 = Field 1 g1 = Field 2 g2 = make_array [Field 1, Field 2] : [Field; 2] + brillig(inline) fn main f0 { b0(): v0 = call f1() -> [Field; 2] @@ -1266,6 +1269,7 @@ mod test { g0 = Field 1 g1 = Field 2 g2 = make_array [Field 1, Field 2] : [Field; 2] + brillig(inline) fn main f0 { b0(): return g2 @@ -1277,6 +1281,7 @@ mod test { fn acir_global_constants_are_inlined_with_new_value_ids() { let src = " g0 = Field 1 + acir(inline) fn main f0 { b0(): v0 = call f1() -> Field @@ -1304,6 +1309,7 @@ mod test { assert_ssa_snapshot!(ssa, @r" g0 = Field 1 + acir(inline) fn main f0 { b0(): return Field 1 @@ -1342,6 +1348,7 @@ mod test { assert_ssa_snapshot!(ssa, @r" g0 = Field 1 + brillig(inline) fn main f0 { b0(): return Field 1 From 8145176218484f04812858bd74e1ea4b5fbc7a94 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 6 Aug 2025 13:05:35 -0400 Subject: [PATCH 6/7] Update compiler/noirc_evaluator/src/ssa/opt/inlining.rs --- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 6d9a338188e..a92a28d058f 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -1218,7 +1218,6 @@ mod test { acir(inline) fn main f0 { b0(): v0 = call f1() -> [Field; 2] - // v1 = array_get g2, index u32 1 -> Field return v0 } acir(inline) fn create_array f1 { From d22caaf0229fec2f46112bf3a24147b7fc910035 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 6 Aug 2025 17:24:12 +0000 Subject: [PATCH 7/7] bump sha256 test timeout --- EXTERNAL_NOIR_LIBRARIES.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/EXTERNAL_NOIR_LIBRARIES.yml b/EXTERNAL_NOIR_LIBRARIES.yml index d19d14b82e2..1874998a569 100644 --- a/EXTERNAL_NOIR_LIBRARIES.yml +++ b/EXTERNAL_NOIR_LIBRARIES.yml @@ -54,7 +54,7 @@ libraries: critical: false sha256: repo: noir-lang/sha256 - timeout: 3 + timeout: 10 critical: true sha512: repo: noir-lang/sha512