Insert the inc_rc during ssa_gen

Author: aakoshh

compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/call.rs

@@ -236,13 +236,6 @@ pub(super) fn simplify_call(
                     vector.pop_front().expect("There are no elements in this vector to be removed")
                 });
 
-                // The popped elements are reused by `ValueId` from the source vector, so any
-                // array-typed element still aliases the source's underlying memory. Bump the
-                // reference count so a subsequent mutation through the popped value does not
-                // mutate the source in place. The same invariant is enforced for plain indexing
-                // by the `ownership` pass (see `handle_index`).
-                inc_rc_array_results(&results, dfg, block, call_stack);
-
                 let new_vector_length =
                     decrement_vector_length(arguments[0], dfg, block, call_stack);
 
@@ -325,12 +318,6 @@ pub(super) fn simplify_call(
                     results.push(vector.remove(index));
                 }
 
-                // The removed elements are reused by `ValueId` from the source vector, so any
-                // array-typed element still aliases the source's memory. Bump the RC to preserve
-                // copy-on-write semantics. See `inc_rc_array_results` for the matching invariant
-                // in `pop_front` / `pop_back`.
-                inc_rc_array_results(&results, dfg, block, call_stack);
-
                 let new_vector = make_array(dfg, vector, typ, block, call_stack);
                 results.insert(0, new_vector);
 
@@ -682,27 +669,6 @@ fn decrement_vector_length(
     update_vector_length(vector_len, dfg, BinaryOp::Sub { unchecked: true }, block, call_stack)
 }
 
-/// Emit `inc_rc` for each value whose type is an array or vector. Used by vector intrinsic
-/// simplifications that hand back `ValueId`s of elements from the source vector: those elements
-/// still alias the source's memory, so the reference count must be bumped to preserve
-/// copy-on-write semantics. No-op under the ACIR runtime, which does not track reference counts.
-fn inc_rc_array_results(
-    values: &[ValueId],
-    dfg: &mut DataFlowGraph,
-    block: BasicBlockId,
-    call_stack: CallStackId,
-) {
-    if dfg.runtime().is_acir() {
-        return;
-    }
-    for value in values {
-        if dfg.type_of_value(*value).is_array() {
-            let instruction = Instruction::IncrementRc { value: *value };
-            dfg.insert_instruction_and_results(instruction, block, None, call_stack);
-        }
-    }
-}
-
 /// Simplify a vector push back when the length is not known to equal capacity, ie. we don't
 /// know whether we to push new items and grow the capacity of the vector, or overwrite the
 /// next padding item.
@@ -789,12 +755,6 @@ fn simplify_vector_pop_back(
         vector.pop_back();
     }
 
-    // The popped elements still alias the source vector's memory (`array_get` reads a value
-    // without bumping its reference count). Bump the RC for any array-typed element so that a
-    // subsequent mutation through the popped value triggers copy-on-write instead of mutating
-    // the source. See `inc_rc_array_results` for the matching invariant in `pop_front`.
-    inc_rc_array_results(results.make_contiguous(), dfg, block, call_stack);
-
     let new_vector = make_array(dfg, vector, vector_type, block, call_stack);
     results.push_front(new_vector);
 

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

@@ -1334,7 +1334,54 @@ impl FunctionContext<'_> {
             }
         }
 
-        Ok(self.insert_call(function, arguments, &call.return_type, call.location))
+        let result = self.insert_call(function, arguments, &call.return_type, call.location);
+        self.codegen_intrinsic_inc_rc_results(function, &call.return_type, &result);
+        Ok(result)
+    }
+
+    /// For vector intrinsics that hand back an element extracted from the source vector
+    /// (`pop_front`, `pop_back`, `remove`), bump the reference count of any array- or
+    /// vector-typed element value. The intrinsic's returned tuple component for the new
+    /// (post-pop) vector is skipped: it is a distinct result, not an alias of the source.
+    ///
+    /// Without this bump the popped element shares its underlying memory with the source
+    /// vector, so a mutation through it would also mutate the source — breaking the
+    /// copy-on-write invariant the rest of the pipeline relies on. The `ownership` pass
+    /// enforces the same invariant for plain `array[i]` accesses (see `handle_index`),
+    /// but it does not see through these intrinsic calls.
+    fn codegen_intrinsic_inc_rc_results(
+        &mut self,
+        function: ValueId,
+        return_type: &ast::Type,
+        result: &Values,
+    ) {
+        if self.builder.current_function.runtime().is_acir() {
+            return;
+        }
+        let Some(intrinsic) = self.builder.get_intrinsic_from_value(function) else {
+            return;
+        };
+        if !matches!(
+            intrinsic,
+            Intrinsic::VectorPopFront | Intrinsic::VectorPopBack | Intrinsic::VectorRemove
+        ) {
+            return;
+        }
+        // All of these vector operations return either ([T], T) or (T, [T])
+        let (ast::Type::Tuple(types), Tree::Branch(value_trees)) = (return_type, result) else {
+            return;
+        };
+        // We are looking for the T in the tuple, ignoring the [T].
+        for (ty, subtree) in types.iter().zip_eq(value_trees.iter()) {
+            if matches!(ty, ast::Type::Vector(_)) {
+                continue;
+            }
+            for value in subtree.clone().into_value_list(self) {
+                if self.builder.type_of_value(value).is_array() {
+                    self.builder.insert_inc_rc(value);
+                }
+            }
+        }
     }
 
     /// Generate SSA for a `multi_scalar_mul` blackbox call.