diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 95e0dd12132..a82c54d8ce6 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/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()); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 5bcddc21275..66cc213a986 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/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 diff --git a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 2f167ec8ab3..106b01833cd 100644 --- a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/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 { .. } => {} } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 9ccf7f11512..902ff0775a9 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/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]; diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 5d9bfc89f61..23d8b425349 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/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, 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, } } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 29e79728303..322639a03d2 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/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"), } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 11201fc8f85..36955480728 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/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; } diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index e99f239e82e..a22232ba49a 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -147,6 +147,7 @@ impl Context { | IfElse { .. } | IncrementRc { .. } | DecrementRc { .. } + | Noop | MakeArray { .. } => false, EnableSideEffectsIf { .. } diff --git a/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs b/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs index eff6576b87f..3ac7535a1c4 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs +++ b/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); } } } diff --git a/cspell.json b/cspell.json index 9a4bca1e339..826e30fa86a 100644 --- a/cspell.json +++ b/cspell.json @@ -202,6 +202,7 @@ "Secpr", "signedness", "signorecello", + "smallvec", "smol", "splitn", "srem",