chore(brillig): Disable the decrement of the ref-count during array/vector copy

compiler/noirc_evaluator/src/brillig/brillig_gen/tests/memory.rs

@@ -136,11 +136,9 @@ fn brillig_array_with_rc_ops() {
     24: sp[5] = u32 add sp[3], sp[1]
     25: store sp[2] at sp[5]
     26: sp[2] = load sp[4]
-    27: sp[2] = u32 sub sp[2], @2
-    28: store sp[2] at sp[4]
-    29: sp[4] = u32 add sp[3], sp[1]
-    30: sp[2] = load sp[4]
-    31: sp[1] = sp[2]
-    32: return
+    27: sp[4] = u32 add sp[3], sp[1]
+    28: sp[2] = load sp[4]
+    29: sp[1] = sp[2]
+    30: return
     ");
 }

compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs

@@ -499,11 +499,17 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
     /// The inputs are:
     /// * the `pointer` to the array/vector
     /// * the `rc` address of the vector where we have the current RC loaded already
-    pub(crate) fn codegen_decrement_rc(&mut self, pointer: MemoryAddress, rc: MemoryAddress) {
-        // Modify the RC (it's on the stack, or scratch space).
-        self.codegen_usize_op_in_place(rc, BrilligBinaryOp::Sub, 1);
-        // Write it back onto the heap.
-        self.store_instruction(pointer, rc);
+    pub(crate) fn codegen_decrement_rc(&mut self, _pointer: MemoryAddress, _rc: MemoryAddress) {
+        // In benchmarks having this on didn't have a noticeable performance benefit,
+        // but it does have a small increase in byte code size and the number of executed opcodes.
+        // When we disabled `dec_rc` in SSA, the performance improved, so for now we disabled this,
+        // in order to not deviate conceptually from SSA. The method is left in place as a reference
+        // to where we could re-enable them.
+
+        // // Modify the RC (it's on the stack, or scratch space).
+        // self.codegen_usize_op_in_place(rc, BrilligBinaryOp::Sub, 1);
+        // // Write it back onto the heap.
+        // self.store_instruction(pointer, rc);
     }
 
     /// Increment the ref-count by 1.

compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

@@ -1095,7 +1095,7 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> {
                 Value::ArrayOrSlice(array.clone())
             } else {
                 if !is_rc_one {
-                    *array.rc.borrow_mut() -= 1;
+                    Self::decrement_rc(&array);
                 }
                 let mut elements = array.elements.borrow().to_vec();
                 elements[index as usize] = value;
@@ -1113,6 +1113,13 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> {
         Ok(())
     }
 
+    /// Decrement the ref-count of an array by 1.
+    fn decrement_rc(_array: &ArrayValue) {
+        // The decrement of the ref-count is currently disabled in SSA as well as the Brillig codegen,
+        // but we might re-enable it in the future if the ownership optimizations change.
+        // *array.rc.borrow_mut() -= 1;
+    }
+
     fn interpret_inc_rc(&self, value_id: ValueId) -> IResult<()> {
         if self.in_unconstrained_context() {
             let array = self.lookup_array_or_slice(value_id, "inc_rc")?;

compiler/noirc_evaluator/src/ssa/interpreter/tests/instructions.rs

@@ -960,7 +960,7 @@ fn array_set_with_offset() {
     let v0 = values[0].as_array_or_slice().unwrap();
     let v1 = values[1].as_array_or_slice().unwrap();
 
-    assert_eq!(*v0.rc.borrow(), 1, "1+1-1; the copy of v1 decreases the RC of v0");
+    assert_eq!(*v0.rc.borrow(), 2, "1+1-0; the copy of v1 does not decrease the RC of v0");
     assert_eq!(*v1.rc.borrow(), 1);
 
     let one = from_constant(1u32.into(), NumericType::NativeField);

compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs

@@ -2373,6 +2373,8 @@ mod test {
     fn do_not_deduplicate_call_with_inc_rc() {
         // This test ensures that a function which mutates an array pointer is marked impure.
         // This protects against future deduplication passes incorrectly assuming purity.
+        // The increasing RC numbers reflect the current expectation that the RC of the
+        // original array does not get decremented when a copy is made.
         let src = r#"
         brillig(inline) fn main f0 {
           b0(v0: u32):
@@ -2381,10 +2383,10 @@ mod test {
             constrain v5 == u32 1
             v8 = call f1(v3) -> [Field; 2]
             v9 = call array_refcount(v3) -> u32
-            constrain v9 == u32 1
+            constrain v9 == u32 2
             v11 = call f1(v3) -> [Field; 2]
             v12 = call array_refcount(v3) -> u32
-            constrain v12 == u32 1
+            constrain v12 == u32 3
             inc_rc v3
             v15 = array_set v3, index v0, value Field 9
             return v3, v15

test_programs/execution_success/reference_counts_inliner_0/src/main.nr

@@ -119,10 +119,13 @@ fn regression_7297() {
             "There is 1 clone after `borrow_mut_two` and before `refcount_1` is defined (cloned before array_refcount call)",
         );
 
+        // The current expectation is that the original array does not get its RC decremented when a copy is made.
+        let decrement_count = 0;
+
         assert_eq(
             refcount_2,
-            refcount_1 + 3 - 1,
-            "after refcount_1 we clone once in passing array to copy_mut, once to array_refcount after, and once within copy_mut, but we decrease in copy_mut when copying",
+            refcount_1 + 3 - decrement_count,
+            "after refcount_1 we clone once in passing array to copy_mut, once to array_refcount after, and once within copy_mut",
         );
     }
 }

test_programs/execution_success/reference_counts_inliner_min/src/main.nr

@@ -119,10 +119,13 @@ fn regression_7297() {
             "There is 1 clone after `borrow_mut_two` and before `refcount_1` is defined (cloned before array_refcount call)",
         );
 
+        // The current expectation is that the original array does not get its RC decremented when a copy is made.
+        let decrement_count = 0;
+
         assert_eq(
             refcount_2,
-            refcount_1 + 3 - 1,
-            "after refcount_1 we clone once in passing array to copy_mut, once to array_refcount after, and once within copy_mut, but we decrease in copy_mut when copying",
+            refcount_1 + 3 - decrement_count,
+            "after refcount_1 we clone once in passing array to copy_mut, once to array_refcount after, and once within copy_mut",
         );
     }
 }

test_programs/execution_success/reference_counts_slices_inliner_0/src/main.nr

@@ -131,6 +131,7 @@ unconstrained fn regression_10335(i: u32) {
     let rc_a = slice_refcount(a);
     let rc_b = slice_refcount(b);
 
-    assert_eq(rc_a, 1);
+    // The current expectation is that the original array does not get its RC decremented.
+    assert_eq(rc_a, 2);
     assert_eq(rc_b, 1);
 }