From 8bd14bc90e38700213d120f4dd601d66a5efce5f Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 13 Jan 2025 14:27:20 +0000 Subject: [PATCH] feat: avoid inserting `inc_rc` instructions into ACIR (#7036) --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 4 +--- .../noirc_evaluator/src/ssa/opt/constant_folding.rs | 3 ++- compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs | 10 ++++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index ddc9814a40..6eae291ca4 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -20,7 +20,6 @@ use iter_extended::vecmap; use serde::{Deserialize, Serialize}; use serde_with::serde_as; use serde_with::DisplayFromStr; -use tracing::warn; /// The DataFlowGraph contains most of the actual data in a function including /// its blocks, instructions, and values. This struct is largely responsible for @@ -238,8 +237,7 @@ impl DataFlowGraph { call_stack: CallStackId, ) -> InsertInstructionResult { if !self.is_handled_by_runtime(&instruction) { - warn!("Attempted to insert instruction not handled by runtime: {instruction:?}"); - return InsertInstructionResult::InstructionRemoved; + panic!("Attempted to insert instruction not handled by runtime: {instruction:?}"); } match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) { diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 2c7d6b4f1e..5b75055c09 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -307,6 +307,7 @@ impl<'brillig> Context<'brillig> { let old_results = dfg.instruction_results(id).to_vec(); // If a copy of this instruction exists earlier in the block, then reuse the previous results. + let runtime_is_brillig = dfg.runtime().is_brillig(); if let Some(cache_result) = self.get_cached(dfg, dom, &instruction, *side_effects_enabled_var, block) { @@ -314,7 +315,7 @@ impl<'brillig> Context<'brillig> { CacheResult::Cached(cached) => { // We track whether we may mutate MakeArray instructions before we deduplicate // them but we still need to issue an extra inc_rc in case they're mutated afterward. - if matches!(instruction, Instruction::MakeArray { .. }) { + if runtime_is_brillig && matches!(instruction, Instruction::MakeArray { .. }) { let value = *cached.last().unwrap(); let inc_rc = Instruction::IncrementRc { value }; let call_stack = dfg.get_instruction_call_stack_id(id); diff --git a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index f0f677e5db..125cf3a12c 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -112,10 +112,12 @@ impl<'f> LoopInvariantContext<'f> { // If we are hoisting a MakeArray instruction, // we need to issue an extra inc_rc in case they are mutated afterward. - if matches!( - self.inserter.function.dfg[instruction_id], - Instruction::MakeArray { .. } - ) { + if self.inserter.function.runtime().is_brillig() + && matches!( + self.inserter.function.dfg[instruction_id], + Instruction::MakeArray { .. } + ) + { let result = self.inserter.function.dfg.instruction_results(instruction_id)[0]; let inc_rc = Instruction::IncrementRc { value: result };