From 18c8596aa5a2539f8b1f1669d8ed0bdc9af984b9 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 6 Aug 2025 20:20:59 +0000 Subject: [PATCH 1/5] add various tests confirming the functionality of remove_paired_rc --- compiler/noirc_evaluator/src/ssa/opt/rc.rs | 184 ++++++++++++++++++++- 1 file changed, 181 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/rc.rs b/compiler/noirc_evaluator/src/ssa/opt/rc.rs index e53f3c0b553..36176a0d072 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/rc.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/rc.rs @@ -154,9 +154,13 @@ fn remove_instructions(to_remove: HashSet, function: &mut Functio #[cfg(test)] mod test { - use crate::ssa::{ - ir::{basic_block::BasicBlockId, dfg::DataFlowGraph, instruction::Instruction}, - ssa_gen::Ssa, + use crate::{ + assert_ssa_snapshot, + ssa::{ + ir::{basic_block::BasicBlockId, dfg::DataFlowGraph, instruction::Instruction}, + opt::assert_normalized_ssa_equals, + ssa_gen::Ssa, + }, }; fn count_inc_rcs(block: BasicBlockId, dfg: &DataFlowGraph) -> usize { @@ -265,4 +269,178 @@ mod test { assert_eq!(count_inc_rcs(entry, &main.dfg), 1); assert_eq!(count_dec_rcs(entry, &main.dfg), 1); } + + #[test] + fn lone_inc_rc() { + let src = " + brillig(inline) fn foo f0 { + b0(v0: [Field; 2]): + inc_rc v0 + return v0 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_paired_rc(); + assert_normalized_ssa_equals(ssa, src); + } + + #[test] + fn lone_dec_rc() { + let src = " + brillig(inline) fn foo f0 { + b0(v0: [Field; 2]): + dec_rc v0 + return v0 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_paired_rc(); + assert_normalized_ssa_equals(ssa, src); + } + + #[test] + fn multiple_rc_pairs_mutation_on_different_types() { + let src = " + brillig(inline) fn mutator f0 { + b0(v0: [Field; 3], v1: [Field; 5]): + inc_rc v0 + inc_rc v1 + v2 = allocate -> &mut [Field; 3] + store v0 at v2 + v3 = load v2 -> [Field; 3] + v6 = array_set v3, index u32 0, value Field 5 + store v6 at v2 + v8 = array_get v1, index u32 1 -> Field + dec_rc v0 + dec_rc v1 + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_paired_rc(); + // We expect the paired RC on v0 to remain, but we expect the paired RC on v1 to be removed + // as they operate over different types ([Field; 2] and [Field; 5]) respectively. + assert_ssa_snapshot!(ssa, @r" + brillig(inline) fn mutator f0 { + b0(v0: [Field; 3], v1: [Field; 5]): + inc_rc v0 + v2 = allocate -> &mut [Field; 3] + store v0 at v2 + v3 = load v2 -> [Field; 3] + v6 = array_set v3, index u32 0, value Field 5 + store v6 at v2 + v8 = array_get v1, index u32 1 -> Field + dec_rc v0 + return + } + "); + } + + #[test] + fn multiple_rc_pairs_mutation_on_matching_types() { + let src = " + brillig(inline) fn mutator f0 { + b0(v0: [Field; 5], v1: [Field; 5]): + inc_rc v0 + inc_rc v1 + v2 = allocate -> &mut [Field; 5] + store v0 at v2 + v3 = load v2 -> [Field; 5] + v6 = array_set v3, index u32 0, value Field 5 + store v6 at v2 + v8 = array_get v1, index u32 1 -> Field + dec_rc v0 + dec_rc v1 + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_paired_rc(); + // We expect the paired RC on v0 to remain, but we expect the paired RC on v1 to be removed + // as they operate over different types ([Field; 2] and [Field; 5]) respectively. + assert_ssa_snapshot!(ssa, @r" + brillig(inline) fn mutator f0 { + b0(v0: [Field; 5], v1: [Field; 5]): + inc_rc v0 + inc_rc v1 + v2 = allocate -> &mut [Field; 5] + store v0 at v2 + v3 = load v2 -> [Field; 5] + v6 = array_set v3, index u32 0, value Field 5 + store v6 at v2 + v8 = array_get v1, index u32 1 -> Field + dec_rc v0 + dec_rc v1 + return + } + "); + } + + #[test] + fn rc_pair_with_same_type_but_different_values() { + let src = " + brillig(inline) fn foo f0 { + b0(v0: [Field; 2], v1: [Field; 2]): + inc_rc v0 + dec_rc v1 + v2 = make_array [v0] : [[Field; 2]; 1] + return v2 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_paired_rc(); + assert_normalized_ssa_equals(ssa, src); + } + + #[test] + fn do_not_remove_pairs_across_blocks() { + let src = " + brillig(inline) fn foo f0 { + b0(v0: [Field; 2]): + inc_rc v0 + jmp b1() + b1(): + dec_rc v0 + jmp b2() + b2(): + v1 = make_array [v0] : [[Field; 2]; 1] + return v1 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_paired_rc(); + assert_normalized_ssa_equals(ssa, src); + } + + #[test] + fn remove_pair_across_blocks() { + let src = " + brillig(inline) fn foo f0 { + b0(v0: [Field; 2]): + inc_rc v0 + jmp b1() + b1(): + jmp b2() + b2(): + dec_rc v0 + v1 = make_array [v0] : [[Field; 2]; 1] + return v1 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_paired_rc(); + assert_ssa_snapshot!(ssa, @r" + brillig(inline) fn foo f0 { + b0(v0: [Field; 2]): + jmp b1() + b1(): + jmp b2() + b2(): + v1 = make_array [v0] : [[Field; 2]; 1] + return v1 + } + "); + } } From 502d88ff515c3649fd9c55b7be96acc9c66abde2 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 6 Aug 2025 20:24:43 +0000 Subject: [PATCH 2/5] remove old comment --- 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 a92a28d058f..5314190fa23 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -1252,7 +1252,6 @@ mod test { 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 { From 0f46e791aa5b9da5b1909e1450d559d7a67c0c46 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 20 Aug 2025 15:14:11 -0400 Subject: [PATCH 3/5] Update compiler/noirc_evaluator/src/ssa/opt/rc.rs --- compiler/noirc_evaluator/src/ssa/opt/rc.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/rc.rs b/compiler/noirc_evaluator/src/ssa/opt/rc.rs index 36176a0d072..b1fd2bccfda 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/rc.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/rc.rs @@ -358,8 +358,7 @@ mod test { let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.remove_paired_rc(); - // We expect the paired RC on v0 to remain, but we expect the paired RC on v1 to be removed - // as they operate over different types ([Field; 2] and [Field; 5]) respectively. + // We expect the paired RCs on v0 and v1 to remain as they operate over the same type ([Field; 5]) assert_ssa_snapshot!(ssa, @r" brillig(inline) fn mutator f0 { b0(v0: [Field; 5], v1: [Field; 5]): From 0ae8eb9b24ada46b8715e484d3efc0ee8b9362a1 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 20 Aug 2025 19:35:38 +0000 Subject: [PATCH 4/5] update comments --- compiler/noirc_evaluator/src/ssa/opt/rc.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/noirc_evaluator/src/ssa/opt/rc.rs b/compiler/noirc_evaluator/src/ssa/opt/rc.rs index b1fd2bccfda..77d0a04e1dd 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/rc.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/rc.rs @@ -410,6 +410,8 @@ mod test { "; let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.remove_paired_rc(); + // This pass is very conservative and only looks for inc_rc's in the entry block and dec_rc's in the exit block + // The dec_rc is not in the return block so we do not expect the rc pair to be removed. assert_normalized_ssa_equals(ssa, src); } @@ -430,6 +432,8 @@ mod test { "; let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.remove_paired_rc(); + // As the program has an RC pair where the increment is in the entry block and + // the decrement is in the return block this pair is safe to remove. assert_ssa_snapshot!(ssa, @r" brillig(inline) fn foo f0 { b0(v0: [Field; 2]): From 3bc0819fb2ec5f8dfcd3ec029086afa95eca0bd5 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 20 Aug 2025 20:40:46 +0000 Subject: [PATCH 5/5] bump noir_json_parser time out --- 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 1874998a569..e22d5ddf417 100644 --- a/EXTERNAL_NOIR_LIBRARIES.yml +++ b/EXTERNAL_NOIR_LIBRARIES.yml @@ -50,7 +50,7 @@ libraries: critical: false noir_json_parser: repo: noir-lang/noir_json_parser - timeout: 12 + timeout: 15 critical: false sha256: repo: noir-lang/sha256