fix(ssa): Simplify instructions during remove_unreachable_instructions

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

@@ -155,36 +155,40 @@ impl ControlFlowGraph {
     /// such that there is a path from every block to the exit node.
     /// Ex: below the forward CFG has one exit node: b2
     /// However, there is no path from b5 to b2
+    /// ```text
     /// forward          reverse
     ///  -------          -------
-    ///   b0*              b0               
-    ///   |                ^             
-    ///   v                |             
-    ///   b1               b1             
-    ///  /  \             ^  ^             
-    /// v    v           /    \             
-    /// b3   b4          b3   b4              
-    /// |    |           ^    ^             
-    /// v    v           |    |             
-    /// b2   b5 <-|      b2*  b5 <-|             
-    ///       \___|            \___|                      
+    ///   b0*              b0
+    ///   |                ^
+    ///   v                |
+    ///   b1               b1
+    ///  /  \             ^  ^
+    /// v    v           /    \
+    /// b3   b4          b3   b4
+    /// |    |           ^    ^
+    /// v    v           |    |
+    /// b2   b5 <-|      b2*  b5 <-|
+    ///       \___|            \___|
+    /// ```
     ///
     /// The extended CFG is the forward CFG with a new 'exit' node:
+    /// ```text
     ///  extended         extended reverse
     ///  -------          -------
-    ///   b0*              b0               
-    ///   |                ^             
-    ///   v                |             
-    ///   b1               b1             
-    ///  /  \             ^  ^             
-    /// v    v           /    \             
-    /// b3   b4          b3   b4              
-    /// |    |           ^    ^             
-    /// v    v           |    |             
-    /// b2   b5 <-|      b2   b5 <-|             
-    /// \    /\___|      ^    ^\___|             
+    ///   b0*              b0
+    ///   |                ^
+    ///   v                |
+    ///   b1               b1
+    ///  /  \             ^  ^
+    /// v    v           /    \
+    /// b3   b4          b3   b4
+    /// |    |           ^    ^
+    /// v    v           |    |
+    /// b2   b5 <-|      b2   b5 <-|
+    /// \    /\___|      ^    ^\___|
     ///  v  v            \    /
     ///  exit             exit*
+    /// ```
     pub(crate) fn extended_reverse(func: &mut Function) -> Self {
         let mut cfg = Self::with_function(func);
         // Exit blocks are the ones having no successor

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

@@ -328,11 +328,11 @@ mod test {
           b0(v0: u32, v1: u32):
             jmp b1(v0)
           b1(v2: u32):
-            v3 = lt v2, v1                                   
+            v3 = lt v2, v1
             jmpif v3 then: b2, else: b3
           b2():
-            call assert_constant(v0)                          
-            v6 = unchecked_add v2, u32 1                      
+            call assert_constant(v0)
+            v6 = unchecked_add v2, u32 1
             jmp b1(v6)
           b3():
             return

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

@@ -171,8 +171,7 @@ impl<'f> PerFunctionContext<'f> {
     /// dom_tree were created from.
     fn mem2reg(&mut self) {
         // Iterate each block in reverse post order = forward order
-        let mut block_order = self.post_order.as_slice().to_vec();
-        block_order.reverse();
+        let block_order = self.post_order.clone().into_vec_reverse();
 
         for block in block_order {
             let references = self.find_starting_references(block);

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

@@ -441,11 +441,9 @@ mod tests {
             v19 = unchecked_add v17, v18
             v20 = make_array [v13, v19] : [u32; 2]
             enable_side_effects u1 1
-            v22 = array_get v20, index u32 0 -> u32
-            v23 = array_get v20, index u32 1 -> u32
-            v24 = add v22, v23
-            v26 = eq v24, u32 3
-            constrain v24 == u32 3
+            v22 = add v13, v19
+            v24 = eq v22, u32 3
+            constrain v22 == u32 3
             return
         }
         ");
@@ -510,11 +508,9 @@ mod tests {
             v18 = unchecked_add v16, v17
             v19 = make_array [v11, v18] : [u32; 2]
             enable_side_effects u1 1
-            v21 = array_get v19, index u32 0 -> u32
-            v22 = array_get v19, index u32 1 -> u32
-            v23 = add v21, v22
-            v24 = eq v23, u32 1
-            constrain v23 == u32 1
+            v21 = add v11, v18
+            v22 = eq v21, u32 1
+            constrain v21 == u32 1
             return
         }
         ");
@@ -547,27 +543,43 @@ acir(inline) impure fn main f0 {
 
         // Merge slices v3 (empty) and v8 ([v2]) into v12, using a dummy value for the element at index 0 of v3, which does not exist.
         assert_ssa_snapshot!(ssa, @r"
-acir(inline) impure fn main f0 {
-  b0(v0: u1, v1: Field, v2: Field):
-    v3 = make_array [] : [Field]
-    v4 = allocate -> &mut u32
-    v5 = allocate -> &mut [Field]
-    enable_side_effects v0
-    v6 = cast v0 as u32
-    v8, v9 = call slice_push_back(v6, v3, v2) -> (u32, [Field])
-    v10 = not v0
-    v11 = cast v0 as u32
-    v13 = array_get v9, index u32 0 -> Field
-    v14 = cast v0 as Field
-    v15 = cast v10 as Field
-    v16 = mul v14, v13
-    v17 = make_array [v16] : [Field]
-    enable_side_effects u1 1
-    v19, v20 = call slice_push_back(v11, v17, v2) -> (u32, [Field])
-    v21 = array_get v20, index u32 0 -> Field
-    constrain v21 == Field 1
-    return
-}
-      ");
+        acir(inline) impure fn main f0 {
+          b0(v0: u1, v1: Field, v2: Field):
+            v3 = make_array [] : [Field]
+            v4 = allocate -> &mut u32
+            v5 = allocate -> &mut [Field]
+            enable_side_effects v0
+            v6 = cast v0 as u32
+            v8, v9 = call slice_push_back(v6, v3, v2) -> (u32, [Field])
+            v10 = not v0
+            v11 = cast v0 as u32
+            v13 = array_get v9, index u32 0 -> Field
+            v14 = cast v0 as Field
+            v15 = cast v10 as Field
+            v16 = mul v14, v13
+            v17 = make_array [v16] : [Field]
+            enable_side_effects u1 1
+            v20 = eq v11, u32 1
+            v21 = not v20
+            v22 = add v11, u32 1
+            v23 = make_array [v16, v2] : [Field]
+            v24 = array_set v23, index v11, value v2
+            v25 = array_get v24, index u32 0 -> Field
+            v26 = cast v21 as Field
+            v27 = cast v20 as Field
+            v28 = mul v26, v25
+            v29 = mul v27, v16
+            v30 = add v28, v29
+            v31 = array_get v24, index u32 1 -> Field
+            v32 = cast v21 as Field
+            v33 = cast v20 as Field
+            v34 = mul v32, v31
+            v35 = mul v33, v2
+            v36 = add v34, v35
+            v37 = make_array [v30, v36] : [Field]
+            constrain v30 == Field 1
+            return
+        }
+        ");
     }
 }

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

@@ -1219,4 +1219,38 @@ mod test {
         }
         "#);
     }
+
+    #[test]
+    fn simplifies_instructions_following_conditional_failure() {
+        // In the following SSA we have:
+        // 1. v1 is a divide-by-zero which turns into an conditional-fail constraint under v0
+        // 2. v2 is replaced by its default value (because division is considered side effecting)
+        // 3. v3 would be turned into a `truncate u64 0 to 32 bits` due to step 2, which is not expected to reach ACIR gen.
+        // We expect 3 to disappear as it can be simplified out.
+        let src = "
+        acir(inline) predicate_pure fn main f0 {
+          b0(v0: u1):
+            enable_side_effects v0
+            v1 = div u64 1, u64 0
+            v2 = div u64 1, u64 1
+            v3 = truncate v2 to 32 bits, max_bit_size: 254
+            enable_side_effects u1 1
+            return
+        }
+        ";
+
+        let ssa = Ssa::from_str(src).unwrap();
+        let ssa = ssa.remove_unreachable_instructions();
+
+        assert_ssa_snapshot!(ssa, @r#"
+        acir(inline) predicate_pure fn main f0 {
+          b0(v0: u1):
+            enable_side_effects v0
+            v3 = div u64 1, u64 0
+            constrain u1 0 == v0, "attempt to divide by zero"
+            enable_side_effects u1 1
+            return
+        }
+        "#);
+    }
 }

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

@@ -1,3 +1,4 @@
+use iter_extended::vecmap;
 use noirc_errors::call_stack::CallStackId;
 
 use acvm::FieldElement;
@@ -68,6 +69,7 @@ impl Function {
             for instruction_id in &instruction_ids {
                 let instruction_id = *instruction_id;
                 let instruction = &mut self.dfg[instruction_id];
+                let orig_instruction_hash = fxhash::hash64(instruction);
                 if !values_to_replace.is_empty() {
                     instruction.replace_values(&values_to_replace);
                 }
@@ -83,11 +85,12 @@ impl Function {
                     values_to_replace: &mut values_to_replace,
                     insert_current_instruction_at_callback_end: true,
                     enable_side_effects,
+                    orig_instruction_hash,
                 };
                 f(&mut context)?;
 
                 if context.insert_current_instruction_at_callback_end {
-                    self.dfg[block_id].insert_instruction(instruction_id);
+                    context.insert_current_instruction();
                 }
             }
 
@@ -108,10 +111,13 @@ pub(crate) struct SimpleOptimizationContext<'dfg, 'mapping> {
     pub(crate) enable_side_effects: ValueId,
     values_to_replace: &'mapping mut ValueMapping,
     insert_current_instruction_at_callback_end: bool,
+    orig_instruction_hash: u64,
 }
 
 impl SimpleOptimizationContext<'_, '_> {
     /// Returns the current instruction being visited.
+    ///
+    /// The instruction has already had its values updated with any replacements to be done.
     pub(crate) fn instruction(&self) -> &Instruction {
         &self.dfg[self.instruction_id]
     }
@@ -124,8 +130,37 @@ impl SimpleOptimizationContext<'_, '_> {
 
     /// Instructs this context to insert the current instruction right away, as opposed
     /// to doing this at the end of `mutate`'s block (unless `remove_current_instruction is called`).
+    ///
+    /// If the instruction or its values has relative to their original content,
+    /// we attempt to simplify the instruction before re-inserting it into the block.
     pub(crate) fn insert_current_instruction(&mut self) {
-        self.dfg[self.block_id].insert_instruction(self.instruction_id);
+        // If the instruction changed, then there is a chance that we can (or have to)
+        // simplify it before we insert it back into the block.
+        let instruction_hash = fxhash::hash64(self.instruction());
+        let simplify = self.orig_instruction_hash != instruction_hash;
+
+        if simplify {
+            // Based on FunctionInserter::push_instruction_value.
+            let instruction = self.instruction().clone();
+            let results = self.dfg.instruction_results(self.instruction_id).to_vec();
+            let ctrl_typevars = instruction
+                .requires_ctrl_typevars()
+                .then(|| vecmap(&results, |result| self.dfg.type_of_value(*result)));
+            let new_results = self.dfg.insert_instruction_and_results_if_simplified(
+                instruction,
+                self.block_id,
+                ctrl_typevars,
+                self.call_stack_id,
+                Some(self.instruction_id),
+            );
+            assert_eq!(results.len(), new_results.len());
+            for i in 0..results.len() {
+                self.values_to_replace.insert(results[i], new_results[i]);
+            }
+        } else {
+            self.dfg[self.block_id].insert_instruction(self.instruction_id);
+        }
+
         self.insert_current_instruction_at_callback_end = false;
     }