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 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 { diff --git a/compiler/noirc_evaluator/src/ssa/opt/rc.rs b/compiler/noirc_evaluator/src/ssa/opt/rc.rs index e53f3c0b553..77d0a04e1dd 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,181 @@ 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 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]): + 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(); + // 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); + } + + #[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(); + // 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]): + jmp b1() + b1(): + jmp b2() + b2(): + v1 = make_array [v0] : [[Field; 2]; 1] + return v1 + } + "); + } }