Skip to content

Commit

Permalink
feat: avoid inserting inc_rc instructions into ACIR (#7036)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomAFrench authored Jan 13, 2025
1 parent 74d258f commit 8bd14bc
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 8 deletions.
4 changes: 1 addition & 3 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,14 +307,15 @@ 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)
{
match cache_result {
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);
Expand Down
10 changes: 6 additions & 4 deletions compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down

0 comments on commit 8bd14bc

Please sign in to comment.