chore: disallow inserting ACIR-only instructions into brillig functions

noir-lang · Jan 10, 2025 · 33cad9b · 33cad9b
1 parent 2903052
commit 33cad9b
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 8 deletions.
diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
@@ -795,7 +795,7 @@ impl<'block> BrilligBlock<'block> {
                 self.brillig_context.deallocate_register(rc_register);
             }
             Instruction::EnableSideEffectsIf { .. } => {
-                todo!("enable_side_effects not supported by brillig")
+                unreachable!("enable_side_effects not supported by brillig")
             }
             Instruction::IfElse { .. } => {
                 unreachable!("IfElse instructions should not be possible in brillig")

diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs
@@ -20,6 +20,7 @@ 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
@@ -181,7 +182,16 @@ impl DataFlowGraph {
 
     /// Check if the function runtime would simply ignore this instruction.
     pub(crate) fn is_handled_by_runtime(&self, instruction: &Instruction) -> bool {
-        !(self.runtime().is_acir() && instruction.is_brillig_only())
+        match self.runtime() {
+            RuntimeType::Acir(_) => !matches!(
+                instruction,
+                Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. }
+            ),
+            RuntimeType::Brillig(_) => !matches!(
+                instruction,
+                Instruction::EnableSideEffectsIf { .. } | Instruction::IfElse { .. }
+            ),
+        }
     }
 
     fn insert_instruction_without_simplification(
@@ -228,6 +238,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;
         }
         match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) {

diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
@@ -1058,12 +1058,6 @@ impl Instruction {
             Instruction::Noop => Remove,
         }
     }
-
-    /// Some instructions are only to be used in Brillig and should be eliminated
-    /// after runtime separation, never to be be reintroduced in an ACIR runtime.
-    pub(crate) fn is_brillig_only(&self) -> bool {
-        matches!(self, Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. })
-    }
 }
 
 /// Given a chain of operations like: