chore: Add Instruction::Noop

compiler/noirc_evaluator/src/acir/mod.rs

@@ -788,6 +788,7 @@ impl<'a> Context<'a> {
                 let result = dfg.instruction_results(instruction_id)[0];
                 self.ssa_values.insert(result, value);
             }
+            Instruction::Noop => (),
         }
 
         self.acir_context.set_call_stack(CallStack::new());

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -837,6 +837,7 @@ impl<'block> BrilligBlock<'block> {
                     self.brillig_context.deallocate_register(items_pointer);
                 }
             }
+            Instruction::Noop => (),
         };
 
         let dead_variables = self

compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs

@@ -365,6 +365,7 @@ impl DependencyContext {
                 | Instruction::DecrementRc { .. }
                 | Instruction::EnableSideEffectsIf { .. }
                 | Instruction::IncrementRc { .. }
+                | Instruction::Noop
                 | Instruction::MakeArray { .. } => {}
             }
         }
@@ -626,6 +627,7 @@ impl Context {
                 | Instruction::DecrementRc { .. }
                 | Instruction::EnableSideEffectsIf { .. }
                 | Instruction::IncrementRc { .. }
+                | Instruction::Noop
                 | Instruction::RangeCheck { .. } => {}
             }
         }

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

@@ -42,7 +42,7 @@
     /// Call instructions require the func signature, but
     /// other instructions may need some more reading on my part
     #[serde_as(as = "HashMap<DisplayFromStr, _>")]
     results: HashMap<InstructionId, smallvec::SmallVec<[ValueId; 1]>>,
 
     /// Storage for all of the values defined in this
     /// function.
@@ -345,7 +345,7 @@
         instruction: InstructionId,
         ctrl_typevars: Option<Vec<Type>>,
     ) {
         let mut results = smallvec::SmallVec::new();
         let mut position = 0;
         self.for_each_instruction_result_type(instruction, ctrl_typevars, |this, typ| {
             let result = this.values.insert(Value::Instruction { typ, position, instruction });
@@ -413,30 +413,6 @@
         matches!(self.values[value].get_type().as_ref(), Type::Reference(_))
     }
 
-    /// Replaces an instruction result with a fresh id.
-    pub(crate) fn replace_result(
-        &mut self,
-        instruction_id: InstructionId,
-        prev_value_id: ValueId,
-    ) -> ValueId {
-        let typ = self.type_of_value(prev_value_id);
-        let results = self.results.get_mut(&instruction_id).unwrap();
-        let res_position = results
-            .iter()
-            .position(|&id| id == prev_value_id)
-            .expect("Result id not found while replacing");
-
-        let value_id = self.values.insert(Value::Instruction {
-            typ,
-            position: res_position,
-            instruction: instruction_id,
-        });
-
-        // Replace the value in list of results for this instruction
-        results[res_position] = value_id;
-        value_id
-    }
-
     /// Returns the number of instructions
     /// inserted into functions.
     pub(crate) fn num_instructions(&self) -> usize {
@@ -448,6 +424,13 @@
         self.results.get(&instruction_id).expect("expected a list of Values").as_slice()
     }
 
+    /// Remove an instruction by replacing it with a `Noop` instruction.
+    /// Doing this avoids shifting over each instruction after this one in its block's instructions vector.
+    pub(crate) fn remove_instruction(&mut self, instruction: InstructionId) {
+        self.instructions[instruction] = Instruction::Noop;
+        self.results.insert(instruction, smallvec::SmallVec::new());
+    }
+
     /// Add a parameter to the given block
     pub(crate) fn add_block_parameter(&mut self, block_id: BasicBlockId, typ: Type) -> ValueId {
         let block = &mut self.blocks[block_id];

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -335,6 +335,14 @@
     /// `typ` should be an array or slice type with an element type
     /// matching each of the `elements` values' types.
     MakeArray { elements: im::Vector<ValueId>, typ: Type },
+
+    /// A No-op instruction. These are intended to replace other instructions in a block's
+    /// instructions vector without having to move each instruction afterward.
+    ///
+    /// A No-op has no results and is always removed when Instruction::simplify is called.
+    /// When replacing another instruction, the instruction's results should always be mapped to a
+    /// new value since they will not be able to refer to their original instruction value any more.
+    Noop,
 }
 
 impl Instruction {
@@ -360,6 +368,7 @@
             | Instruction::IncrementRc { .. }
             | Instruction::DecrementRc { .. }
             | Instruction::RangeCheck { .. }
+            | Instruction::Noop
             | Instruction::EnableSideEffectsIf { .. } => InstructionResultType::None,
             Instruction::Allocate { .. }
             | Instruction::Load { .. }
@@ -398,8 +407,8 @@
             // These can fail.
             Constrain(..) | RangeCheck { .. } => true,
 
             // This should never be side-effectful
-            MakeArray { .. } => false,
+            MakeArray { .. } | Noop => false,
 
             // Some binary math can overflow or underflow
             Binary(binary) => match binary.operator {
@@ -460,6 +469,10 @@
             // We can deduplicate these instructions if we know the predicate is also the same.
             Constrain(..) | RangeCheck { .. } => deduplicate_with_predicate,
 
+            // Noop instructions can always be deduplicated, although they're more likely to be
+            // removed entirely.
+            Noop => true,
+
             // Arrays can be mutated in unconstrained code so code that handles this case must
             // take care to track whether the array was possibly mutated or not before
             // deduplicating. Since we don't know if the containing pass checks for this, we
@@ -504,6 +517,7 @@
             | ArrayGet { .. }
             | IfElse { .. }
             | ArraySet { .. }
+            | Noop
             | MakeArray { .. } => true,
 
             // Store instructions must be removed by DIE in acir code, any load
@@ -594,6 +608,7 @@
             | Instruction::IfElse { .. }
             | Instruction::IncrementRc { .. }
             | Instruction::DecrementRc { .. }
+            | Instruction::Noop
             | Instruction::MakeArray { .. } => false,
         }
     }
@@ -673,6 +688,7 @@
                 elements: elements.iter().copied().map(f).collect(),
                 typ: typ.clone(),
             },
+            Instruction::Noop => Instruction::Noop,
         }
     }
 
@@ -737,6 +753,7 @@
                     *element = f(*element);
                 }
             }
+            Instruction::Noop => (),
         }
     }
 
@@ -802,6 +819,7 @@
                     f(*element);
                 }
             }
+            Instruction::Noop => (),
         }
     }
 
@@ -1035,6 +1053,7 @@
                 }
             }
             Instruction::MakeArray { .. } => None,
+            Instruction::Noop => Remove,
         }
     }
 }

compiler/noirc_evaluator/src/ssa/ir/printer.rs

@@ -235,6 +235,7 @@ fn display_instruction_inner(
 
             writeln!(f, "] : {typ}")
         }
+        Instruction::Noop => writeln!(f, "no-op"),
     }
 }
 

compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs

@@ -67,12 +67,11 @@ fn replace_known_slice_lengths(
         let call_returns = func.dfg.instruction_results(instruction_id);
         let original_slice_length = call_returns[0];
 
-        // We won't use the new id for the original unknown length.
-        // This isn't strictly necessary as a new result will be defined the next time for which the instruction
-        // is reinserted but this avoids leaving the program in an invalid state.
-        func.dfg.replace_result(instruction_id, original_slice_length);
         let known_length = func.dfg.make_constant(known_length.into(), NumericType::length_type());
         func.dfg.set_value_from_id(original_slice_length, known_length);
+
+        // Now remove the original instruction
+        func.dfg.remove_instruction(instruction_id);
     });
 }
 

compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs

@@ -147,6 +147,7 @@ impl Context {
             | IfElse { .. }
             | IncrementRc { .. }
             | DecrementRc { .. }
+            | Noop
             | MakeArray { .. } => false,
 
             EnableSideEffectsIf { .. }

compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs

@@ -45,11 +45,11 @@ impl Function {
             let call_returns = self.dfg.instruction_results(instruction_id);
             let original_return_id = call_returns[0];
 
-            // We replace the result with a fresh id. This will be unused, so the DIE pass will remove the leftover intrinsic call.
-            self.dfg.replace_result(instruction_id, original_return_id);
-
             // Replace all uses of the original return value with the constant
             self.dfg.set_value_from_id(original_return_id, is_within_unconstrained);
+
+            // Now remove the original instruction
+            self.dfg.remove_instruction(instruction_id);
         }
     }
 }