fix: Add a remove_unreachable_instructions SSA pass (with unreachable terminator)

compiler/noirc_evaluator/src/acir/mod.rs

@@ -915,8 +915,11 @@ impl<'a> Context<'a> {
     ) -> usize {
         let return_values = match terminator {
             TerminatorInstruction::Return { return_values, .. } => return_values,
+            TerminatorInstruction::Unreachable { .. } => return 0,
             // TODO(https://github.com/noir-lang/noir/issues/4616): Enable recursion on foldable/non-inlined ACIR functions
-            _ => unreachable!("ICE: Program must have a singular return"),
+            TerminatorInstruction::JmpIf { .. } | TerminatorInstruction::Jmp { .. } => {
+                unreachable!("ICE: Program must have a singular return")
+            }
         };
 
         return_values
@@ -935,7 +938,10 @@ impl<'a> Context<'a> {
                 (return_values, *call_stack)
             }
             // TODO(https://github.com/noir-lang/noir/issues/4616): Enable recursion on foldable/non-inlined ACIR functions
-            _ => unreachable!("ICE: Program must have a singular return"),
+            TerminatorInstruction::JmpIf { .. } | TerminatorInstruction::Jmp { .. } => {
+                unreachable!("ICE: Program must have a singular return")
+            }
+            TerminatorInstruction::Unreachable { .. } => return Ok((vec![], vec![])),
         };
 
         let mut has_constant_return = false;

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -251,6 +251,9 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                 });
                 self.brillig_context.codegen_return(&return_registers);
             }
+            TerminatorInstruction::Unreachable { .. } => {
+                // If we assume this is unreachable code then there's nothing to do here
+            }
         }
     }
 

compiler/noirc_evaluator/src/ssa.rs

@@ -213,7 +213,9 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass> {
         SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get and Set Optimizations"),
         // Perform another DIE pass to update the used globals after offsetting Brillig indexes.
         SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination"),
-        // A function can be potentially unreachable post-DIE if all calls to that function were removed.
+        SsaPass::new(Ssa::remove_unreachable_instructions, "Remove Unreachable Instructions"),
+        // A function can be potentially unreachable post-DIE if all calls to that function were removed,
+        // or after the removal of unreachable instructions.
         SsaPass::new(Ssa::remove_unreachable_functions, "Removing Unreachable Functions"),
         SsaPass::new(Ssa::checked_to_unchecked, "Checked to unchecked"),
         SsaPass::new_try(
@@ -229,6 +231,7 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass> {
 pub fn secondary_passes(brillig: &Brillig) -> Vec<SsaPass> {
     vec![
         SsaPass::new(move |ssa| ssa.fold_constants_with_brillig(brillig), "Inlining Brillig Calls"),
+        SsaPass::new(Ssa::remove_unreachable_instructions, "Remove Unreachable Instructions"),
         // It could happen that we inlined all calls to a given brillig function.
         // In that case it's unused so we can remove it. This is what we check next.
         SsaPass::new(Ssa::remove_unreachable_functions, "Removing Unreachable Functions"),

compiler/noirc_evaluator/src/ssa/function_builder/mod.rs

@@ -442,6 +442,12 @@ impl FunctionBuilder {
         self.terminate_block_with(TerminatorInstruction::Return { return_values, call_stack });
     }
 
+    /// Terminate the current block with an unreachable instruction
+    pub fn terminate_with_unreachable(&mut self) {
+        let call_stack = self.call_stack;
+        self.terminate_block_with(TerminatorInstruction::Unreachable { call_stack });
+    }
+
     /// Returns a ValueId pointing to the given function or imports the function
     /// into the current function if it was not already, and returns that ID.
     pub fn import_function(&mut self, function: FunctionId) -> ValueId {

compiler/noirc_evaluator/src/ssa/interpreter/errors.rs

@@ -58,6 +58,8 @@ pub enum InterpreterError {
     ToRadixFailed { field_id: ValueId, field: FieldElement, radix: u32 },
     #[error("Failed to solve blackbox function {name}: {reason}")]
     BlackBoxError { name: String, reason: String },
+    #[error("Reached the unreachable")]
+    ReachedTheUnreachable,
 }
 
 /// These errors can only result from interpreting malformed SSA

compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

@@ -262,6 +262,9 @@
 
                     break return_values;
                 }
+                Some(TerminatorInstruction::Unreachable { .. }) => {
+                    return Err(InterpreterError::ReachedTheUnreachable);
+                }
             }
         };
 
@@ -1174,7 +1177,7 @@
             }));
         }
 
         // Disable this instruction if it is side-effectful and side effects are disabled.
         if !side_effects_enabled {
             let zero = NumericValue::zero(lhs.get_type());
             return Ok(Value::Numeric(zero));

compiler/noirc_evaluator/src/ssa/ir/basic_block.rs

@@ -149,8 +149,9 @@ impl BasicBlock {
             Some(TerminatorInstruction::JmpIf { then_destination, else_destination, .. }) => {
                 vec![*then_destination, *else_destination].into_iter()
             }
-            Some(TerminatorInstruction::Return { .. }) => vec![].into_iter(),
-            None => vec![].into_iter(),
+            Some(TerminatorInstruction::Return { .. })
+            | Some(TerminatorInstruction::Unreachable { .. })
+            | None => vec![].into_iter(),
         }
     }
 }

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

@@ -853,6 +853,10 @@ pub(crate) enum TerminatorInstruction {
     /// as the block arguments. Then the exit block can terminate in a return
     /// instruction returning these values.
     Return { return_values: Vec<ValueId>, call_stack: CallStackId },
+
+    /// A terminator that will never be reached because an instruction in its block
+    /// will always produce an assertion failure.
+    Unreachable { call_stack: CallStackId },
 }
 
 impl TerminatorInstruction {
@@ -873,6 +877,7 @@ impl TerminatorInstruction {
                     *return_value = f(*return_value);
                 }
             }
+            Unreachable { .. } => (),
         }
     }
 
@@ -893,6 +898,7 @@ impl TerminatorInstruction {
                     f(*return_value);
                 }
             }
+            Unreachable { .. } => (),
         }
     }
 
@@ -913,6 +919,7 @@ impl TerminatorInstruction {
                     f(index, *return_value);
                 }
             }
+            Unreachable { .. } => (),
         }
     }
 
@@ -927,23 +934,25 @@ impl TerminatorInstruction {
             Jmp { destination, .. } => {
                 *destination = f(*destination);
             }
-            Return { .. } => (),
+            Return { .. } | Unreachable { .. } => (),
         }
     }
 
     pub(crate) fn call_stack(&self) -> CallStackId {
         match self {
             TerminatorInstruction::JmpIf { call_stack, .. }
             | TerminatorInstruction::Jmp { call_stack, .. }
-            | TerminatorInstruction::Return { call_stack, .. } => *call_stack,
+            | TerminatorInstruction::Return { call_stack, .. }
+            | TerminatorInstruction::Unreachable { call_stack } => *call_stack,
         }
     }
 
     pub(crate) fn set_call_stack(&mut self, new_call_stack: CallStackId) {
         match self {
             TerminatorInstruction::JmpIf { call_stack, .. }
             | TerminatorInstruction::Jmp { call_stack, .. }
-            | TerminatorInstruction::Return { call_stack, .. } => *call_stack = new_call_stack,
+            | TerminatorInstruction::Return { call_stack, .. }
+            | TerminatorInstruction::Unreachable { call_stack } => *call_stack = new_call_stack,
         }
     }
 }

compiler/noirc_evaluator/src/ssa/ir/printer.rs

@@ -157,6 +157,9 @@ fn display_terminator(
                 writeln!(f, "    return {}", value_list(dfg, return_values))
             }
         }
+        Some(TerminatorInstruction::Unreachable { .. }) => {
+            writeln!(f, "    unreachable")
+        }
         None => writeln!(f, "    (no terminator instruction)"),
     }
 }

compiler/noirc_evaluator/src/ssa/opt/basic_conditional.rs

@@ -361,6 +361,9 @@ impl Context<'_> {
                 let return_values = vecmap(return_values, |value| self.inserter.resolve(value));
                 TerminatorInstruction::Return { return_values, call_stack }
             }
+            TerminatorInstruction::Unreachable { call_stack } => {
+                TerminatorInstruction::Unreachable { call_stack }
+            }
         };
         self.inserter.function.dfg.set_block_terminator(conditional.block_entry, new_terminator);
         self.inserter.map_data_bus_in_place();

compiler/noirc_evaluator/src/ssa/opt/die/prune_dead_parameters.rs

@@ -128,6 +128,9 @@ impl Function {
                     TerminatorInstruction::Return { .. } => {
                         unreachable!("ICE: A return block should not be a predecessor");
                     }
+                    TerminatorInstruction::Unreachable { .. } => {
+                        unreachable!("ICE: An unreachable block should not be a predecessor");
+                    }
                 }
             }
         }

compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs

@@ -468,6 +468,10 @@ impl<'f> Context<'f> {
                 self.inserter.function.dfg.set_block_terminator(target, new_return);
                 vec![]
             }
+            TerminatorInstruction::Unreachable { .. } => {
+                // Nothing to do
+                vec![]
+            }
         }
     }
 

compiler/noirc_evaluator/src/ssa/opt/inlining.rs

@@ -727,6 +727,12 @@ impl<'function> PerFunctionContext<'function> {
 
                 Some((block_id, return_values))
             }
+            TerminatorInstruction::Unreachable { .. } => {
+                // Note: `unreachable` terminators are only added during the `remove_unreachable_instructions`,
+                // which runs near the end of the optimization pipeline, so never before inlining.
+                // If we ever want to run it before inlining we'll have to handle this case.
+                panic!("Unreachable terminator instruction should not exist during inlining.")
+            }
         }
     }
 }

compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs

@@ -68,7 +68,7 @@
 //!
 //! Repeating this algorithm for each block in the function in program order should result in
 //! optimizing out most known loads. However, identifying all aliases correctly has been proven
 //! undecidable in general (Landi, 1992). So this pass will not always optimize out all loads
 //! that could theoretically be optimized out. This pass can be performed at any time in the
 //! SSA optimization pipeline, although it will be more successful the simpler the program's CFG is.
 //! This pass is currently performed several times to enable other passes - most notably being
@@ -619,7 +619,7 @@
         self.inserter.map_terminator_in_place(block);
 
         match self.inserter.function.dfg[block].unwrap_terminator() {
-            TerminatorInstruction::JmpIf { .. } => (), // Nothing to do
+            TerminatorInstruction::JmpIf { .. } | TerminatorInstruction::Unreachable { .. } => (), // Nothing to do
             TerminatorInstruction::Jmp { destination, arguments, .. } => {
                 let destination_parameters = self.inserter.function.dfg[*destination].parameters();
                 assert_eq!(destination_parameters.len(), arguments.len());

compiler/noirc_evaluator/src/ssa/opt/mod.rs

@@ -30,7 +30,8 @@ mod remove_bit_shifts;
 mod remove_enable_side_effects;
 mod remove_if_else;
 mod remove_truncate_after_range_check;
-mod remove_unreachable;
+mod remove_unreachable_functions;
+mod remove_unreachable_instructions;
 mod simple_optimization;
 mod simplify_cfg;
 mod unrolling;