chore: Add Instruction::Noop (#6899)

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

@@ -448,6 +448,13 @@ impl DataFlowGraph {
         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 @@ pub(crate) enum Instruction {
     /// `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 @@ impl Instruction {
             | Instruction::IncrementRc { .. }
             | Instruction::DecrementRc { .. }
             | Instruction::RangeCheck { .. }
+            | Instruction::Noop
             | Instruction::EnableSideEffectsIf { .. } => InstructionResultType::None,
             Instruction::Allocate { .. }
             | Instruction::Load { .. }
@@ -399,7 +408,7 @@ impl Instruction {
             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 @@ impl Instruction {
             // 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 @@ impl Instruction {
             | ArrayGet { .. }
             | IfElse { .. }
             | ArraySet { .. }
+            | Noop
             | MakeArray { .. } => true,
 
             // Store instructions must be removed by DIE in acir code, any load
@@ -594,6 +608,7 @@ impl Instruction {
             | Instruction::IfElse { .. }
             | Instruction::IncrementRc { .. }
             | Instruction::DecrementRc { .. }
+            | Instruction::Noop
             | Instruction::MakeArray { .. } => false,
         }
     }
@@ -673,6 +688,7 @@ impl Instruction {
                 elements: elements.iter().copied().map(f).collect(),
                 typ: typ.clone(),
             },
+            Instruction::Noop => Instruction::Noop,
         }
     }
 
@@ -737,6 +753,7 @@ impl Instruction {
                     *element = f(*element);
                 }
             }
+            Instruction::Noop => (),
         }
     }
 
@@ -802,6 +819,7 @@ impl Instruction {
                     f(*element);
                 }
             }
+            Instruction::Noop => (),
         }
     }
 
@@ -1035,6 +1053,7 @@ impl Instruction {
                 }
             }
             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/inlining.rs

@@ -458,6 +458,7 @@ impl<'function> PerFunctionContext<'function> {
     /// and blocks respectively. If these assertions trigger it means a value is being used before
     /// the instruction or block that defines the value is inserted.
     fn translate_value(&mut self, id: ValueId) -> ValueId {
+        let id = self.source_function.dfg.resolve(id);
         if let Some(value) = self.values.get(&id) {
             return *value;
         }

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);
         }
     }
 }

cspell.json

@@ -202,6 +202,7 @@
         "Secpr",
         "signedness",
         "signorecello",
+        "smallvec",
         "smol",
         "splitn",
         "srem",