feat(ssa): Perform dead instruction elimination on intrinsic functions (

#2276)

crates/nargo_cli/tests/compile_success/intrinsic_die/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "intrinsic_die"
+type = "bin"
+authors = [""]
+compiler_version = "0.6.0"
+
+[dependencies]

crates/nargo_cli/tests/compile_success/intrinsic_die/src/main.nr

@@ -0,0 +1,9 @@
+use dep::std;
+
+// This test checks that we perform dead-instruction-elimination on intrinsic functions.
+
+fn main(x: Field) {
+    let bytes = x.to_be_bytes(32);
+
+    let hash = std::hash::pedersen([x]);
+}

crates/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -71,6 +71,30 @@ impl std::fmt::Display for Intrinsic {
 }
 
 impl Intrinsic {
+    /// Returns whether the `Intrinsic` has side effects.
+    ///
+    /// If there are no side effects then the `Intrinsic` can be removed if the result is unused.
+    pub(crate) fn has_side_effects(&self) -> bool {
+        match self {
+            Intrinsic::Println | Intrinsic::AssertConstant => true,
+
+            Intrinsic::Sort
+            | Intrinsic::ArrayLen
+            | Intrinsic::SlicePushBack
+            | Intrinsic::SlicePushFront
+            | Intrinsic::SlicePopBack
+            | Intrinsic::SlicePopFront
+            | Intrinsic::SliceInsert
+            | Intrinsic::SliceRemove
+            | Intrinsic::StrAsBytes
+            | Intrinsic::ToBits(_)
+            | Intrinsic::ToRadix(_) => false,
+
+            // Some black box functions have side-effects
+            Intrinsic::BlackBox(func) => matches!(func, BlackBoxFunc::RecursiveAggregation),
+        }
+    }
+
     /// Lookup an Intrinsic by name and return it if found.
     /// If there is no such intrinsic by that name, None is returned.
     pub(crate) fn lookup(name: &str) -> Option<Intrinsic> {
@@ -183,6 +207,29 @@ impl Instruction {
         matches!(self.result_type(), InstructionResultType::Unknown)
     }
 
+    pub(crate) fn has_side_effects(&self, dfg: &DataFlowGraph) -> bool {
+        use Instruction::*;
+
+        match self {
+            Binary(_)
+            | Cast(_, _)
+            | Not(_)
+            | Truncate { .. }
+            | Allocate
+            | Load { .. }
+            | ArrayGet { .. }
+            | ArraySet { .. } => false,
+
+            Constrain(_) | Store { .. } | EnableSideEffects { .. } => true,
+
+            // Some `Intrinsic`s have side effects so we must check what kind of `Call` this is.
+            Call { func, .. } => match dfg[*func] {
+                Value::Intrinsic(intrinsic) => intrinsic.has_side_effects(),
+                _ => false,
+            },
+        }
+    }
+
     /// Maps each ValueId inside this instruction to a new ValueId, returning the new instruction.
     /// Note that the returned instruction is fresh and will not have an assigned InstructionId
     /// until it is manually inserted in a DataFlowGraph later.

crates/noirc_evaluator/src/ssa/opt/die.rs

@@ -7,7 +7,7 @@ use crate::ssa::{
         basic_block::{BasicBlock, BasicBlockId},
         dfg::DataFlowGraph,
         function::Function,
-        instruction::{Instruction, InstructionId},
+        instruction::InstructionId,
         post_order::PostOrder,
         value::{Value, ValueId},
     },
@@ -88,20 +88,15 @@ impl Context {
     /// An instruction can be removed as long as it has no side-effects, and none of its result
     /// values have been referenced.
     fn is_unused(&self, instruction_id: InstructionId, function: &Function) -> bool {
-        use Instruction::*;
-
         let instruction = &function.dfg[instruction_id];
 
-        // These instruction types cannot be removed
-        if matches!(
-            instruction,
-            Constrain(_) | Call { .. } | Store { .. } | EnableSideEffects { .. }
-        ) {
-            return false;
+        if instruction.has_side_effects(&function.dfg) {
+            // If the instruction has side effects we should never remove it.
+            false
+        } else {
+            let results = function.dfg.instruction_results(instruction_id);
+            results.iter().all(|result| !self.used_values.contains(result))
         }
-
-        let results = function.dfg.instruction_results(instruction_id);
-        results.iter().all(|result| !self.used_values.contains(result))
     }
 
     /// Adds values referenced by the terminator to the set of used values.
@@ -134,7 +129,12 @@ impl Context {
 #[cfg(test)]
 mod test {
     use crate::ssa::{
-        ir::{function::RuntimeType, instruction::BinaryOp, map::Id, types::Type},
+        ir::{
+            function::RuntimeType,
+            instruction::{BinaryOp, Intrinsic},
+            map::Id,
+            types::Type,
+        },
         ssa_builder::FunctionBuilder,
     };
 
@@ -159,7 +159,6 @@ mod test {
         //     return v9
         // }
         let main_id = Id::test_new(0);
-        let println_id = Id::test_new(1);
 
         // Compiling main
         let mut builder = FunctionBuilder::new("main".into(), main_id, RuntimeType::Acir);
@@ -187,6 +186,8 @@ mod test {
         let v9 = builder.insert_binary(v7, BinaryOp::Add, two);
         let v10 = builder.insert_binary(v7, BinaryOp::Add, three);
         let _v11 = builder.insert_binary(v10, BinaryOp::Add, v10);
+
+        let println_id = builder.import_intrinsic_id(Intrinsic::Println);
         builder.insert_call(println_id, vec![v8], vec![]);
         builder.terminate_with_return(vec![v9]);