Special case inc_rc instead

noir-lang · Nov 28, 2023 · ead0806 · ead0806
1 parent fcc90db
commit ead0806
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 8 deletions.
diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
@@ -275,15 +275,10 @@ impl Instruction {
             | ArrayGet { .. }
             | ArraySet { .. } => false,
 
-            // IncrementRc is not counted as having side effects since we still
-            // want it to be removed by the DIE pass if its parameter is unused.
-            // At the time of writing has_side_effects is only used by DIE but
-            // we should keep this in mind (or rename this method?) if it is ever used elsewhere.
-            IncrementRc { .. } => false,
-
             Constrain(..)
             | Store { .. }
             | EnableSideEffects { .. }
+            | IncrementRc { .. }
             | RangeCheck { .. } => true,
 
             // Some `Intrinsic`s have side effects so we must check what kind of `Call` this is.

diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs
@@ -7,7 +7,7 @@ use crate::ssa::{
         basic_block::{BasicBlock, BasicBlockId},
         dfg::DataFlowGraph,
         function::Function,
-        instruction::InstructionId,
+        instruction::{InstructionId, Instruction},
         post_order::PostOrder,
         value::{Value, ValueId},
     },
@@ -90,7 +90,12 @@ impl Context {
     fn is_unused(&self, instruction_id: InstructionId, function: &Function) -> bool {
         let instruction = &function.dfg[instruction_id];
 
-        if instruction.has_side_effects(&function.dfg) {
+        // IncrementRc is a special case, we want to remove it if possible
+        // but it has no results so it'd always be removed if left to the normal check.
+        // We must check whether its parameter is unused instead.
+        if let Instruction::IncrementRc { value } = instruction {
+            !self.used_values.contains(value)
+        } else if instruction.has_side_effects(&function.dfg) {
             // If the instruction has side effects we should never remove it.
             false
         } else {