From 504aad0a43fb31fd51596b7caa907611022d59da Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Wed, 9 Aug 2023 21:11:31 +0000
Subject: [PATCH 01/28] cleanup load instruction search w/ predecessors methods

---
 .../execution_success/references/src/main.nr  |  18 ++
 crates/noirc_evaluator/src/ssa.rs             |   4 +
 crates/noirc_evaluator/src/ssa/ir/function.rs |   6 +-
 .../src/ssa/ir/instruction/call.rs            |   1 +
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 269 ++++++++++++++----
 .../noirc_evaluator/src/ssa/opt/unrolling.rs  |  10 +-
 .../src/monomorphization/mod.rs               |   9 +-
 7 files changed, 239 insertions(+), 78 deletions(-)

diff --git a/crates/nargo_cli/tests/execution_success/references/src/main.nr b/crates/nargo_cli/tests/execution_success/references/src/main.nr
index aeed8e0b90..96b94c6873 100644
--- a/crates/nargo_cli/tests/execution_success/references/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/references/src/main.nr
@@ -1,4 +1,6 @@
 fn main(mut x: Field) {
+    regression_2218(x, 10);
+
     add1(&mut x);
     assert(x == 3);
 
@@ -97,3 +99,19 @@ fn regression_2030() {
     let _ = *array[0];
     *array[0] = 1;
 }
+
+fn regression_2218(x: Field, y: Field) {
+    let q = &mut &mut 0;
+    let q1 = *q;
+    let q2 = *q;
+
+    if x != y {
+        // Make sure that we correct load reference aliases through multiple blocks
+        if x != 20 {
+            *q1 = 1;
+            *q2 = 2; // now we'd expect q1 == q2 == 2
+            
+            assert(*q1 == 2);  
+        }
+    }
+}
diff --git a/crates/noirc_evaluator/src/ssa.rs b/crates/noirc_evaluator/src/ssa.rs
index 81982e463a..e689241f9d 100644
--- a/crates/noirc_evaluator/src/ssa.rs
+++ b/crates/noirc_evaluator/src/ssa.rs
@@ -49,12 +49,16 @@ pub(crate) fn optimize_into_acir(
         ssa = ssa
             .inline_functions()
             .print(print_ssa_passes, "After Inlining:")
+            // Run mem2reg with the CFG separated into blocks
+            .mem2reg()
+            .print(print_ssa_passes, "After Mem2Reg:")
             .unroll_loops()
             .print(print_ssa_passes, "After Unrolling:")
             .simplify_cfg()
             .print(print_ssa_passes, "After Simplifying:")
             .flatten_cfg()
             .print(print_ssa_passes, "After Flattening:")
+            // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores
             .mem2reg()
             .print(print_ssa_passes, "After Mem2Reg:")
             .fold_constants()
diff --git a/crates/noirc_evaluator/src/ssa/ir/function.rs b/crates/noirc_evaluator/src/ssa/ir/function.rs
index ab04c1497b..ab97d41878 100644
--- a/crates/noirc_evaluator/src/ssa/ir/function.rs
+++ b/crates/noirc_evaluator/src/ssa/ir/function.rs
@@ -1,4 +1,4 @@
-use std::collections::HashSet;
+use std::collections::BTreeSet;
 
 use iter_extended::vecmap;
 
@@ -107,8 +107,8 @@ impl Function {
     /// Note that self.dfg.basic_blocks_iter() iterates over all blocks,
     /// whether reachable or not. This function should be used if you
     /// want to iterate only reachable blocks.
-    pub(crate) fn reachable_blocks(&self) -> HashSet<BasicBlockId> {
-        let mut blocks = HashSet::new();
+    pub(crate) fn reachable_blocks(&self) -> BTreeSet<BasicBlockId> {
+        let mut blocks = BTreeSet::new();
         let mut stack = vec![self.entry_block];
 
         while let Some(block) = stack.pop() {
diff --git a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs
index a28f2d88bc..18d45228d5 100644
--- a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs
+++ b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs
@@ -110,6 +110,7 @@ pub(super) fn simplify_call(
             }
         }
         Intrinsic::SlicePopFront => {
+            dbg!(arguments[0]);
             let slice = dfg.get_array_constant(arguments[0]);
             if let Some((mut slice, typ)) = slice {
                 let element_count = typ.element_size();
diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index b9e849bb77..91bc1f3069 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -3,37 +3,57 @@
 //! mutable variables into values that are easier to manipulate.
 use std::collections::{BTreeMap, HashMap, HashSet};
 
-use iter_extended::vecmap;
-
 use crate::ssa::{
     ir::{
         basic_block::BasicBlockId,
+        cfg::ControlFlowGraph,
         dfg::DataFlowGraph,
+        dom::DominatorTree,
+        function::Function,
         instruction::{Instruction, InstructionId, TerminatorInstruction},
+        post_order::PostOrder,
         value::{Value, ValueId},
     },
     ssa_gen::Ssa,
 };
 
+use super::unrolling::{find_all_loops, Loops};
+
 impl Ssa {
     /// Attempts to remove any load instructions that recover values that are already available in
-    /// scope, and attempts to remove store that are subsequently redundant, as long as they are
-    /// not stores on memory that will be passed into a function call or returned.
+    /// scope, and attempts to remove stores that are subsequently redundant.
+    /// As long as they are not stores on memory used inside of loops
     pub(crate) fn mem2reg(mut self) -> Ssa {
         for function in self.functions.values_mut() {
             let mut all_protected_allocations = HashSet::new();
-            let contexts = vecmap(function.reachable_blocks(), |block| {
-                let mut context = PerBlockContext::new(block);
-                let allocations_protected_by_block =
-                    context.analyze_allocations_and_eliminate_known_loads(&mut function.dfg);
+
+            let mut context = PerFunctionContext::new(function);
+
+            for block in function.reachable_blocks() {
+                let cfg: ControlFlowGraph = ControlFlowGraph::with_function(function);
+
+                // Maps Load instruction id -> value to replace the result of the load with
+                let mut loads_to_substitute_per_block = BTreeMap::new();
+
+                // Maps Load result id -> value to replace the result of the load with
+                let mut load_values_to_substitute = BTreeMap::new();
+
+                let allocations_protected_by_block = context
+                    .analyze_allocations_and_eliminate_known_loads(
+                        &mut function.dfg,
+                        &mut loads_to_substitute_per_block,
+                        &mut load_values_to_substitute,
+                        block,
+                        &cfg,
+                    );
                 all_protected_allocations.extend(allocations_protected_by_block.into_iter());
-                context
-            });
+            }
+
             // Now that we have a comprehensive list of used allocations across all the
             // function's blocks, it is safe to remove any stores that do not touch such
             // allocations.
-            for context in contexts {
-                context.remove_unused_stores(&mut function.dfg, &all_protected_allocations);
+            for block in function.reachable_blocks() {
+                context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, block);
             }
         }
 
@@ -41,10 +61,27 @@ impl Ssa {
     }
 }
 
-struct PerBlockContext {
-    block_id: BasicBlockId,
-    last_stores: BTreeMap<AllocId, ValueId>,
+struct PerFunctionContext {
+    last_stores_with_block: BTreeMap<(AllocId, BasicBlockId), ValueId>,
+    // Maps Load result id -> (value, block_id)
+    // Used to replace the result of a load with the appropriate block
+    load_values_to_substitute_per_func: BTreeMap<ValueId, (ValueId, BasicBlockId)>,
     store_ids: Vec<InstructionId>,
+    cfg: ControlFlowGraph,
+    post_order: PostOrder,
+    loops: Loops,
+}
+impl PerFunctionContext {
+    fn new(function: &Function) -> Self {
+        PerFunctionContext {
+            last_stores_with_block: BTreeMap::new(),
+            load_values_to_substitute_per_func: BTreeMap::new(),
+            store_ids: Vec::new(),
+            cfg: ControlFlowGraph::with_function(function),
+            post_order: PostOrder::with_function(function),
+            loops: find_all_loops(function),
+        }
+    }
 }
 
 /// An AllocId is the ValueId returned from an allocate instruction. E.g. v0 in v0 = allocate.
@@ -52,50 +89,50 @@ struct PerBlockContext {
 /// an allocate instruction.
 type AllocId = ValueId;
 
-impl PerBlockContext {
-    fn new(block_id: BasicBlockId) -> Self {
-        PerBlockContext { block_id, last_stores: BTreeMap::new(), store_ids: Vec::new() }
-    }
-
+impl PerFunctionContext {
     // Attempts to remove load instructions for which the result is already known from previous
     // store instructions to the same address in the same block.
     fn analyze_allocations_and_eliminate_known_loads(
         &mut self,
         dfg: &mut DataFlowGraph,
+        loads_to_substitute: &mut BTreeMap<InstructionId, ValueId>,
+        load_values_to_substitute_per_block: &mut BTreeMap<ValueId, ValueId>,
+        block_id: BasicBlockId,
+        cfg: &ControlFlowGraph,
     ) -> HashSet<AllocId> {
         let mut protected_allocations = HashSet::new();
-        let block = &dfg[self.block_id];
-
-        // Maps Load instruction id -> value to replace the result of the load with
-        let mut loads_to_substitute = HashMap::new();
-
-        // Maps Load result id -> value to replace the result of the load with
-        let mut load_values_to_substitute = HashMap::new();
+        let block = &dfg[block_id];
 
         for instruction_id in block.instructions() {
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
-                    if let Some(value) = load_values_to_substitute.get(&address) {
-                        address = *value;
-                    }
-
-                    self.last_stores.insert(address, *value);
+                    address = self.fetch_load_value_to_substitute_recursively(
+                        cfg,
+                        block_id,
+                        address,
+                        &mut HashSet::new(),
+                    );
+
+                    self.last_stores_with_block.insert((address, block_id), *value);
                     self.store_ids.push(*instruction_id);
                 }
                 Instruction::Load { mut address } => {
-                    if let Some(value) = load_values_to_substitute.get(&address) {
-                        address = *value;
-                    }
-
-                    if let Some(last_value) = self.last_stores.get(&address) {
-                        let result_value = *dfg
-                            .instruction_results(*instruction_id)
-                            .first()
-                            .expect("ICE: Load instructions should have single result");
-
-                        loads_to_substitute.insert(*instruction_id, *last_value);
-                        load_values_to_substitute.insert(result_value, *last_value);
-                    } else {
+                    address = self.fetch_load_value_to_substitute_recursively(
+                        cfg,
+                        block_id,
+                        address,
+                        &mut HashSet::new(),
+                    );
+
+                    let found_last_value = self.find_load_to_substitute(
+                        block_id,
+                        dfg,
+                        address,
+                        instruction_id,
+                        loads_to_substitute,
+                        load_values_to_substitute_per_block,
+                    );
+                    if !found_last_value {
                         protected_allocations.insert(address);
                     }
                 }
@@ -122,21 +159,128 @@ impl PerBlockContext {
         }
 
         // Substitute load result values
-        for (result_value, new_value) in load_values_to_substitute {
-            let result_value = dfg.resolve(result_value);
-            dfg.set_value_from_id(result_value, new_value);
+        for (result_value, new_value) in load_values_to_substitute_per_block {
+            let result_value = dfg.resolve(*result_value);
+            dfg.set_value_from_id(result_value, *new_value);
         }
 
         // Delete load instructions
         // Even though we could let DIE handle this, doing it here makes the debug output easier
         // to read.
-        dfg[self.block_id]
+        dfg[block_id]
             .instructions_mut()
             .retain(|instruction| !loads_to_substitute.contains_key(instruction));
 
         protected_allocations
     }
 
+    fn fetch_load_value_to_substitute_recursively(
+        &self,
+        cfg: &ControlFlowGraph,
+        block_id: BasicBlockId,
+        address: ValueId,
+        checked_blocks: &mut HashSet<BasicBlockId>,
+    ) -> ValueId {
+        checked_blocks.insert(block_id);
+
+        if let Some((value, load_block_id)) = self.load_values_to_substitute_per_func.get(&address)
+        {
+            if *load_block_id == block_id {
+                return *value;
+            }
+        }
+
+        let mut dom_tree = DominatorTree::with_cfg_and_post_order(cfg, &self.post_order);
+
+        let predecessors = cfg.predecessors(block_id);
+        for predecessor in predecessors {
+            if dom_tree.is_reachable(predecessor) && dom_tree.dominates(predecessor, block_id) {
+                if let Some((value, load_block_id)) =
+                    self.load_values_to_substitute_per_func.get(&address)
+                {
+                    if *load_block_id == predecessor {
+                        return *value;
+                    }
+                }
+
+                if !checked_blocks.contains(&predecessor) {
+                    return self.fetch_load_value_to_substitute_recursively(
+                        cfg,
+                        predecessor,
+                        address,
+                        checked_blocks,
+                    );
+                }
+            }
+        }
+        address
+    }
+
+    fn find_load_to_substitute(
+        &mut self,
+        block_id: BasicBlockId,
+        dfg: &DataFlowGraph,
+        address: ValueId,
+        instruction_id: &InstructionId,
+        loads_to_substitute: &mut BTreeMap<InstructionId, ValueId>,
+        load_values_to_substitute_per_block: &mut BTreeMap<ValueId, ValueId>,
+    ) -> bool {
+        let mut stack = vec![block_id];
+        let mut visited = HashSet::new();
+
+        let mut dom_tree = DominatorTree::with_cfg_and_post_order(&self.cfg, &self.post_order);
+        while let Some(block) = stack.pop() {
+            visited.insert(block);
+
+            for l in self.loops.yet_to_unroll.iter() {
+                // We do not want to substitute loads that take place within loops as this pass
+                // can occur before loop unrolling
+                if block == l.header {
+                    return false;
+                }
+            }
+
+            if let Some(last_value) = self.last_stores_with_block.get(&(address, block)) {
+                let result_value = *dfg
+                    .instruction_results(*instruction_id)
+                    .first()
+                    .expect("ICE: Load instructions should have single result");
+
+                loads_to_substitute.insert(*instruction_id, *last_value);
+                load_values_to_substitute_per_block.insert(result_value, *last_value);
+                self.load_values_to_substitute_per_func.insert(result_value, (*last_value, block));
+                return true;
+            }
+
+            let predecessors = self.cfg.predecessors(block);
+            for predecessor in predecessors {
+                // TODO: Do I need is_reachable here? We are looping over only the reachable blocks but does
+                // that include a reachable block's predecessors?
+                if dom_tree.is_reachable(predecessor) && dom_tree.dominates(predecessor, block) {
+                    if let Some(last_value) =
+                        self.last_stores_with_block.get(&(address, predecessor))
+                    {
+                        let result_value = *dfg
+                            .instruction_results(*instruction_id)
+                            .first()
+                            .expect("ICE: Load instructions should have single result");
+
+                        loads_to_substitute.insert(*instruction_id, *last_value);
+                        load_values_to_substitute_per_block.insert(result_value, *last_value);
+                        self.load_values_to_substitute_per_func
+                            .insert(result_value, (*last_value, block));
+                        return true;
+                    }
+                    // Only recurse further if the predecessor dominates the current block we are checking
+                    if !visited.contains(&predecessor) {
+                        stack.push(predecessor);
+                    }
+                }
+            }
+        }
+        false
+    }
+
     /// Checks whether the given value id refers to an allocation.
     fn value_is_from_allocation(value: ValueId, dfg: &DataFlowGraph) -> bool {
         match &dfg[value] {
@@ -150,9 +294,10 @@ impl PerBlockContext {
     /// Removes all store instructions identified during analysis that aren't present in the
     /// provided `protected_allocations` `HashSet`.
     fn remove_unused_stores(
-        self,
+        &self,
         dfg: &mut DataFlowGraph,
         protected_allocations: &HashSet<AllocId>,
+        block_id: BasicBlockId,
     ) {
         // Scan for unused stores
         let mut stores_to_remove = HashSet::new();
@@ -169,7 +314,7 @@ impl PerBlockContext {
         }
 
         // Delete unused stores
-        dfg[self.block_id]
+        dfg[block_id]
             .instructions_mut()
             .retain(|instruction| !stores_to_remove.contains(instruction));
     }
@@ -320,7 +465,7 @@ mod tests {
             .count()
     }
 
-    // Test that loads across multiple blocks are not removed
+    // Test that loads across multiple blocks are removed
     #[test]
     fn multiple_blocks() {
         // fn main {
@@ -364,24 +509,22 @@ mod tests {
         // fn main {
         //   b0():
         //     v0 = allocate
-        //     store v0, Field 5
-        //     jmp b1(Field 5):  // Optimized to constant 5
-        //   b1(v2: Field):
-        //     v3 = load v0      // kept in program
-        //     store v0, Field 6
-        //     return v2, v3, Field 6 // Optimized to constant 6
+        //     jmp b1(Field 5)
+        //   b1(v3: Field):
+        //     return v3, Field 5, Field 6 // Optimized to constants 5 and 6
         // }
         let ssa = ssa.mem2reg();
+
         let main = ssa.main();
         assert_eq!(main.reachable_blocks().len(), 2);
 
-        // Only the load from the entry block should be removed
+        // The loads should be removed
         assert_eq!(count_loads(main.entry_block(), &main.dfg), 0);
-        assert_eq!(count_loads(b1, &main.dfg), 1);
+        assert_eq!(count_loads(b1, &main.dfg), 0);
 
-        // All stores should be kept
-        assert_eq!(count_stores(main.entry_block(), &main.dfg), 1);
-        assert_eq!(count_stores(b1, &main.dfg), 1);
+        // All stores should be removed
+        assert_eq!(count_stores(main.entry_block(), &main.dfg), 0);
+        assert_eq!(count_stores(b1, &main.dfg), 0);
 
         // The jmp to b1 should also be a constant 5 now
         match main.dfg[main.entry_block()].terminator() {
diff --git a/crates/noirc_evaluator/src/ssa/opt/unrolling.rs b/crates/noirc_evaluator/src/ssa/opt/unrolling.rs
index f6d7c95227..6795133dfe 100644
--- a/crates/noirc_evaluator/src/ssa/opt/unrolling.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/unrolling.rs
@@ -34,10 +34,10 @@ impl Ssa {
     }
 }
 
-struct Loop {
+pub(crate) struct Loop {
     /// The header block of a loop is the block which dominates all the
     /// other blocks in the loop.
-    header: BasicBlockId,
+    pub(crate) header: BasicBlockId,
 
     /// The start of the back_edge n -> d is the block n at the end of
     /// the loop that jumps back to the header block d which restarts the loop.
@@ -47,12 +47,12 @@ struct Loop {
     blocks: HashSet<BasicBlockId>,
 }
 
-struct Loops {
+pub(crate) struct Loops {
     /// The loops that failed to be unrolled so that we do not try to unroll them again.
     /// Each loop is identified by its header block id.
     failed_to_unroll: HashSet<BasicBlockId>,
 
-    yet_to_unroll: Vec<Loop>,
+    pub(crate) yet_to_unroll: Vec<Loop>,
     modified_blocks: HashSet<BasicBlockId>,
     cfg: ControlFlowGraph,
     dom_tree: DominatorTree,
@@ -60,7 +60,7 @@ struct Loops {
 
 /// Find a loop in the program by finding a node that dominates any predecessor node.
 /// The edge where this happens will be the back-edge of the loop.
-fn find_all_loops(function: &Function) -> Loops {
+pub(crate) fn find_all_loops(function: &Function) -> Loops {
     let cfg = ControlFlowGraph::with_function(function);
     let post_order = PostOrder::with_function(function);
     let mut dom_tree = DominatorTree::with_cfg_and_post_order(&cfg, &post_order);
diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs
index 77503d19bc..463164f61c 100644
--- a/crates/noirc_frontend/src/monomorphization/mod.rs
+++ b/crates/noirc_frontend/src/monomorphization/mod.rs
@@ -957,14 +957,9 @@ impl<'interner> Monomorphizer<'interner> {
         ast::Expression::Assign(ast::Assign { expression, lvalue })
     }
 
-    fn lvalue(
-        &mut self,
-        lvalue: HirLValue,
-    ) -> ast::LValue {
+    fn lvalue(&mut self, lvalue: HirLValue) -> ast::LValue {
         match lvalue {
-            HirLValue::Ident(ident, _) => {
-                ast::LValue::Ident(self.local_ident(&ident).unwrap())
-            }
+            HirLValue::Ident(ident, _) => ast::LValue::Ident(self.local_ident(&ident).unwrap()),
             HirLValue::MemberAccess { object, field_index, .. } => {
                 let field_index = field_index.unwrap();
                 let object = Box::new(self.lvalue(*object));

From d129a0a0392e36e401485e9afa9bd5c19476804d Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Wed, 9 Aug 2023 21:16:09 +0000
Subject: [PATCH 02/28] update regression_2218 test

---
 .../nargo_cli/tests/execution_success/references/src/main.nr   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crates/nargo_cli/tests/execution_success/references/src/main.nr b/crates/nargo_cli/tests/execution_success/references/src/main.nr
index 96b94c6873..a6fc5433fd 100644
--- a/crates/nargo_cli/tests/execution_success/references/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/references/src/main.nr
@@ -106,9 +106,10 @@ fn regression_2218(x: Field, y: Field) {
     let q2 = *q;
 
     if x != y {
+        *q1 = 1;
         // Make sure that we correct load reference aliases through multiple blocks
         if x != 20 {
-            *q1 = 1;
+            *q1 = 10;
             *q2 = 2; // now we'd expect q1 == q2 == 2
             
             assert(*q1 == 2);  

From 17d66aae791775ff66b4e027f02ee7f242d00a9a Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 01:32:55 +0000
Subject: [PATCH 03/28] cargo clippy

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 91bc1f3069..a55294a472 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -1,7 +1,7 @@
 //! mem2reg implements a pass for promoting values stored in memory to values in registers where
 //! possible. This is particularly important for converting our memory-based representation of
 //! mutable variables into values that are easier to manipulate.
-use std::collections::{BTreeMap, HashMap, HashSet};
+use std::collections::{BTreeMap, HashSet};
 
 use crate::ssa::{
     ir::{

From e32e665ae2eeea2eea3e6942e3f96e8c3e5bca0d Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 14:19:34 +0000
Subject: [PATCH 04/28] clean up recursive load fetch method

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 21 +++++--------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index a55294a472..f7f2b4092f 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -30,8 +30,6 @@ impl Ssa {
             let mut context = PerFunctionContext::new(function);
 
             for block in function.reachable_blocks() {
-                let cfg: ControlFlowGraph = ControlFlowGraph::with_function(function);
-
                 // Maps Load instruction id -> value to replace the result of the load with
                 let mut loads_to_substitute_per_block = BTreeMap::new();
 
@@ -44,7 +42,6 @@ impl Ssa {
                         &mut loads_to_substitute_per_block,
                         &mut load_values_to_substitute,
                         block,
-                        &cfg,
                     );
                 all_protected_allocations.extend(allocations_protected_by_block.into_iter());
             }
@@ -98,7 +95,6 @@ impl PerFunctionContext {
         loads_to_substitute: &mut BTreeMap<InstructionId, ValueId>,
         load_values_to_substitute_per_block: &mut BTreeMap<ValueId, ValueId>,
         block_id: BasicBlockId,
-        cfg: &ControlFlowGraph,
     ) -> HashSet<AllocId> {
         let mut protected_allocations = HashSet::new();
         let block = &dfg[block_id];
@@ -107,10 +103,8 @@ impl PerFunctionContext {
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
                     address = self.fetch_load_value_to_substitute_recursively(
-                        cfg,
                         block_id,
                         address,
-                        &mut HashSet::new(),
                     );
 
                     self.last_stores_with_block.insert((address, block_id), *value);
@@ -118,10 +112,8 @@ impl PerFunctionContext {
                 }
                 Instruction::Load { mut address } => {
                     address = self.fetch_load_value_to_substitute_recursively(
-                        cfg,
                         block_id,
                         address,
-                        &mut HashSet::new(),
                     );
 
                     let found_last_value = self.find_load_to_substitute(
@@ -176,12 +168,11 @@ impl PerFunctionContext {
 
     fn fetch_load_value_to_substitute_recursively(
         &self,
-        cfg: &ControlFlowGraph,
         block_id: BasicBlockId,
         address: ValueId,
-        checked_blocks: &mut HashSet<BasicBlockId>,
     ) -> ValueId {
-        checked_blocks.insert(block_id);
+        let mut visited = HashSet::new();
+        visited.insert(block_id);
 
         if let Some((value, load_block_id)) = self.load_values_to_substitute_per_func.get(&address)
         {
@@ -190,9 +181,9 @@ impl PerFunctionContext {
             }
         }
 
-        let mut dom_tree = DominatorTree::with_cfg_and_post_order(cfg, &self.post_order);
+        let mut dom_tree = DominatorTree::with_cfg_and_post_order(&self.cfg, &self.post_order);
 
-        let predecessors = cfg.predecessors(block_id);
+        let predecessors = self.cfg.predecessors(block_id);
         for predecessor in predecessors {
             if dom_tree.is_reachable(predecessor) && dom_tree.dominates(predecessor, block_id) {
                 if let Some((value, load_block_id)) =
@@ -203,12 +194,10 @@ impl PerFunctionContext {
                     }
                 }
 
-                if !checked_blocks.contains(&predecessor) {
+                if !visited.contains(&predecessor) {
                     return self.fetch_load_value_to_substitute_recursively(
-                        cfg,
                         predecessor,
                         address,
-                        checked_blocks,
                     );
                 }
             }

From ada6129d6e6231f361dbe46c7501b8fbd479c9c1 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 14:46:19 +0000
Subject: [PATCH 05/28] add test to mem2reg pass

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 108 +++++++++++++++---
 1 file changed, 95 insertions(+), 13 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index f7f2b4092f..d477d1c642 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -102,19 +102,13 @@ impl PerFunctionContext {
         for instruction_id in block.instructions() {
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
-                    address = self.fetch_load_value_to_substitute_recursively(
-                        block_id,
-                        address,
-                    );
+                    address = self.fetch_load_value_to_substitute_recursively(block_id, address);
 
                     self.last_stores_with_block.insert((address, block_id), *value);
                     self.store_ids.push(*instruction_id);
                 }
                 Instruction::Load { mut address } => {
-                    address = self.fetch_load_value_to_substitute_recursively(
-                        block_id,
-                        address,
-                    );
+                    address = self.fetch_load_value_to_substitute_recursively(block_id, address);
 
                     let found_last_value = self.find_load_to_substitute(
                         block_id,
@@ -195,10 +189,7 @@ impl PerFunctionContext {
                 }
 
                 if !visited.contains(&predecessor) {
-                    return self.fetch_load_value_to_substitute_recursively(
-                        predecessor,
-                        address,
-                    );
+                    return self.fetch_load_value_to_substitute_recursively(predecessor, address);
                 }
             }
         }
@@ -321,7 +312,7 @@ mod tests {
             basic_block::BasicBlockId,
             dfg::DataFlowGraph,
             function::RuntimeType,
-            instruction::{Instruction, Intrinsic, TerminatorInstruction},
+            instruction::{BinaryOp, Instruction, Intrinsic, TerminatorInstruction},
             map::Id,
             types::Type,
         },
@@ -526,4 +517,95 @@ mod tests {
             _ => unreachable!(),
         };
     }
+
+    // Test that a load in a predecessor block has been removed if the value
+    // is later stored in a successor block
+    #[test]
+    fn store_with_load_in_predecessor_block() {
+        // fn main {
+        //     b0():
+        //       v0 = allocate
+        //       store Field 0 at v0
+        //       v2 = allocate
+        //       store v0 at v2
+        //       v3 = load v2
+        //       v4 = load v2
+        //       jmp b1()
+        //     b1():
+        //       store Field 1 at v3
+        //       store Field 2 at v4
+        //       v8 = load v3
+        //       v9 = eq v8, Field 2
+        //       return
+        // }
+        let main_id = Id::test_new(0);
+        let mut builder = FunctionBuilder::new("main".into(), main_id, RuntimeType::Acir);
+
+        let v0 = builder.insert_allocate();
+
+        let zero = builder.field_constant(0u128);
+        builder.insert_store(v0, zero);
+
+        let v2 = builder.insert_allocate();
+        builder.insert_store(v2, v0);
+
+        let v3 = builder.insert_load(v2, Type::field());
+        let v4 = builder.insert_load(v2, Type::field());
+        let b1 = builder.insert_block();
+        builder.terminate_with_jmp(b1, vec![]);
+
+        builder.switch_to_block(b1);
+
+        let one = builder.field_constant(1u128);
+        builder.insert_store(v3, one);
+
+        let two = builder.field_constant(2u128);
+        builder.insert_store(v4, two);
+
+        let v8 = builder.insert_load(v3, Type::field());
+        let _ = builder.insert_binary(v8, BinaryOp::Eq, two);
+
+        builder.terminate_with_return(vec![]);
+
+        let ssa = builder.finish();
+        assert_eq!(ssa.main().reachable_blocks().len(), 2);
+
+        // Expected result:
+        // fn main {
+        //     b0():
+        //       v0 = allocate
+        //       v2 = allocate
+        //       jmp b1()
+        //     b1():
+        //       v8 = eq Field 2, Field 2
+        //       return
+        // }
+        let ssa = ssa.mem2reg();
+
+        let main = ssa.main();
+        assert_eq!(main.reachable_blocks().len(), 2);
+
+        // All loads should be removed
+        assert_eq!(count_loads(main.entry_block(), &main.dfg), 0);
+        assert_eq!(count_loads(b1, &main.dfg), 0);
+
+        // All stores should be removed
+        assert_eq!(count_stores(main.entry_block(), &main.dfg), 0);
+        assert_eq!(count_stores(b1, &main.dfg), 0);
+
+        let b1_instructions = main.dfg[b1].instructions();
+        // The first instruction should be a binary operation
+        match &main.dfg[b1_instructions[0]] {
+            Instruction::Binary(binary) => {
+                let lhs =
+                    main.dfg.get_numeric_constant(binary.lhs).expect("Expected constant value");
+                let rhs =
+                    main.dfg.get_numeric_constant(binary.rhs).expect("Expected constant value");
+
+                assert_eq!(lhs, rhs);
+                assert_eq!(lhs, FieldElement::from(2u128));
+            }
+            _ => unreachable!(),
+        }
+    }
 }

From bedfa2af951fa0b78f9c8aa710546569408940cb Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 14:57:00 +0000
Subject: [PATCH 06/28] cleaner search methods

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 44 ++++++-------------
 1 file changed, 13 insertions(+), 31 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index d477d1c642..bfcb28ce45 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -160,6 +160,8 @@ impl PerFunctionContext {
         protected_allocations
     }
 
+    // TODO: switch to while-loop based search so I am not recomputing the dom tree or pass it directly outside of the function
+    // I would rather have this func match `find_load_to_substitute`
     fn fetch_load_value_to_substitute_recursively(
         &self,
         block_id: BasicBlockId,
@@ -168,6 +170,7 @@ impl PerFunctionContext {
         let mut visited = HashSet::new();
         visited.insert(block_id);
 
+        // If the load value to substitute is in the current block we substitute it
         if let Some((value, load_block_id)) = self.load_values_to_substitute_per_func.get(&address)
         {
             if *load_block_id == block_id {
@@ -179,18 +182,11 @@ impl PerFunctionContext {
 
         let predecessors = self.cfg.predecessors(block_id);
         for predecessor in predecessors {
-            if dom_tree.is_reachable(predecessor) && dom_tree.dominates(predecessor, block_id) {
-                if let Some((value, load_block_id)) =
-                    self.load_values_to_substitute_per_func.get(&address)
-                {
-                    if *load_block_id == predecessor {
-                        return *value;
-                    }
-                }
-
-                if !visited.contains(&predecessor) {
-                    return self.fetch_load_value_to_substitute_recursively(predecessor, address);
-                }
+            if dom_tree.is_reachable(predecessor)
+                && dom_tree.dominates(predecessor, block_id)
+                && !visited.contains(&predecessor)
+            {
+                return self.fetch_load_value_to_substitute_recursively(predecessor, address);
             }
         }
         address
@@ -236,25 +232,11 @@ impl PerFunctionContext {
             for predecessor in predecessors {
                 // TODO: Do I need is_reachable here? We are looping over only the reachable blocks but does
                 // that include a reachable block's predecessors?
-                if dom_tree.is_reachable(predecessor) && dom_tree.dominates(predecessor, block) {
-                    if let Some(last_value) =
-                        self.last_stores_with_block.get(&(address, predecessor))
-                    {
-                        let result_value = *dfg
-                            .instruction_results(*instruction_id)
-                            .first()
-                            .expect("ICE: Load instructions should have single result");
-
-                        loads_to_substitute.insert(*instruction_id, *last_value);
-                        load_values_to_substitute_per_block.insert(result_value, *last_value);
-                        self.load_values_to_substitute_per_func
-                            .insert(result_value, (*last_value, block));
-                        return true;
-                    }
-                    // Only recurse further if the predecessor dominates the current block we are checking
-                    if !visited.contains(&predecessor) {
-                        stack.push(predecessor);
-                    }
+                if dom_tree.is_reachable(predecessor)
+                    && dom_tree.dominates(predecessor, block)
+                    && !visited.contains(&predecessor)
+                {
+                    stack.push(predecessor);
                 }
             }
         }

From 3d422e131f8d682398c0f49e5b62a9bca28b0ade Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 15:05:14 +0000
Subject: [PATCH 07/28] switch from recursive to while-loop based search for
 loads

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 49 +++++++++----------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index bfcb28ce45..f77a5b1154 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -102,18 +102,18 @@ impl PerFunctionContext {
         for instruction_id in block.instructions() {
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
-                    address = self.fetch_load_value_to_substitute_recursively(block_id, address);
+                    address = self.fetch_load_value_to_substitute(block_id, address);
 
                     self.last_stores_with_block.insert((address, block_id), *value);
                     self.store_ids.push(*instruction_id);
                 }
                 Instruction::Load { mut address } => {
-                    address = self.fetch_load_value_to_substitute_recursively(block_id, address);
+                    address = self.fetch_load_value_to_substitute(block_id, address);
 
                     let found_last_value = self.find_load_to_substitute(
                         block_id,
-                        dfg,
                         address,
+                        dfg,
                         instruction_id,
                         loads_to_substitute,
                         load_values_to_substitute_per_block,
@@ -160,33 +160,30 @@ impl PerFunctionContext {
         protected_allocations
     }
 
-    // TODO: switch to while-loop based search so I am not recomputing the dom tree or pass it directly outside of the function
-    // I would rather have this func match `find_load_to_substitute`
-    fn fetch_load_value_to_substitute_recursively(
-        &self,
-        block_id: BasicBlockId,
-        address: ValueId,
-    ) -> ValueId {
+    fn fetch_load_value_to_substitute(&self, block_id: BasicBlockId, address: ValueId) -> ValueId {
+        let mut stack = vec![block_id];
         let mut visited = HashSet::new();
-        visited.insert(block_id);
-
-        // If the load value to substitute is in the current block we substitute it
-        if let Some((value, load_block_id)) = self.load_values_to_substitute_per_func.get(&address)
-        {
-            if *load_block_id == block_id {
-                return *value;
-            }
-        }
 
         let mut dom_tree = DominatorTree::with_cfg_and_post_order(&self.cfg, &self.post_order);
+        while let Some(block) = stack.pop() {
+            visited.insert(block);
 
-        let predecessors = self.cfg.predecessors(block_id);
-        for predecessor in predecessors {
-            if dom_tree.is_reachable(predecessor)
-                && dom_tree.dominates(predecessor, block_id)
-                && !visited.contains(&predecessor)
+            if let Some((value, load_block_id)) =
+                self.load_values_to_substitute_per_func.get(&address)
             {
-                return self.fetch_load_value_to_substitute_recursively(predecessor, address);
+                if *load_block_id == block {
+                    return *value;
+                }
+            }
+
+            let predecessors = self.cfg.predecessors(block);
+            for predecessor in predecessors {
+                if dom_tree.is_reachable(predecessor)
+                    && dom_tree.dominates(predecessor, block)
+                    && !visited.contains(&predecessor)
+                {
+                    stack.push(predecessor);
+                }
             }
         }
         address
@@ -195,8 +192,8 @@ impl PerFunctionContext {
     fn find_load_to_substitute(
         &mut self,
         block_id: BasicBlockId,
-        dfg: &DataFlowGraph,
         address: ValueId,
+        dfg: &DataFlowGraph,
         instruction_id: &InstructionId,
         loads_to_substitute: &mut BTreeMap<InstructionId, ValueId>,
         load_values_to_substitute_per_block: &mut BTreeMap<ValueId, ValueId>,

From c12c5bbd85238b021681ead38669d2d1560a1ecb Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 15:22:54 +0000
Subject: [PATCH 08/28] add comments to mem2reg pass

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index f77a5b1154..9b3862ca5e 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -160,6 +160,10 @@ impl PerFunctionContext {
         protected_allocations
     }
 
+    // This method finds the load values to substitute for a given address
+    // The search starts at the block supplied as a parameter. If there is not a load to substitute
+    // the CFG is analyzed to determine whether a predecessor block has a load value to substitute.
+    // If there is no load value to substitute the original address is returned.
     fn fetch_load_value_to_substitute(&self, block_id: BasicBlockId, address: ValueId) -> ValueId {
         let mut stack = vec![block_id];
         let mut visited = HashSet::new();
@@ -168,6 +172,8 @@ impl PerFunctionContext {
         while let Some(block) = stack.pop() {
             visited.insert(block);
 
+            // Check whether there is a load value to substitute in the current block.
+            // Return the value if found.
             if let Some((value, load_block_id)) =
                 self.load_values_to_substitute_per_func.get(&address)
             {
@@ -176,6 +182,7 @@ impl PerFunctionContext {
                 }
             }
 
+            // If no load values to substitute have been found in the current block, check the block's predecessors.
             let predecessors = self.cfg.predecessors(block);
             for predecessor in predecessors {
                 if dom_tree.is_reachable(predecessor)
@@ -189,6 +196,10 @@ impl PerFunctionContext {
         address
     }
 
+    // This method determines which loads should be substituted.
+    // Starting at the block supplied as a parameter, we check whether a store has occurred with the given address.
+    // If no store has occurred in the supplied block, the CFG is analyzed to determine whether
+    // a predecessor block has a store at the given address.
     fn find_load_to_substitute(
         &mut self,
         block_id: BasicBlockId,
@@ -213,6 +224,8 @@ impl PerFunctionContext {
                 }
             }
 
+            // Check whether there has been a store instruction in the current block
+            // If there has been a store, add a load to be substituted.
             if let Some(last_value) = self.last_stores_with_block.get(&(address, block)) {
                 let result_value = *dfg
                     .instruction_results(*instruction_id)
@@ -225,6 +238,7 @@ impl PerFunctionContext {
                 return true;
             }
 
+            // If no stores have been found in the current block, check the block's predecessors.
             let predecessors = self.cfg.predecessors(block);
             for predecessor in predecessors {
                 // TODO: Do I need is_reachable here? We are looping over only the reachable blocks but does

From 9988d913cbadbd47b768c8b0e5dcc2d0c694c62f Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 15:24:08 +0000
Subject: [PATCH 09/28] fmt

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 9b3862ca5e..55439fe881 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -68,6 +68,7 @@ struct PerFunctionContext {
     post_order: PostOrder,
     loops: Loops,
 }
+
 impl PerFunctionContext {
     fn new(function: &Function) -> Self {
         PerFunctionContext {

From 7cc6afe27135a08b9dcb3c4e390feefe181c19e6 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 15:30:38 +0000
Subject: [PATCH 10/28] remove dbg

---
 crates/noirc_evaluator/src/ssa/ir/instruction/call.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs
index 18d45228d5..a28f2d88bc 100644
--- a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs
+++ b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs
@@ -110,7 +110,6 @@ pub(super) fn simplify_call(
             }
         }
         Intrinsic::SlicePopFront => {
-            dbg!(arguments[0]);
             let slice = dfg.get_array_constant(arguments[0]);
             if let Some((mut slice, typ)) = slice {
                 let element_count = typ.element_size();

From fc7327a3ee07c522f74a2d818a4ecd856d2ac59f Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 17:23:12 +0000
Subject: [PATCH 11/28] move dom tree per func

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 55439fe881..68825d6472 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -28,6 +28,8 @@ impl Ssa {
             let mut all_protected_allocations = HashSet::new();
 
             let mut context = PerFunctionContext::new(function);
+            let post_order = PostOrder::with_function(function);
+            let mut dom_tree = DominatorTree::with_cfg_and_post_order(&context.cfg, &post_order);
 
             for block in function.reachable_blocks() {
                 // Maps Load instruction id -> value to replace the result of the load with
@@ -42,6 +44,7 @@ impl Ssa {
                         &mut loads_to_substitute_per_block,
                         &mut load_values_to_substitute,
                         block,
+                        &mut dom_tree,
                     );
                 all_protected_allocations.extend(allocations_protected_by_block.into_iter());
             }
@@ -53,7 +56,6 @@ impl Ssa {
                 context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, block);
             }
         }
-
         self
     }
 }
@@ -65,7 +67,6 @@ struct PerFunctionContext {
     load_values_to_substitute_per_func: BTreeMap<ValueId, (ValueId, BasicBlockId)>,
     store_ids: Vec<InstructionId>,
     cfg: ControlFlowGraph,
-    post_order: PostOrder,
     loops: Loops,
 }
 
@@ -76,7 +77,6 @@ impl PerFunctionContext {
             load_values_to_substitute_per_func: BTreeMap::new(),
             store_ids: Vec::new(),
             cfg: ControlFlowGraph::with_function(function),
-            post_order: PostOrder::with_function(function),
             loops: find_all_loops(function),
         }
     }
@@ -96,6 +96,7 @@ impl PerFunctionContext {
         loads_to_substitute: &mut BTreeMap<InstructionId, ValueId>,
         load_values_to_substitute_per_block: &mut BTreeMap<ValueId, ValueId>,
         block_id: BasicBlockId,
+        dom_tree: &mut DominatorTree,
     ) -> HashSet<AllocId> {
         let mut protected_allocations = HashSet::new();
         let block = &dfg[block_id];
@@ -103,13 +104,13 @@ impl PerFunctionContext {
         for instruction_id in block.instructions() {
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
-                    address = self.fetch_load_value_to_substitute(block_id, address);
+                    address = self.fetch_load_value_to_substitute(block_id, address, dom_tree);
 
                     self.last_stores_with_block.insert((address, block_id), *value);
                     self.store_ids.push(*instruction_id);
                 }
                 Instruction::Load { mut address } => {
-                    address = self.fetch_load_value_to_substitute(block_id, address);
+                    address = self.fetch_load_value_to_substitute(block_id, address, dom_tree);
 
                     let found_last_value = self.find_load_to_substitute(
                         block_id,
@@ -118,6 +119,7 @@ impl PerFunctionContext {
                         instruction_id,
                         loads_to_substitute,
                         load_values_to_substitute_per_block,
+                        dom_tree,
                     );
                     if !found_last_value {
                         protected_allocations.insert(address);
@@ -165,11 +167,11 @@ impl PerFunctionContext {
     // The search starts at the block supplied as a parameter. If there is not a load to substitute
     // the CFG is analyzed to determine whether a predecessor block has a load value to substitute.
     // If there is no load value to substitute the original address is returned.
-    fn fetch_load_value_to_substitute(&self, block_id: BasicBlockId, address: ValueId) -> ValueId {
+    fn fetch_load_value_to_substitute(&self, block_id: BasicBlockId, address: ValueId, dom_tree: &mut DominatorTree) -> ValueId {
         let mut stack = vec![block_id];
         let mut visited = HashSet::new();
 
-        let mut dom_tree = DominatorTree::with_cfg_and_post_order(&self.cfg, &self.post_order);
+        // let mut dom_tree = DominatorTree::with_cfg_and_post_order(&self.cfg, &self.post_order);
         while let Some(block) = stack.pop() {
             visited.insert(block);
 
@@ -209,11 +211,11 @@ impl PerFunctionContext {
         instruction_id: &InstructionId,
         loads_to_substitute: &mut BTreeMap<InstructionId, ValueId>,
         load_values_to_substitute_per_block: &mut BTreeMap<ValueId, ValueId>,
+        dom_tree: &mut DominatorTree,
     ) -> bool {
         let mut stack = vec![block_id];
         let mut visited = HashSet::new();
 
-        let mut dom_tree = DominatorTree::with_cfg_and_post_order(&self.cfg, &self.post_order);
         while let Some(block) = stack.pop() {
             visited.insert(block);
 

From 0bb6401187cfc5bbdc3cef5bd168aae9a32cbcd7 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 17:23:26 +0000
Subject: [PATCH 12/28] cargo fmt

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 68825d6472..6f65d3eebd 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -167,7 +167,12 @@ impl PerFunctionContext {
     // The search starts at the block supplied as a parameter. If there is not a load to substitute
     // the CFG is analyzed to determine whether a predecessor block has a load value to substitute.
     // If there is no load value to substitute the original address is returned.
-    fn fetch_load_value_to_substitute(&self, block_id: BasicBlockId, address: ValueId, dom_tree: &mut DominatorTree) -> ValueId {
+    fn fetch_load_value_to_substitute(
+        &self,
+        block_id: BasicBlockId,
+        address: ValueId,
+        dom_tree: &mut DominatorTree,
+    ) -> ValueId {
         let mut stack = vec![block_id];
         let mut visited = HashSet::new();
 

From 5baf9e8f2252afbc2876d4b7c639ff3d4b21ba12 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 17:26:46 +0000
Subject: [PATCH 13/28] make dom tree part of the func context

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 28 +++++++++----------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 6f65d3eebd..a2aa75da4d 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -28,8 +28,6 @@ impl Ssa {
             let mut all_protected_allocations = HashSet::new();
 
             let mut context = PerFunctionContext::new(function);
-            let post_order = PostOrder::with_function(function);
-            let mut dom_tree = DominatorTree::with_cfg_and_post_order(&context.cfg, &post_order);
 
             for block in function.reachable_blocks() {
                 // Maps Load instruction id -> value to replace the result of the load with
@@ -44,7 +42,6 @@ impl Ssa {
                         &mut loads_to_substitute_per_block,
                         &mut load_values_to_substitute,
                         block,
-                        &mut dom_tree,
                     );
                 all_protected_allocations.extend(allocations_protected_by_block.into_iter());
             }
@@ -67,16 +64,21 @@ struct PerFunctionContext {
     load_values_to_substitute_per_func: BTreeMap<ValueId, (ValueId, BasicBlockId)>,
     store_ids: Vec<InstructionId>,
     cfg: ControlFlowGraph,
+    dom_tree: DominatorTree,
     loops: Loops,
 }
 
 impl PerFunctionContext {
     fn new(function: &Function) -> Self {
+        let cfg = ControlFlowGraph::with_function(function);
+        let post_order = PostOrder::with_function(function);
+        let dom_tree = DominatorTree::with_cfg_and_post_order(&cfg, &post_order);
         PerFunctionContext {
             last_stores_with_block: BTreeMap::new(),
             load_values_to_substitute_per_func: BTreeMap::new(),
             store_ids: Vec::new(),
-            cfg: ControlFlowGraph::with_function(function),
+            cfg,
+            dom_tree,
             loops: find_all_loops(function),
         }
     }
@@ -96,7 +98,6 @@ impl PerFunctionContext {
         loads_to_substitute: &mut BTreeMap<InstructionId, ValueId>,
         load_values_to_substitute_per_block: &mut BTreeMap<ValueId, ValueId>,
         block_id: BasicBlockId,
-        dom_tree: &mut DominatorTree,
     ) -> HashSet<AllocId> {
         let mut protected_allocations = HashSet::new();
         let block = &dfg[block_id];
@@ -104,13 +105,13 @@ impl PerFunctionContext {
         for instruction_id in block.instructions() {
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
-                    address = self.fetch_load_value_to_substitute(block_id, address, dom_tree);
+                    address = self.fetch_load_value_to_substitute(block_id, address);
 
                     self.last_stores_with_block.insert((address, block_id), *value);
                     self.store_ids.push(*instruction_id);
                 }
                 Instruction::Load { mut address } => {
-                    address = self.fetch_load_value_to_substitute(block_id, address, dom_tree);
+                    address = self.fetch_load_value_to_substitute(block_id, address);
 
                     let found_last_value = self.find_load_to_substitute(
                         block_id,
@@ -119,7 +120,6 @@ impl PerFunctionContext {
                         instruction_id,
                         loads_to_substitute,
                         load_values_to_substitute_per_block,
-                        dom_tree,
                     );
                     if !found_last_value {
                         protected_allocations.insert(address);
@@ -168,10 +168,9 @@ impl PerFunctionContext {
     // the CFG is analyzed to determine whether a predecessor block has a load value to substitute.
     // If there is no load value to substitute the original address is returned.
     fn fetch_load_value_to_substitute(
-        &self,
+        &mut self,
         block_id: BasicBlockId,
         address: ValueId,
-        dom_tree: &mut DominatorTree,
     ) -> ValueId {
         let mut stack = vec![block_id];
         let mut visited = HashSet::new();
@@ -193,8 +192,8 @@ impl PerFunctionContext {
             // If no load values to substitute have been found in the current block, check the block's predecessors.
             let predecessors = self.cfg.predecessors(block);
             for predecessor in predecessors {
-                if dom_tree.is_reachable(predecessor)
-                    && dom_tree.dominates(predecessor, block)
+                if self.dom_tree.is_reachable(predecessor)
+                    && self.dom_tree.dominates(predecessor, block)
                     && !visited.contains(&predecessor)
                 {
                     stack.push(predecessor);
@@ -216,7 +215,6 @@ impl PerFunctionContext {
         instruction_id: &InstructionId,
         loads_to_substitute: &mut BTreeMap<InstructionId, ValueId>,
         load_values_to_substitute_per_block: &mut BTreeMap<ValueId, ValueId>,
-        dom_tree: &mut DominatorTree,
     ) -> bool {
         let mut stack = vec![block_id];
         let mut visited = HashSet::new();
@@ -251,8 +249,8 @@ impl PerFunctionContext {
             for predecessor in predecessors {
                 // TODO: Do I need is_reachable here? We are looping over only the reachable blocks but does
                 // that include a reachable block's predecessors?
-                if dom_tree.is_reachable(predecessor)
-                    && dom_tree.dominates(predecessor, block)
+                if self.dom_tree.is_reachable(predecessor)
+                    && self.dom_tree.dominates(predecessor, block)
                     && !visited.contains(&predecessor)
                 {
                     stack.push(predecessor);

From 06256c6e8ccf5cb33e29650bee826ea003302969 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Thu, 10 Aug 2023 17:31:12 +0000
Subject: [PATCH 14/28] remove old ocmment

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index a2aa75da4d..f73b66a983 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -24,6 +24,7 @@ impl Ssa {
     /// scope, and attempts to remove stores that are subsequently redundant.
     /// As long as they are not stores on memory used inside of loops
     pub(crate) fn mem2reg(mut self) -> Ssa {
+        dbg!("start mem2reg");
         for function in self.functions.values_mut() {
             let mut all_protected_allocations = HashSet::new();
 
@@ -53,6 +54,7 @@ impl Ssa {
                 context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, block);
             }
         }
+        dbg!("ended mem2reg");
         self
     }
 }
@@ -175,7 +177,6 @@ impl PerFunctionContext {
         let mut stack = vec![block_id];
         let mut visited = HashSet::new();
 
-        // let mut dom_tree = DominatorTree::with_cfg_and_post_order(&self.cfg, &self.post_order);
         while let Some(block) = stack.pop() {
             visited.insert(block);
 

From c8f9317d90b9f8083f5b6e6cb135584b18c7fe6d Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Fri, 11 Aug 2023 02:03:51 +0000
Subject: [PATCH 15/28] remove loops from mem2reg function context

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 56 +++++++++----------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index f73b66a983..5f14d62b42 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -67,7 +67,6 @@ struct PerFunctionContext {
     store_ids: Vec<InstructionId>,
     cfg: ControlFlowGraph,
     dom_tree: DominatorTree,
-    loops: Loops,
 }
 
 impl PerFunctionContext {
@@ -81,7 +80,6 @@ impl PerFunctionContext {
             store_ids: Vec::new(),
             cfg,
             dom_tree,
-            loops: find_all_loops(function),
         }
     }
 }
@@ -190,15 +188,8 @@ impl PerFunctionContext {
                 }
             }
 
-            // If no load values to substitute have been found in the current block, check the block's predecessors.
-            let predecessors = self.cfg.predecessors(block);
-            for predecessor in predecessors {
-                if self.dom_tree.is_reachable(predecessor)
-                    && self.dom_tree.dominates(predecessor, block)
-                    && !visited.contains(&predecessor)
-                {
-                    stack.push(predecessor);
-                }
+            if let Some(predecessor) = self.block_has_predecessor(block, &visited) {
+                stack.push(predecessor);
             }
         }
         address
@@ -223,14 +214,6 @@ impl PerFunctionContext {
         while let Some(block) = stack.pop() {
             visited.insert(block);
 
-            for l in self.loops.yet_to_unroll.iter() {
-                // We do not want to substitute loads that take place within loops as this pass
-                // can occur before loop unrolling
-                if block == l.header {
-                    return false;
-                }
-            }
-
             // Check whether there has been a store instruction in the current block
             // If there has been a store, add a load to be substituted.
             if let Some(last_value) = self.last_stores_with_block.get(&(address, block)) {
@@ -245,22 +228,35 @@ impl PerFunctionContext {
                 return true;
             }
 
-            // If no stores have been found in the current block, check the block's predecessors.
-            let predecessors = self.cfg.predecessors(block);
-            for predecessor in predecessors {
-                // TODO: Do I need is_reachable here? We are looping over only the reachable blocks but does
-                // that include a reachable block's predecessors?
-                if self.dom_tree.is_reachable(predecessor)
-                    && self.dom_tree.dominates(predecessor, block)
-                    && !visited.contains(&predecessor)
-                {
-                    stack.push(predecessor);
-                }
+            if let Some(predecessor) = self.block_has_predecessor(block, &visited) {
+                stack.push(predecessor);
             }
         }
         false
     }
 
+    // If no loads or stores have been found in the current block, check the block's predecessors.
+    // Only check blocks with one predecessor as otherwise a constant value could be propogated
+    // through successor blocks with multiple branches that rely on other simplification passes
+    // such as loop unrolling or flattening of the CFG.
+    fn block_has_predecessor(
+        &mut self,
+        block_id: BasicBlockId,
+        visited: &HashSet<BasicBlockId>,
+    ) -> Option<BasicBlockId> {
+        let mut predecessors = self.cfg.predecessors(block_id);
+        if predecessors.len() == 1 {
+            let predecessor = predecessors.next().unwrap();
+            if self.dom_tree.is_reachable(predecessor)
+                && self.dom_tree.dominates(predecessor, block_id)
+                && !visited.contains(&predecessor)
+            {
+                return Some(predecessor);
+            }
+        }
+        None
+    }
+
     /// Checks whether the given value id refers to an allocation.
     fn value_is_from_allocation(value: ValueId, dfg: &DataFlowGraph) -> bool {
         match &dfg[value] {

From a824ed3d2f3bcf8b5e2362bb95f208ed965f8daf Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Fri, 11 Aug 2023 02:41:25 +0000
Subject: [PATCH 16/28] remove import

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 5f14d62b42..191ed8a0d9 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -17,8 +17,6 @@ use crate::ssa::{
     ssa_gen::Ssa,
 };
 
-use super::unrolling::{find_all_loops, Loops};
-
 impl Ssa {
     /// Attempts to remove any load instructions that recover values that are already available in
     /// scope, and attempts to remove stores that are subsequently redundant.

From e345f8cced4943f68985951407a2d410c3b29772 Mon Sep 17 00:00:00 2001
From: jfecher <jake@aztecprotocol.com>
Date: Fri, 11 Aug 2023 11:46:43 -0500
Subject: [PATCH 17/28] Update crates/noirc_evaluator/src/ssa/opt/mem2reg.rs

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 191ed8a0d9..0e75eb2223 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -52,7 +52,6 @@ impl Ssa {
                 context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, block);
             }
         }
-        dbg!("ended mem2reg");
         self
     }
 }

From fb53610927e748dd2461995f3f98cfe050f8ac50 Mon Sep 17 00:00:00 2001
From: jfecher <jake@aztecprotocol.com>
Date: Fri, 11 Aug 2023 11:46:52 -0500
Subject: [PATCH 18/28] Update crates/noirc_evaluator/src/ssa/opt/mem2reg.rs

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 0e75eb2223..c020bcb65c 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -22,7 +22,6 @@ impl Ssa {
     /// scope, and attempts to remove stores that are subsequently redundant.
     /// As long as they are not stores on memory used inside of loops
     pub(crate) fn mem2reg(mut self) -> Ssa {
-        dbg!("start mem2reg");
         for function in self.functions.values_mut() {
             let mut all_protected_allocations = HashSet::new();
 

From e07c999d558867ce0c4bf1404a6c54d84c0f0733 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Sat, 12 Aug 2023 22:22:42 +0000
Subject: [PATCH 19/28] m em2reg debug work

---
 .../array_dynamic/Prover.toml                 |   3 +-
 .../array_dynamic/src/main.nr                 |  36 +--
 .../execution_success/references/src/main.nr  | 187 ++++++++++++---
 .../execution_success/regression/src/main.nr  |   1 +
 .../execution_success/sha2_blocks/src/main.nr |  26 +-
 crates/noirc_evaluator/src/ssa.rs             |   4 +
 crates/noirc_evaluator/src/ssa/ir/cfg.rs      |   9 +-
 .../noirc_evaluator/src/ssa/ir/post_order.rs  |   1 +
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 225 +++++++++++++++++-
 .../noirc_evaluator/src/ssa/opt/unrolling.rs  |   4 +-
 10 files changed, 412 insertions(+), 84 deletions(-)

diff --git a/crates/nargo_cli/tests/execution_success/array_dynamic/Prover.toml b/crates/nargo_cli/tests/execution_success/array_dynamic/Prover.toml
index 750b3129ec..957a044a1a 100644
--- a/crates/nargo_cli/tests/execution_success/array_dynamic/Prover.toml
+++ b/crates/nargo_cli/tests/execution_success/array_dynamic/Prover.toml
@@ -1,5 +1,6 @@
 x = [104, 101, 108, 108, 111]
-z = "59"
+# z = "59"
+z = "4"
 t = "10"
 index = [0,1,2,3,4]
 index2 =  [0,1,2,3,4]
diff --git a/crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr b/crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr
index b27c96b30c..d9411bfb5f 100644
--- a/crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr
@@ -1,32 +1,38 @@
 
 fn main(x: [u32; 5], mut z: u32, t: u32, index: [Field;5], index2: [Field;5], offset: Field, sublen: Field) {
-    let idx = (z - 5*t - 5) as Field;
+    // let idx = (z - 5*t - 5) as Field;
+    let idx = z as Field;
+    // dep::std::println(idx);
     //dynamic array test
     dyn_array(x, idx, idx - 3); 
 
     //regression for issue 1283
-    let mut s = 0;
-    let x3 = [246,159,32,176,8];
-    for i in 0..5 {
-      s += x3[index[i]];
-    }
-    assert(s!=0);
+    // let mut s = 0;
+    // let x3 = [246,159,32,176,8];
+    // for i in 0..5 {
+    //   s += x3[index[i]];
+    // }
+    // assert(s!=0);
 
-    if 3 < (sublen as u32) {
-      assert(index[offset + 3] == index2[3]);
-    }
+    // if 3 < (sublen as u32) {
+    //   assert(index[offset + 3] == index2[3]);
+    // }
 }
 
 fn dyn_array(mut x: [u32; 5], y: Field, z: Field) {
-    assert(x[y] == 111);
-    assert(x[z] == 101);
-    x[z] = 0;
-   assert(x[y] == 111);
-    assert(x[1] == 0);
+    // assert(x[y] == 111);
+    // assert(x[z] == 101);
+    // x[z] = 0;
+    // assert(x[y] == 111);
+    // assert(x[1] == 0);
     if y as u32 < 10 {
+      // dep::std::println(x);
+      let q = x[y] - 2;
+      // dep::std::println(q);
       x[y] = x[y] - 2;
     } else {
       x[y] = 0;
     }
+    // dep::std::println(x);
     assert(x[4] == 109);
 }
\ No newline at end of file
diff --git a/crates/nargo_cli/tests/execution_success/references/src/main.nr b/crates/nargo_cli/tests/execution_success/references/src/main.nr
index a6fc5433fd..e8ee19e33f 100644
--- a/crates/nargo_cli/tests/execution_success/references/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/references/src/main.nr
@@ -1,41 +1,48 @@
 fn main(mut x: Field) {
-    regression_2218(x, 10);
-
-    add1(&mut x);
-    assert(x == 3);
-
-    let mut s = S { y: x };
-    s.add2();
-    assert(s.y == 5);
-
-    // Regression for #1946: Method resolution error when calling &mut methods with a variable of type &mut T
-    let s_ref = &mut s;
-    s_ref.add2();
-    assert(s.y == 7);
-
-    // Test that normal mutable variables are still copied
-    let mut a = 0;
-    mutate_copy(a);
-    assert(a == 0);
-
-    // Test something 3 allocations deep
-    let mut nested_allocations = Nested { y: &mut &mut 0 };
-    add1(*nested_allocations.y);
-    assert(**nested_allocations.y == 1);
-
-    // Test nested struct allocations with a mutable reference to an array.
-    let mut c = C {
-        foo: 0,
-        bar: &mut C2 {
-            array: &mut [1, 2],
-        },
-    };
-    *c.bar.array = [3, 4];
-    assert(*c.bar.array == [3, 4]);
-
-    regression_1887();
-    regression_2054();
-    regression_2030();
+    // add1(&mut x);
+    // assert(x == 3);
+
+    // let mut s = S { y: x };
+    // s.add2();
+    // assert(s.y == 5);
+
+    // // Regression for #1946: Method resolution error when calling &mut methods with a variable of type &mut T
+    // let s_ref = &mut s;
+    // s_ref.add2();
+    // assert(s.y == 7);
+
+    // // Test that normal mutable variables are still copied
+    // let mut a = 0;
+    // mutate_copy(a);
+    // assert(a == 0);
+
+    // // Test something 3 allocations deep
+    // let mut nested_allocations = Nested { y: &mut &mut 0 };
+    // add1(*nested_allocations.y);
+    // assert(**nested_allocations.y == 1);
+
+    // // Test nested struct allocations with a mutable reference to an array.
+    // let mut c = C {
+    //     foo: 0,
+    //     bar: &mut C2 {
+    //         array: &mut [1, 2],
+    //     },
+    // };
+    // *c.bar.array = [3, 4];
+    // assert(*c.bar.array == [3, 4]);
+
+    // regression_1887();
+    // regression_2054();
+    // regression_2030();
+
+
+    // Compile time works
+    // regression_2218(5, 10);
+    // This breaks, thus we should fix our mem2reg here
+    // regression_2218(x, 10);
+    regression_2218_old(x, 10);
+    // regression_2218_loop(x, 10);
+    // regression_2221(true);
 }
 
 fn add1(x: &mut Field) {
@@ -111,8 +118,112 @@ fn regression_2218(x: Field, y: Field) {
         if x != 20 {
             *q1 = 10;
             *q2 = 2; // now we'd expect q1 == q2 == 2
-            
+            // dep::std::println(*q1);
             assert(*q1 == 2);  
+        } else {
+            *q2 = 15;
+            assert(*q1 == 15);
+        }
+    }
+    // TODO: this breaks mem2reg per func 
+    else {
+        *q2 = 20;
+        assert(*q1 == 20);  
+    }
+    // dep::std::println(*q1);
+    assert(*q1 == 2);  
+}
+
+fn regression_2218_old(x: Field, y: Field) {
+    let q = &mut &mut 0;
+    let q1 = *q;
+    let q2 = *q;
+
+    if x != y {
+        *q1 = 1;
+        // Make sure that we correct load reference aliases through multiple blocks
+        if x != 20 {
+            *q1 = 10;
+            *q2 = 2; // now we'd expect q1 == q2 == 2
+            // dep::std::println(*q1);
+            assert(*q1 == 2); 
         }
     }
+    // dep::std::println(*q1);
+    assert(*q1 == 2);  
 }
+
+fn regression_2218_loop(x: Field, y: Field) {
+    let q = &mut &mut 0;
+    let q1 = *q;
+    let q2 = *q;
+
+    for _ in 0..1 {
+        if x != y {
+            *q1 = 10;
+            *q2 = 2; // now we'd expect q1 == q2 == 2
+            
+            assert(*q1 == 2);
+        }  
+    }
+    // dep::std::println(*q1);
+    assert(*q1 == 2);
+
+    // for _ in 0..1 {
+    //     for _ in 0..5 {
+    //         if x != y {
+    //             *q1 = 1;
+    //             // Make sure that we correct load reference aliases through multiple blocks
+    //             if x != 20 {
+    //                 *q1 = 10;
+    //                 *q2 = 2; // now we'd expect q1 == q2 == 2
+                    
+    //                 assert(*q1 == 2);
+    //             }
+    //         }
+    //     }
+    //     if x != y {
+    //         *q1 = 1;
+    //         for _ in 0..5 {
+    //             // Make sure that we correct load reference aliases through multiple blocks
+    //             if x != 20 {
+    //                 *q1 = 10;
+    //                 *q2 = 2; // now we'd expect q1 == q2 == 2
+                    
+    //                 assert(*q1 == 2);
+    //             }
+    //         }
+    //     }
+    // }
+    // assert(*q1 == 2);  
+
+    // if x != y {
+    //     for _ in 0..5 {
+    //         if x != y {
+    //             *q1 = 1;
+    //             // Make sure that we correct load reference aliases through multiple blocks
+    //             if x != 20 {
+    //                 *q1 = 10;
+    //                 *q2 = 2; // now we'd expect q1 == q2 == 2
+
+    //                 assert(*q1 == 2);  
+    //             }
+    //         }
+    //     }
+    // } 
+    // assert(*q1 == 2);  
+}
+
+fn regression_2221(x: bool) {
+    let ref = &mut 0;
+    let mut array = [ref, ref];
+
+    if x {
+        *array[0] = 1;
+    } else {
+        *array[0] = 2;
+    }
+
+    assert(*array[0] == 1);
+    // dep::std::println(*array[0]);
+}
\ No newline at end of file
diff --git a/crates/nargo_cli/tests/execution_success/regression/src/main.nr b/crates/nargo_cli/tests/execution_success/regression/src/main.nr
index 54769c3970..103161482b 100644
--- a/crates/nargo_cli/tests/execution_success/regression/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/regression/src/main.nr
@@ -76,6 +76,7 @@ fn main(x: [u8; 5], z: Field)
     //Issue 1144
 	let (nib, len) = compact_decode(x,z);
 	assert(len == 5);
+	// dep::std::println(nib);
 	assert([nib[0], nib[1], nib[2], nib[3], nib[4]] == [15, 1, 12, 11, 8]);
 
 	// Issue 1169
diff --git a/crates/nargo_cli/tests/execution_success/sha2_blocks/src/main.nr b/crates/nargo_cli/tests/execution_success/sha2_blocks/src/main.nr
index fcdcdb8684..f1afea12da 100644
--- a/crates/nargo_cli/tests/execution_success/sha2_blocks/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/sha2_blocks/src/main.nr
@@ -3,20 +3,20 @@ use dep::std;
 
 fn main(x: [u8; 3], result256: [u8; 32], result512: [u8; 64])
 {
-	// One-block tests.
-	let mut digest256 = std::sha256::digest(x);
-	assert(digest256 == result256);
+	// // One-block tests.
+	// let mut digest256 = std::sha256::digest(x);
+	// assert(digest256 == result256);
 
-	let mut digest512 = std::sha512::digest(x);
-	assert(digest512 == result512);
+	// let mut digest512 = std::sha512::digest(x);
+	// assert(digest512 == result512);
 
-	// Two-block SHA256 test. Taken from https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/SHA256.pdf
-	let y: [u8; 56] = [97,98,99,100,98,99,100,101,99,100,101,102,100,101,102,103,101,102,103,104,102,103,104,105,103,104,105,106,104,105,106,107,105,106,107,108,106,107,108,109,107,108,109,110,108,109,110,111,109,110,111,112,110,111,112,113]; // "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"
-	digest256 = std::sha256::digest(y);
-	assert(digest256 == [36,141,106,97,210,6,56,184,229,192,38,147,12,62,96,57,163,60,228,89,100,255,33,103,246,236,237,212,25,219,6,193]);
+	// // Two-block SHA256 test. Taken from https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/SHA256.pdf
+	// let y: [u8; 56] = [97,98,99,100,98,99,100,101,99,100,101,102,100,101,102,103,101,102,103,104,102,103,104,105,103,104,105,106,104,105,106,107,105,106,107,108,106,107,108,109,107,108,109,110,108,109,110,111,109,110,111,112,110,111,112,113]; // "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"
+	// digest256 = std::sha256::digest(y);
+	// assert(digest256 == [36,141,106,97,210,6,56,184,229,192,38,147,12,62,96,57,163,60,228,89,100,255,33,103,246,236,237,212,25,219,6,193]);
 
-	// Two-block SHA256 test. Taken from https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/SHA512.pdf
-	let z: [u8; 112] = [97,98,99,100,101,102,103,104,98,99,100,101,102,103,104,105,99,100,101,102,103,104,105,106,100,101,102,103,104,105,106,107,101,102,103,104,105,106,107,108,102,103,104,105,106,107,108,109,103,104,105,106,107,108,109,110,104,105,106,107,108,109,110,111,105,106,107,108,109,110,111,112,106,107,108,109,110,111,112,113,107,108,109,110,111,112,113,114,108,109,110,111,112,113,114,115,109,110,111,112,113,114,115,116,110,111,112,113,114,115,116,117]; // "abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmnoijklmnopjklmnopqklmnopqrlmnopqrsmnopqrstnopqrstu"
-	digest512 = std::sha512::digest(z);
-	assert(digest512 == [142,149,155,117,218,227,19,218,140,244,247,40,20,252,20,63,143,119,121,198,235,159,127,161,114,153,174,173,182,136,144,24,80,29,40,158,73,0,247,228,51,27,153,222,196,181,67,58,199,211,41,238,182,221,38,84,94,150,229,91,135,75,233,9]);
+	// // Two-block SHA256 test. Taken from https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/SHA512.pdf
+	// let z: [u8; 112] = [97,98,99,100,101,102,103,104,98,99,100,101,102,103,104,105,99,100,101,102,103,104,105,106,100,101,102,103,104,105,106,107,101,102,103,104,105,106,107,108,102,103,104,105,106,107,108,109,103,104,105,106,107,108,109,110,104,105,106,107,108,109,110,111,105,106,107,108,109,110,111,112,106,107,108,109,110,111,112,113,107,108,109,110,111,112,113,114,108,109,110,111,112,113,114,115,109,110,111,112,113,114,115,116,110,111,112,113,114,115,116,117]; // "abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmnoijklmnopjklmnopqklmnopqrlmnopqrsmnopqrstnopqrstu"
+	// digest512 = std::sha512::digest(z);
+	// assert(digest512 == [142,149,155,117,218,227,19,218,140,244,247,40,20,252,20,63,143,119,121,198,235,159,127,161,114,153,174,173,182,136,144,24,80,29,40,158,73,0,247,228,51,27,153,222,196,181,67,58,199,211,41,238,182,221,38,84,94,150,229,91,135,75,233,9]);
 }
diff --git a/crates/noirc_evaluator/src/ssa.rs b/crates/noirc_evaluator/src/ssa.rs
index e5009607ee..6e67315d0a 100644
--- a/crates/noirc_evaluator/src/ssa.rs
+++ b/crates/noirc_evaluator/src/ssa.rs
@@ -56,6 +56,10 @@ pub(crate) fn optimize_into_acir(
             .print(print_ssa_passes, "After Unrolling:")
             .simplify_cfg()
             .print(print_ssa_passes, "After Simplifying:")
+            // Run mem2reg before flattening to handle any promotion
+            // of values that can be accessed after loop unrolling
+            .mem2reg()   
+            .print(print_ssa_passes, "After Mem2Reg:")
             .flatten_cfg()
             .print(print_ssa_passes, "After Flattening:")
             // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores
diff --git a/crates/noirc_evaluator/src/ssa/ir/cfg.rs b/crates/noirc_evaluator/src/ssa/ir/cfg.rs
index 8faa0fa856..26b473eb06 100644
--- a/crates/noirc_evaluator/src/ssa/ir/cfg.rs
+++ b/crates/noirc_evaluator/src/ssa/ir/cfg.rs
@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::{HashMap, BTreeSet};
 
 use super::{
     basic_block::{BasicBlock, BasicBlockId},
@@ -10,13 +10,14 @@ use super::{
 struct CfgNode {
     /// Set of blocks that containing jumps that target this block.
     /// The predecessor set has no meaningful order.
-    pub(crate) predecessors: HashSet<BasicBlockId>,
+    pub(crate) predecessors: BTreeSet<BasicBlockId>,
 
     /// Set of blocks that are the targets of jumps in this block.
     /// The successors set has no meaningful order.
-    pub(crate) successors: HashSet<BasicBlockId>,
+    pub(crate) successors: BTreeSet<BasicBlockId>,
 }
 
+#[derive(Clone)]
 /// The Control Flow Graph maintains a mapping of blocks to their predecessors
 /// and successors where predecessors are basic blocks and successors are
 /// basic blocks.
@@ -31,7 +32,7 @@ impl ControlFlowGraph {
         // therefore we must ensure that a node exists for the entry block, regardless of whether
         // it later comes to describe any edges after calling compute.
         let entry_block = func.entry_block();
-        let empty_node = CfgNode { predecessors: HashSet::new(), successors: HashSet::new() };
+        let empty_node = CfgNode { predecessors: BTreeSet::new(), successors: BTreeSet::new() };
         let data = HashMap::from([(entry_block, empty_node)]);
 
         let mut cfg = ControlFlowGraph { data };
diff --git a/crates/noirc_evaluator/src/ssa/ir/post_order.rs b/crates/noirc_evaluator/src/ssa/ir/post_order.rs
index e3bdbd491d..e405fc4604 100644
--- a/crates/noirc_evaluator/src/ssa/ir/post_order.rs
+++ b/crates/noirc_evaluator/src/ssa/ir/post_order.rs
@@ -13,6 +13,7 @@ enum Visit {
     Last,
 }
 
+#[derive(Clone)]
 pub(crate) struct PostOrder(Vec<BasicBlockId>);
 
 impl PostOrder {
diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 191ed8a0d9..9c82804725 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -17,6 +17,8 @@ use crate::ssa::{
     ssa_gen::Ssa,
 };
 
+use super::unrolling::{Loops, find_all_loops};
+
 impl Ssa {
     /// Attempts to remove any load instructions that recover values that are already available in
     /// scope, and attempts to remove stores that are subsequently redundant.
@@ -27,8 +29,22 @@ impl Ssa {
             let mut all_protected_allocations = HashSet::new();
 
             let mut context = PerFunctionContext::new(function);
-
-            for block in function.reachable_blocks() {
+            // let post_order_slice = .iter().rev();
+            let post_order = PostOrder::with_function(function);
+            let post_order_slice = post_order.as_slice();
+
+            for b in post_order_slice.iter().rev() {
+                println!("BLOCK: {b}");
+                let predecessors = context.cfg.predecessors(*b);
+                for p in predecessors {
+                    println!("predecessor: {p}");
+                }
+            }
+            dbg!(post_order_slice);
+            // TODO: for array_dynamic the normal post_order works, but for regression_2218 we need it reverse
+            let post_order_iter = post_order_slice.iter().rev();
+            // let post_order_iter = post_order_slice.iter();
+            for block in post_order_iter.clone() {
                 // Maps Load instruction id -> value to replace the result of the load with
                 let mut loads_to_substitute_per_block = BTreeMap::new();
 
@@ -40,16 +56,26 @@ impl Ssa {
                         &mut function.dfg,
                         &mut loads_to_substitute_per_block,
                         &mut load_values_to_substitute,
-                        block,
+                        *block,
                     );
+                // dbg!(all_protected_allocations.clone());
                 all_protected_allocations.extend(allocations_protected_by_block.into_iter());
+
+                // Broken strategy because the alias will not be recognized, just don't mem2reg compile time blocks
+                // let remaining_loads = context.find_remaining_loads(&mut function.dfg, *block);
+                // dbg!(remaining_loads.clone());
+                // let mut new_remaining_loads = HashSet::new();
+                // for load in remaining_loads.clone() {
+                //     let result_value = function.dfg.resolve(load);
+                //     new_remaining_loads.insert(result_value);
+                // }
+                // dbg!(new_remaining_loads.clone());
+                // all_protected_allocations.extend(remaining_loads.into_iter());
+                // all_protected_allocations.extend(new_remaining_loads.into_iter());
             }
 
-            // Now that we have a comprehensive list of used allocations across all the
-            // function's blocks, it is safe to remove any stores that do not touch such
-            // allocations.
-            for block in function.reachable_blocks() {
-                context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, block);
+            for block in post_order_iter {
+                context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, *block);
             }
         }
         dbg!("ended mem2reg");
@@ -64,7 +90,9 @@ struct PerFunctionContext {
     load_values_to_substitute_per_func: BTreeMap<ValueId, (ValueId, BasicBlockId)>,
     store_ids: Vec<InstructionId>,
     cfg: ControlFlowGraph,
+    post_order: PostOrder, 
     dom_tree: DominatorTree,
+    loops: Loops,
 }
 
 impl PerFunctionContext {
@@ -77,7 +105,9 @@ impl PerFunctionContext {
             load_values_to_substitute_per_func: BTreeMap::new(),
             store_ids: Vec::new(),
             cfg,
+            post_order,
             dom_tree,
+            loops: find_all_loops(function),
         }
     }
 }
@@ -99,18 +129,43 @@ impl PerFunctionContext {
     ) -> HashSet<AllocId> {
         let mut protected_allocations = HashSet::new();
         let block = &dfg[block_id];
+        // println!("BLOCK: {block_id}");
 
         for instruction_id in block.instructions() {
+            // dbg!(&dfg[*instruction_id]);
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
+                    println!("STORE: {address}");
                     address = self.fetch_load_value_to_substitute(block_id, address);
+                    println!("address after: {address}");
 
+                    // println!("about to insert store: {address}, block_id: {block_id}, value: {value}");
                     self.last_stores_with_block.insert((address, block_id), *value);
                     self.store_ids.push(*instruction_id);
                 }
                 Instruction::Load { mut address } => {
-                    address = self.fetch_load_value_to_substitute(block_id, address);
+                    // dbg!(load_values_to_substitute_per_block.get(&address).is_some());
 
+                    println!("LOAD: {address}");
+                    address = self.fetch_load_value_to_substitute(block_id, address);
+                    println!("address after: {address}");
+
+                    let predecessors = self.cfg.predecessors(block_id);
+                    let p_len = predecessors.len();
+                    dbg!(p_len);
+                    drop(predecessors);
+
+                    // let mut found_last_value = false;
+                    // if p_len != 2 {
+                    //     found_last_value = self.find_load_to_substitute(
+                    //             block_id,
+                    //             address,
+                    //             dfg,
+                    //             instruction_id,
+                    //             loads_to_substitute,
+                    //             load_values_to_substitute_per_block,
+                    //     );
+                    // }
                     let found_last_value = self.find_load_to_substitute(
                         block_id,
                         address,
@@ -119,8 +174,51 @@ impl PerFunctionContext {
                         loads_to_substitute,
                         load_values_to_substitute_per_block,
                     );
+                    dbg!(found_last_value);
                     if !found_last_value {
+                        // We want to protect allocations that do not have a load to substitute 
+                        dbg!(address);
+                        let resolved_address = dfg.resolve(address);
+                        dbg!(resolved_address);
                         protected_allocations.insert(address);
+
+                        // Need this to solve regression 2218 with loops
+                        let found_last_value = self.find_load_to_substitute(
+                            block_id,
+                            resolved_address,
+                            dfg,
+                            instruction_id,
+                            loads_to_substitute,
+                            load_values_to_substitute_per_block,
+                        );
+                        dbg!(found_last_value);
+                        if found_last_value {
+                            protected_allocations.insert(resolved_address);
+                        }
+
+                        // We also want to check for allocations that share a value we are substituting
+                        // with the one we are protecting. 
+                        // This check prevents the pass from removing stores to reference aliases across
+                        // multiple blocks. 
+                        // TODO: this feels like a cop-out where I am restricting
+                        // functionality so the pass has to be run again rather than checking stores across blocks
+                        // but it may be needed that we can only do these removals after flattening
+                        // NOTE: not the cause of the extra allocate in regression test
+                        if let Some((value, outer_block)) =
+                            self.load_values_to_substitute_per_func.get(&address)
+                        {
+                            println!("value: {value}");
+                            for (address, (inner_value, block)) in &self.load_values_to_substitute_per_func {
+                                println!("inner_value: {inner_value}");
+                                println!("address: {address}");
+                                dbg!(*inner_value == *value);
+                                dbg!(*outer_block == *block);
+                                if *inner_value == *value && *outer_block == *block {
+                                    dbg!(address);
+                                    protected_allocations.insert(*address);
+                                }
+                            }
+                        }
                     }
                 }
                 Instruction::Call { arguments, .. } => {
@@ -161,7 +259,25 @@ impl PerFunctionContext {
         protected_allocations
     }
 
-    // This method finds the load values to substitute for a given address
+    fn find_remaining_loads(
+        &mut self,
+        dfg: &mut DataFlowGraph,
+        block_id: BasicBlockId,
+    ) -> HashSet<AllocId> {
+        let mut protected_allocations = HashSet::new();
+        let block = &dfg[block_id];
+
+        for instruction_id in block.instructions() {
+            if let Instruction::Load { address } = &dfg[*instruction_id] {
+                protected_allocations.insert(*address);
+
+            }
+        }
+
+        protected_allocations
+    }
+
+    // This method will fetch already saved load values to substitute for a given address.
     // The search starts at the block supplied as a parameter. If there is not a load to substitute
     // the CFG is analyzed to determine whether a predecessor block has a load value to substitute.
     // If there is no load value to substitute the original address is returned.
@@ -176,6 +292,22 @@ impl PerFunctionContext {
         while let Some(block) = stack.pop() {
             visited.insert(block);
 
+            // TODO: check that any of the predecssor blocks are loops
+            for l in self.loops.yet_to_unroll.iter() {
+                // We do not want to substitute loads that take place within loops as this pass
+                // can occur before loop unrolling
+                // The pass should be ran again following loop unrolling as new values
+                if l.blocks.contains(&block) {
+                    return address;
+                }
+            }
+
+            // let predecessors = self.cfg.predecessors(block);
+            // dbg!(predecessors.len());
+            // if predecessors.len() == 2 {
+            //     return address;
+            // }
+
             // Check whether there is a load value to substitute in the current block.
             // Return the value if found.
             if let Some((value, load_block_id)) =
@@ -189,11 +321,27 @@ impl PerFunctionContext {
             if let Some(predecessor) = self.block_has_predecessor(block, &visited) {
                 stack.push(predecessor);
             }
+
+            // If no load values to substitute have been found in the current block, check the block's predecessors.
+            // let predecessors = self.cfg.predecessors(block);
+            // for predecessor in predecessors {
+            //     // if self.dom_tree.is_reachable(predecessor)
+            //     // && self.dom_tree.dominates(predecessor, block)
+            //     // && !visited.contains(&predecessor) {
+            //     //     stack.push(predecessor);
+            //     // }
+            //     if self.dom_tree.is_reachable(predecessor)
+            //         && !visited.contains(&predecessor)
+            //     {
+            //         stack.push(predecessor);
+            //     }
+            // }
         }
         address
     }
 
-    // This method determines which loads should be substituted.
+    // This method determines which loads should be substituted and saves them 
+    // to be substituted further in the pass.
     // Starting at the block supplied as a parameter, we check whether a store has occurred with the given address.
     // If no store has occurred in the supplied block, the CFG is analyzed to determine whether
     // a predecessor block has a store at the given address.
@@ -210,25 +358,74 @@ impl PerFunctionContext {
         let mut visited = HashSet::new();
 
         while let Some(block) = stack.pop() {
+            println!("BLOCK in find_load_to_substitute: {block}");
+            // let predecessors = self.cfg.predecessors(block);
+            // for predecessor in predecessors {
+            //     println!("predecessor: {predecessor}");
+            // }
             visited.insert(block);
 
+            for l in self.loops.yet_to_unroll.iter() {
+                // We do not want to substitute loads that take place within loops as this pass
+                // can occur before loop unrolling
+                // The pass should be ran again following loop unrolling as new values
+                // dbg!(l.blocks.clone());
+                if l.blocks.contains(&block) {
+                    dbg!(block);
+                    return false;
+                }
+            }
+
+            // let predecessors = self.cfg.clone().predecessors(block_id);
+            // dbg!(predecessors.len());
+            // if predecessors.len() == 2 {
+            //     return false;
+            // }
+
             // Check whether there has been a store instruction in the current block
             // If there has been a store, add a load to be substituted.
             if let Some(last_value) = self.last_stores_with_block.get(&(address, block)) {
+                // println!("last_value: {last_value}");
                 let result_value = *dfg
                     .instruction_results(*instruction_id)
                     .first()
                     .expect("ICE: Load instructions should have single result");
 
+                // println!("result_value: {result_value}");
                 loads_to_substitute.insert(*instruction_id, *last_value);
                 load_values_to_substitute_per_block.insert(result_value, *last_value);
                 self.load_values_to_substitute_per_func.insert(result_value, (*last_value, block));
                 return true;
             }
 
+            // TODO: some situations we only need to check block with one predecessor and in some need to check more
             if let Some(predecessor) = self.block_has_predecessor(block, &visited) {
                 stack.push(predecessor);
             }
+
+            // If no load values to substitute have been found in the current block, check the block's predecessors.
+            // let predecessors = self.cfg.predecessors(block);
+            // // dbg!(predecessors.len());
+            // for predecessor in predecessors {
+            //     // dbg!(predecessor);
+            //     // check out the post order here see if there is a match
+            //     // perhaps it is not dominating as it doesn't have to flow through
+            //     // dbg!(self.dom_tree.dominates(predecessor, block));
+
+            //     // Need this for array_dynamic
+            //     // if self.dom_tree.is_reachable(predecessor)
+            //     // && self.dom_tree.dominates(predecessor, block)
+            //     // && !visited.contains(&predecessor) {
+            //     //     stack.push(predecessor);
+            //     // }
+            //     // TODO: references regression 2218 with loops working with this and blocks constructed from successors
+            //     // only a few tests failing after that
+            //     if self.dom_tree.is_reachable(predecessor)
+            //         && !visited.contains(&predecessor)
+            //     {
+            //         stack.push(predecessor);
+            //     }
+            // }
         }
         false
     }
@@ -243,6 +440,7 @@ impl PerFunctionContext {
         visited: &HashSet<BasicBlockId>,
     ) -> Option<BasicBlockId> {
         let mut predecessors = self.cfg.predecessors(block_id);
+        // dbg!(predecessors.len());
         if predecessors.len() == 1 {
             let predecessor = predecessors.next().unwrap();
             if self.dom_tree.is_reachable(predecessor)
@@ -251,6 +449,11 @@ impl PerFunctionContext {
             {
                 return Some(predecessor);
             }
+            // if self.dom_tree.is_reachable(predecessor)
+            // && !visited.contains(&predecessor)
+            // {
+            //     return Some(predecessor);
+            // }
         }
         None
     }
diff --git a/crates/noirc_evaluator/src/ssa/opt/unrolling.rs b/crates/noirc_evaluator/src/ssa/opt/unrolling.rs
index 20df50d578..def9d2fa20 100644
--- a/crates/noirc_evaluator/src/ssa/opt/unrolling.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/unrolling.rs
@@ -49,14 +49,14 @@ impl Ssa {
 pub(crate) struct Loop {
     /// The header block of a loop is the block which dominates all the
     /// other blocks in the loop.
-    pub(crate) header: BasicBlockId,
+    header: BasicBlockId,
 
     /// The start of the back_edge n -> d is the block n at the end of
     /// the loop that jumps back to the header block d which restarts the loop.
     back_edge_start: BasicBlockId,
 
     /// All the blocks contained within the loop, including `header` and `back_edge_start`.
-    blocks: HashSet<BasicBlockId>,
+    pub(crate) blocks: HashSet<BasicBlockId>,
 }
 
 pub(crate) struct Loops {

From 265b88381b088110ba156081070180241e2536b9 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 14:36:40 +0000
Subject: [PATCH 20/28] working func mem2reg for various reference alias cases

---
 .../execution_success/references/src/main.nr  | 232 ++++++++--------
 crates/noirc_evaluator/src/ssa.rs             |   2 +-
 crates/noirc_evaluator/src/ssa/ir/cfg.rs      |   2 +-
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 254 +++++-------------
 4 files changed, 187 insertions(+), 303 deletions(-)

diff --git a/crates/nargo_cli/tests/execution_success/references/src/main.nr b/crates/nargo_cli/tests/execution_success/references/src/main.nr
index e8ee19e33f..f5ac47c65a 100644
--- a/crates/nargo_cli/tests/execution_success/references/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/references/src/main.nr
@@ -1,48 +1,46 @@
 fn main(mut x: Field) {
-    // add1(&mut x);
-    // assert(x == 3);
-
-    // let mut s = S { y: x };
-    // s.add2();
-    // assert(s.y == 5);
-
-    // // Regression for #1946: Method resolution error when calling &mut methods with a variable of type &mut T
-    // let s_ref = &mut s;
-    // s_ref.add2();
-    // assert(s.y == 7);
-
-    // // Test that normal mutable variables are still copied
-    // let mut a = 0;
-    // mutate_copy(a);
-    // assert(a == 0);
-
-    // // Test something 3 allocations deep
-    // let mut nested_allocations = Nested { y: &mut &mut 0 };
-    // add1(*nested_allocations.y);
-    // assert(**nested_allocations.y == 1);
-
-    // // Test nested struct allocations with a mutable reference to an array.
-    // let mut c = C {
-    //     foo: 0,
-    //     bar: &mut C2 {
-    //         array: &mut [1, 2],
-    //     },
-    // };
-    // *c.bar.array = [3, 4];
-    // assert(*c.bar.array == [3, 4]);
-
-    // regression_1887();
-    // regression_2054();
-    // regression_2030();
-
-
-    // Compile time works
-    // regression_2218(5, 10);
-    // This breaks, thus we should fix our mem2reg here
-    // regression_2218(x, 10);
-    regression_2218_old(x, 10);
-    // regression_2218_loop(x, 10);
-    // regression_2221(true);
+    add1(&mut x);
+    assert(x == 3);
+
+    let mut s = S { y: x };
+    s.add2();
+    assert(s.y == 5);
+
+    // Regression for #1946: Method resolution error when calling &mut methods with a variable of type &mut T
+    let s_ref = &mut s;
+    s_ref.add2();
+    assert(s.y == 7);
+
+    // Test that normal mutable variables are still copied
+    let mut a = 0;
+    mutate_copy(a);
+    assert(a == 0);
+
+    // Test something 3 allocations deep
+    let mut nested_allocations = Nested { y: &mut &mut 0 };
+    add1(*nested_allocations.y);
+    assert(**nested_allocations.y == 1);
+
+    // Test nested struct allocations with a mutable reference to an array.
+    let mut c = C {
+        foo: 0,
+        bar: &mut C2 {
+            array: &mut [1, 2],
+        },
+    };
+    *c.bar.array = [3, 4];
+    assert(*c.bar.array == [3, 4]);
+
+    regression_1887();
+    regression_2054();
+    regression_2030();
+
+    // x == 3 at this point
+    regression_2218_if_inner_if(x, 10);
+    regression_2218_if_inner_else(20, x);
+    regression_2218_else(x, 3);
+
+    regression_2218_loop(x, 10);
 }
 
 fn add1(x: &mut Field) {
@@ -107,7 +105,7 @@ fn regression_2030() {
     *array[0] = 1;
 }
 
-fn regression_2218(x: Field, y: Field) {
+fn regression_2218(x: Field, y: Field) -> Field {
     let q = &mut &mut 0;
     let q1 = *q;
     let q2 = *q;
@@ -118,39 +116,34 @@ fn regression_2218(x: Field, y: Field) {
         if x != 20 {
             *q1 = 10;
             *q2 = 2; // now we'd expect q1 == q2 == 2
-            // dep::std::println(*q1);
+
             assert(*q1 == 2);  
         } else {
             *q2 = 15;
             assert(*q1 == 15);
         }
-    }
-    // TODO: this breaks mem2reg per func 
-    else {
+    } else {
         *q2 = 20;
         assert(*q1 == 20);  
     }
-    // dep::std::println(*q1);
-    assert(*q1 == 2);  
+    // Have to assign value to return it
+    let value = *q1;
+    value
 }
 
-fn regression_2218_old(x: Field, y: Field) {
-    let q = &mut &mut 0;
-    let q1 = *q;
-    let q2 = *q;
+fn regression_2218_if_inner_if(x: Field, y: Field) {
+    let value = regression_2218(x, y);
+    assert(value == 2);
+}
 
-    if x != y {
-        *q1 = 1;
-        // Make sure that we correct load reference aliases through multiple blocks
-        if x != 20 {
-            *q1 = 10;
-            *q2 = 2; // now we'd expect q1 == q2 == 2
-            // dep::std::println(*q1);
-            assert(*q1 == 2); 
-        }
-    }
-    // dep::std::println(*q1);
-    assert(*q1 == 2);  
+fn regression_2218_if_inner_else(x: Field, y: Field) {
+    let value = regression_2218(x, y);
+    assert(value == 15);
+}
+
+fn regression_2218_else(x: Field, y: Field) {
+    let value = regression_2218(x, y);
+    assert(value == 20);  
 }
 
 fn regression_2218_loop(x: Field, y: Field) {
@@ -164,66 +157,63 @@ fn regression_2218_loop(x: Field, y: Field) {
             *q2 = 2; // now we'd expect q1 == q2 == 2
             
             assert(*q1 == 2);
-        }  
+        } else {
+            *q2 = 20;
+            assert(*q1 == 20);  
+        } 
     }
-    // dep::std::println(*q1);
     assert(*q1 == 2);
 
-    // for _ in 0..1 {
-    //     for _ in 0..5 {
-    //         if x != y {
-    //             *q1 = 1;
-    //             // Make sure that we correct load reference aliases through multiple blocks
-    //             if x != 20 {
-    //                 *q1 = 10;
-    //                 *q2 = 2; // now we'd expect q1 == q2 == 2
+    for _ in 0..1 {
+        for _ in 0..5 {
+            if x != y {
+                *q1 = 1;
+                // Make sure that we correct load reference aliases through multiple blocks
+                if x != 20 {
+                    *q1 = 10;
+                    *q2 = 2; // now we'd expect q1 == q2 == 2
                     
-    //                 assert(*q1 == 2);
-    //             }
-    //         }
-    //     }
-    //     if x != y {
-    //         *q1 = 1;
-    //         for _ in 0..5 {
-    //             // Make sure that we correct load reference aliases through multiple blocks
-    //             if x != 20 {
-    //                 *q1 = 10;
-    //                 *q2 = 2; // now we'd expect q1 == q2 == 2
+                    assert(*q1 == 2);
+                }
+            } else {
+                *q2 = 20;
+                assert(*q1 == 20);
+            }
+        }
+        if x != y {
+            *q1 = 1;
+            for _ in 0..5 {
+                // Make sure that we correct load reference aliases through multiple blocks
+                if x != 20 {
+                    *q1 = 10;
+                    *q2 = 2; // now we'd expect q1 == q2 == 2
                     
-    //                 assert(*q1 == 2);
-    //             }
-    //         }
-    //     }
-    // }
-    // assert(*q1 == 2);  
-
-    // if x != y {
-    //     for _ in 0..5 {
-    //         if x != y {
-    //             *q1 = 1;
-    //             // Make sure that we correct load reference aliases through multiple blocks
-    //             if x != 20 {
-    //                 *q1 = 10;
-    //                 *q2 = 2; // now we'd expect q1 == q2 == 2
-
-    //                 assert(*q1 == 2);  
-    //             }
-    //         }
-    //     }
-    // } 
-    // assert(*q1 == 2);  
-}
-
-fn regression_2221(x: bool) {
-    let ref = &mut 0;
-    let mut array = [ref, ref];
+                    assert(*q1 == 2);
+                }
+            }
+        } else {
+            *q2 = 20;
+            assert(*q1 == 20);  
+        }
+    }
+    assert(*q1 == 2);  
 
-    if x {
-        *array[0] = 1;
+    if x != y {
+        for _ in 0..5 {
+            if x != y {
+                *q1 = 1;
+                // Make sure that we correct load reference aliases through multiple blocks
+                if x != 20 {
+                    *q1 = 10;
+                    *q2 = 2; // now we'd expect q1 == q2 == 2
+
+                    assert(*q1 == 2);  
+                }
+            }
+        }
     } else {
-        *array[0] = 2;
+        *q2 = 20;
+        assert(*q1 == 20);  
     }
-
-    assert(*array[0] == 1);
-    // dep::std::println(*array[0]);
+    assert(*q1 == 2);  
 }
\ No newline at end of file
diff --git a/crates/noirc_evaluator/src/ssa.rs b/crates/noirc_evaluator/src/ssa.rs
index 6e67315d0a..a2e46265f9 100644
--- a/crates/noirc_evaluator/src/ssa.rs
+++ b/crates/noirc_evaluator/src/ssa.rs
@@ -58,7 +58,7 @@ pub(crate) fn optimize_into_acir(
             .print(print_ssa_passes, "After Simplifying:")
             // Run mem2reg before flattening to handle any promotion
             // of values that can be accessed after loop unrolling
-            .mem2reg()   
+            .mem2reg()
             .print(print_ssa_passes, "After Mem2Reg:")
             .flatten_cfg()
             .print(print_ssa_passes, "After Flattening:")
diff --git a/crates/noirc_evaluator/src/ssa/ir/cfg.rs b/crates/noirc_evaluator/src/ssa/ir/cfg.rs
index 26b473eb06..62c0833589 100644
--- a/crates/noirc_evaluator/src/ssa/ir/cfg.rs
+++ b/crates/noirc_evaluator/src/ssa/ir/cfg.rs
@@ -1,4 +1,4 @@
-use std::collections::{HashMap, BTreeSet};
+use std::collections::{BTreeSet, HashMap};
 
 use super::{
     basic_block::{BasicBlock, BasicBlockId},
diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 9c82804725..8c07c9ed29 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -17,14 +17,11 @@ use crate::ssa::{
     ssa_gen::Ssa,
 };
 
-use super::unrolling::{Loops, find_all_loops};
-
 impl Ssa {
     /// Attempts to remove any load instructions that recover values that are already available in
     /// scope, and attempts to remove stores that are subsequently redundant.
     /// As long as they are not stores on memory used inside of loops
     pub(crate) fn mem2reg(mut self) -> Ssa {
-        dbg!("start mem2reg");
         for function in self.functions.values_mut() {
             let mut all_protected_allocations = HashSet::new();
 
@@ -33,14 +30,6 @@ impl Ssa {
             let post_order = PostOrder::with_function(function);
             let post_order_slice = post_order.as_slice();
 
-            for b in post_order_slice.iter().rev() {
-                println!("BLOCK: {b}");
-                let predecessors = context.cfg.predecessors(*b);
-                for p in predecessors {
-                    println!("predecessor: {p}");
-                }
-            }
-            dbg!(post_order_slice);
             // TODO: for array_dynamic the normal post_order works, but for regression_2218 we need it reverse
             let post_order_iter = post_order_slice.iter().rev();
             // let post_order_iter = post_order_slice.iter();
@@ -58,27 +47,13 @@ impl Ssa {
                         &mut load_values_to_substitute,
                         *block,
                     );
-                // dbg!(all_protected_allocations.clone());
                 all_protected_allocations.extend(allocations_protected_by_block.into_iter());
-
-                // Broken strategy because the alias will not be recognized, just don't mem2reg compile time blocks
-                // let remaining_loads = context.find_remaining_loads(&mut function.dfg, *block);
-                // dbg!(remaining_loads.clone());
-                // let mut new_remaining_loads = HashSet::new();
-                // for load in remaining_loads.clone() {
-                //     let result_value = function.dfg.resolve(load);
-                //     new_remaining_loads.insert(result_value);
-                // }
-                // dbg!(new_remaining_loads.clone());
-                // all_protected_allocations.extend(remaining_loads.into_iter());
-                // all_protected_allocations.extend(new_remaining_loads.into_iter());
             }
 
             for block in post_order_iter {
                 context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, *block);
             }
         }
-        dbg!("ended mem2reg");
         self
     }
 }
@@ -90,9 +65,11 @@ struct PerFunctionContext {
     load_values_to_substitute_per_func: BTreeMap<ValueId, (ValueId, BasicBlockId)>,
     store_ids: Vec<InstructionId>,
     cfg: ControlFlowGraph,
-    post_order: PostOrder, 
+    post_order: PostOrder,
     dom_tree: DominatorTree,
-    loops: Loops,
+    // Maps block id -> bool stating whether the block shares a common successor
+    // with one of its predecessors
+    has_common_successor: BTreeMap<BasicBlockId, bool>,
 }
 
 impl PerFunctionContext {
@@ -107,7 +84,7 @@ impl PerFunctionContext {
             cfg,
             post_order,
             dom_tree,
-            loops: find_all_loops(function),
+            has_common_successor: BTreeMap::new(),
         }
     }
 }
@@ -129,43 +106,27 @@ impl PerFunctionContext {
     ) -> HashSet<AllocId> {
         let mut protected_allocations = HashSet::new();
         let block = &dfg[block_id];
-        // println!("BLOCK: {block_id}");
+
+        let has_common_successor = self.has_common_successor(block_id);
+        // Maintain a map for whether a block has a common successor to avoid
+        // analyzing the CFG for successors on every store or load
+        self.has_common_successor.insert(block_id, has_common_successor);
 
         for instruction_id in block.instructions() {
-            // dbg!(&dfg[*instruction_id]);
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
-                    println!("STORE: {address}");
                     address = self.fetch_load_value_to_substitute(block_id, address);
-                    println!("address after: {address}");
 
-                    // println!("about to insert store: {address}, block_id: {block_id}, value: {value}");
                     self.last_stores_with_block.insert((address, block_id), *value);
                     self.store_ids.push(*instruction_id);
+
+                    if has_common_successor {
+                        protected_allocations.insert(address);
+                    }
                 }
                 Instruction::Load { mut address } => {
-                    // dbg!(load_values_to_substitute_per_block.get(&address).is_some());
-
-                    println!("LOAD: {address}");
                     address = self.fetch_load_value_to_substitute(block_id, address);
-                    println!("address after: {address}");
-
-                    let predecessors = self.cfg.predecessors(block_id);
-                    let p_len = predecessors.len();
-                    dbg!(p_len);
-                    drop(predecessors);
-
-                    // let mut found_last_value = false;
-                    // if p_len != 2 {
-                    //     found_last_value = self.find_load_to_substitute(
-                    //             block_id,
-                    //             address,
-                    //             dfg,
-                    //             instruction_id,
-                    //             loads_to_substitute,
-                    //             load_values_to_substitute_per_block,
-                    //     );
-                    // }
+
                     let found_last_value = self.find_load_to_substitute(
                         block_id,
                         address,
@@ -174,51 +135,18 @@ impl PerFunctionContext {
                         loads_to_substitute,
                         load_values_to_substitute_per_block,
                     );
-                    dbg!(found_last_value);
                     if !found_last_value {
-                        // We want to protect allocations that do not have a load to substitute 
-                        dbg!(address);
-                        let resolved_address = dfg.resolve(address);
-                        dbg!(resolved_address);
+                        // We want to protect allocations that do not have a load to substitute
                         protected_allocations.insert(address);
+                        // We also want to check for allocations that share a value
+                        // with the one we are protecting.
+                        // This check prevents the pass from removing stores to a value that
+                        // is used by reference aliases in different blocks
+                        protected_allocations.insert(dfg.resolve(address));
+                    }
 
-                        // Need this to solve regression 2218 with loops
-                        let found_last_value = self.find_load_to_substitute(
-                            block_id,
-                            resolved_address,
-                            dfg,
-                            instruction_id,
-                            loads_to_substitute,
-                            load_values_to_substitute_per_block,
-                        );
-                        dbg!(found_last_value);
-                        if found_last_value {
-                            protected_allocations.insert(resolved_address);
-                        }
-
-                        // We also want to check for allocations that share a value we are substituting
-                        // with the one we are protecting. 
-                        // This check prevents the pass from removing stores to reference aliases across
-                        // multiple blocks. 
-                        // TODO: this feels like a cop-out where I am restricting
-                        // functionality so the pass has to be run again rather than checking stores across blocks
-                        // but it may be needed that we can only do these removals after flattening
-                        // NOTE: not the cause of the extra allocate in regression test
-                        if let Some((value, outer_block)) =
-                            self.load_values_to_substitute_per_func.get(&address)
-                        {
-                            println!("value: {value}");
-                            for (address, (inner_value, block)) in &self.load_values_to_substitute_per_func {
-                                println!("inner_value: {inner_value}");
-                                println!("address: {address}");
-                                dbg!(*inner_value == *value);
-                                dbg!(*outer_block == *block);
-                                if *inner_value == *value && *outer_block == *block {
-                                    dbg!(address);
-                                    protected_allocations.insert(*address);
-                                }
-                            }
-                        }
+                    if has_common_successor {
+                        protected_allocations.insert(address);
                     }
                 }
                 Instruction::Call { arguments, .. } => {
@@ -270,7 +198,6 @@ impl PerFunctionContext {
         for instruction_id in block.instructions() {
             if let Instruction::Load { address } = &dfg[*instruction_id] {
                 protected_allocations.insert(*address);
-
             }
         }
 
@@ -292,22 +219,14 @@ impl PerFunctionContext {
         while let Some(block) = stack.pop() {
             visited.insert(block);
 
-            // TODO: check that any of the predecssor blocks are loops
-            for l in self.loops.yet_to_unroll.iter() {
-                // We do not want to substitute loads that take place within loops as this pass
-                // can occur before loop unrolling
-                // The pass should be ran again following loop unrolling as new values
-                if l.blocks.contains(&block) {
-                    return address;
-                }
+            // We do not want to substitute loads that take place within loops or yet to be simplified branches
+            // as this pass can occur before loop unrolling and flattening.
+            // The mem2reg pass should be ran again following all optimization passes as new values
+            // may be able to be promoted
+            if self.has_common_successor[&block_id] {
+                return address;
             }
 
-            // let predecessors = self.cfg.predecessors(block);
-            // dbg!(predecessors.len());
-            // if predecessors.len() == 2 {
-            //     return address;
-            // }
-
             // Check whether there is a load value to substitute in the current block.
             // Return the value if found.
             if let Some((value, load_block_id)) =
@@ -318,29 +237,15 @@ impl PerFunctionContext {
                 }
             }
 
+            // If no load values to substitute have been found in the current block, check the block's predecessors.
             if let Some(predecessor) = self.block_has_predecessor(block, &visited) {
                 stack.push(predecessor);
             }
-
-            // If no load values to substitute have been found in the current block, check the block's predecessors.
-            // let predecessors = self.cfg.predecessors(block);
-            // for predecessor in predecessors {
-            //     // if self.dom_tree.is_reachable(predecessor)
-            //     // && self.dom_tree.dominates(predecessor, block)
-            //     // && !visited.contains(&predecessor) {
-            //     //     stack.push(predecessor);
-            //     // }
-            //     if self.dom_tree.is_reachable(predecessor)
-            //         && !visited.contains(&predecessor)
-            //     {
-            //         stack.push(predecessor);
-            //     }
-            // }
         }
         address
     }
 
-    // This method determines which loads should be substituted and saves them 
+    // This method determines which loads should be substituted and saves them
     // to be substituted further in the pass.
     // Starting at the block supplied as a parameter, we check whether a store has occurred with the given address.
     // If no store has occurred in the supplied block, the CFG is analyzed to determine whether
@@ -358,74 +263,34 @@ impl PerFunctionContext {
         let mut visited = HashSet::new();
 
         while let Some(block) = stack.pop() {
-            println!("BLOCK in find_load_to_substitute: {block}");
-            // let predecessors = self.cfg.predecessors(block);
-            // for predecessor in predecessors {
-            //     println!("predecessor: {predecessor}");
-            // }
             visited.insert(block);
 
-            for l in self.loops.yet_to_unroll.iter() {
-                // We do not want to substitute loads that take place within loops as this pass
-                // can occur before loop unrolling
-                // The pass should be ran again following loop unrolling as new values
-                // dbg!(l.blocks.clone());
-                if l.blocks.contains(&block) {
-                    dbg!(block);
-                    return false;
-                }
+            // We do not want to substitute loads that take place within loops or yet to be simplified branches
+            // as this pass can occur before loop unrolling and flattening.
+            // The mem2reg pass should be ran again following all optimization passes as new values
+            // may be able to be promoted
+            if self.has_common_successor[&block_id] {
+                return false;
             }
 
-            // let predecessors = self.cfg.clone().predecessors(block_id);
-            // dbg!(predecessors.len());
-            // if predecessors.len() == 2 {
-            //     return false;
-            // }
-
             // Check whether there has been a store instruction in the current block
             // If there has been a store, add a load to be substituted.
             if let Some(last_value) = self.last_stores_with_block.get(&(address, block)) {
-                // println!("last_value: {last_value}");
                 let result_value = *dfg
                     .instruction_results(*instruction_id)
                     .first()
                     .expect("ICE: Load instructions should have single result");
 
-                // println!("result_value: {result_value}");
                 loads_to_substitute.insert(*instruction_id, *last_value);
                 load_values_to_substitute_per_block.insert(result_value, *last_value);
                 self.load_values_to_substitute_per_func.insert(result_value, (*last_value, block));
                 return true;
             }
 
-            // TODO: some situations we only need to check block with one predecessor and in some need to check more
+            // If no load values to substitute have been found in the current block, check the block's predecessors.
             if let Some(predecessor) = self.block_has_predecessor(block, &visited) {
                 stack.push(predecessor);
             }
-
-            // If no load values to substitute have been found in the current block, check the block's predecessors.
-            // let predecessors = self.cfg.predecessors(block);
-            // // dbg!(predecessors.len());
-            // for predecessor in predecessors {
-            //     // dbg!(predecessor);
-            //     // check out the post order here see if there is a match
-            //     // perhaps it is not dominating as it doesn't have to flow through
-            //     // dbg!(self.dom_tree.dominates(predecessor, block));
-
-            //     // Need this for array_dynamic
-            //     // if self.dom_tree.is_reachable(predecessor)
-            //     // && self.dom_tree.dominates(predecessor, block)
-            //     // && !visited.contains(&predecessor) {
-            //     //     stack.push(predecessor);
-            //     // }
-            //     // TODO: references regression 2218 with loops working with this and blocks constructed from successors
-            //     // only a few tests failing after that
-            //     if self.dom_tree.is_reachable(predecessor)
-            //         && !visited.contains(&predecessor)
-            //     {
-            //         stack.push(predecessor);
-            //     }
-            // }
         }
         false
     }
@@ -440,7 +305,6 @@ impl PerFunctionContext {
         visited: &HashSet<BasicBlockId>,
     ) -> Option<BasicBlockId> {
         let mut predecessors = self.cfg.predecessors(block_id);
-        // dbg!(predecessors.len());
         if predecessors.len() == 1 {
             let predecessor = predecessors.next().unwrap();
             if self.dom_tree.is_reachable(predecessor)
@@ -449,15 +313,45 @@ impl PerFunctionContext {
             {
                 return Some(predecessor);
             }
-            // if self.dom_tree.is_reachable(predecessor)
-            // && !visited.contains(&predecessor)
-            // {
-            //     return Some(predecessor);
-            // }
         }
         None
     }
 
+    fn has_common_successor(&mut self, block_id: BasicBlockId) -> bool {
+        let mut predecessors = self.cfg.predecessors(block_id);
+        if let Some(predecessor) = predecessors.next() {
+            let pred_successors = self.find_all_successors(predecessor);
+            let current_successors: HashSet<_> = self.cfg.successors(block_id).collect();
+            return pred_successors.into_iter().any(|b| current_successors.contains(&b));
+        }
+        false
+    }
+
+    fn find_all_successors(&self, block_id: BasicBlockId) -> HashSet<BasicBlockId> {
+        let mut stack = vec![];
+        let mut visited = HashSet::new();
+
+        // Fetch initial block successors
+        let successors = self.cfg.successors(block_id);
+        for successor in successors {
+            if !visited.contains(&successor) {
+                stack.push(successor);
+            }
+        }
+
+        // Follow the CFG to fetch the remaining successors
+        while let Some(block) = stack.pop() {
+            visited.insert(block);
+            let successors = self.cfg.successors(block);
+            for successor in successors {
+                if !visited.contains(&successor) {
+                    stack.push(successor);
+                }
+            }
+        }
+        visited
+    }
+
     /// Checks whether the given value id refers to an allocation.
     fn value_is_from_allocation(value: ValueId, dfg: &DataFlowGraph) -> bool {
         match &dfg[value] {

From 3906ca248466ee29bafc49dcfeeafe0bcbc4dfa5 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 14:37:43 +0000
Subject: [PATCH 21/28] array dynamic back to original

---
 .../array_dynamic/Prover.toml                 |  3 +-
 .../array_dynamic/src/main.nr                 | 36 ++++++++-----------
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/crates/nargo_cli/tests/execution_success/array_dynamic/Prover.toml b/crates/nargo_cli/tests/execution_success/array_dynamic/Prover.toml
index 957a044a1a..750b3129ec 100644
--- a/crates/nargo_cli/tests/execution_success/array_dynamic/Prover.toml
+++ b/crates/nargo_cli/tests/execution_success/array_dynamic/Prover.toml
@@ -1,6 +1,5 @@
 x = [104, 101, 108, 108, 111]
-# z = "59"
-z = "4"
+z = "59"
 t = "10"
 index = [0,1,2,3,4]
 index2 =  [0,1,2,3,4]
diff --git a/crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr b/crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr
index d9411bfb5f..69e5fdc93d 100644
--- a/crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr
@@ -1,38 +1,32 @@
 
 fn main(x: [u32; 5], mut z: u32, t: u32, index: [Field;5], index2: [Field;5], offset: Field, sublen: Field) {
-    // let idx = (z - 5*t - 5) as Field;
-    let idx = z as Field;
-    // dep::std::println(idx);
+    let idx = (z - 5*t - 5) as Field;
     //dynamic array test
     dyn_array(x, idx, idx - 3); 
 
     //regression for issue 1283
-    // let mut s = 0;
-    // let x3 = [246,159,32,176,8];
-    // for i in 0..5 {
-    //   s += x3[index[i]];
-    // }
-    // assert(s!=0);
+    let mut s = 0;
+    let x3 = [246,159,32,176,8];
+    for i in 0..5 {
+      s += x3[index[i]];
+    }
+    assert(s!=0);
 
-    // if 3 < (sublen as u32) {
-    //   assert(index[offset + 3] == index2[3]);
-    // }
+    if 3 < (sublen as u32) {
+      assert(index[offset + 3] == index2[3]);
+    }
 }
 
 fn dyn_array(mut x: [u32; 5], y: Field, z: Field) {
-    // assert(x[y] == 111);
-    // assert(x[z] == 101);
-    // x[z] = 0;
-    // assert(x[y] == 111);
-    // assert(x[1] == 0);
+    assert(x[y] == 111);
+    assert(x[z] == 101);
+    x[z] = 0;
+    assert(x[y] == 111);
+    assert(x[1] == 0);
     if y as u32 < 10 {
-      // dep::std::println(x);
-      let q = x[y] - 2;
-      // dep::std::println(q);
       x[y] = x[y] - 2;
     } else {
       x[y] = 0;
     }
-    // dep::std::println(x);
     assert(x[4] == 109);
 }
\ No newline at end of file

From 7ed502d7b367ce9e2e6ae92967c3ea3cd8ef0c6e Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 14:42:55 +0000
Subject: [PATCH 22/28] use reachable blocks

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 8c07c9ed29..252e8e82fa 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -26,14 +26,8 @@ impl Ssa {
             let mut all_protected_allocations = HashSet::new();
 
             let mut context = PerFunctionContext::new(function);
-            // let post_order_slice = .iter().rev();
-            let post_order = PostOrder::with_function(function);
-            let post_order_slice = post_order.as_slice();
-
-            // TODO: for array_dynamic the normal post_order works, but for regression_2218 we need it reverse
-            let post_order_iter = post_order_slice.iter().rev();
-            // let post_order_iter = post_order_slice.iter();
-            for block in post_order_iter.clone() {
+
+            for block in function.reachable_blocks() {
                 // Maps Load instruction id -> value to replace the result of the load with
                 let mut loads_to_substitute_per_block = BTreeMap::new();
 
@@ -45,13 +39,13 @@ impl Ssa {
                         &mut function.dfg,
                         &mut loads_to_substitute_per_block,
                         &mut load_values_to_substitute,
-                        *block,
+                        block,
                     );
                 all_protected_allocations.extend(allocations_protected_by_block.into_iter());
             }
 
-            for block in post_order_iter {
-                context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, *block);
+            for block in function.reachable_blocks() {
+                context.remove_unused_stores(&mut function.dfg, &all_protected_allocations, block);
             }
         }
         self

From 71c5693d024496a17de5820019e675fec108bf56 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 15:11:46 +0000
Subject: [PATCH 23/28] restore sha2_blocks test

---
 .../execution_success/sha2_blocks/src/main.nr | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/crates/nargo_cli/tests/execution_success/sha2_blocks/src/main.nr b/crates/nargo_cli/tests/execution_success/sha2_blocks/src/main.nr
index f1afea12da..fcdcdb8684 100644
--- a/crates/nargo_cli/tests/execution_success/sha2_blocks/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/sha2_blocks/src/main.nr
@@ -3,20 +3,20 @@ use dep::std;
 
 fn main(x: [u8; 3], result256: [u8; 32], result512: [u8; 64])
 {
-	// // One-block tests.
-	// let mut digest256 = std::sha256::digest(x);
-	// assert(digest256 == result256);
+	// One-block tests.
+	let mut digest256 = std::sha256::digest(x);
+	assert(digest256 == result256);
 
-	// let mut digest512 = std::sha512::digest(x);
-	// assert(digest512 == result512);
+	let mut digest512 = std::sha512::digest(x);
+	assert(digest512 == result512);
 
-	// // Two-block SHA256 test. Taken from https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/SHA256.pdf
-	// let y: [u8; 56] = [97,98,99,100,98,99,100,101,99,100,101,102,100,101,102,103,101,102,103,104,102,103,104,105,103,104,105,106,104,105,106,107,105,106,107,108,106,107,108,109,107,108,109,110,108,109,110,111,109,110,111,112,110,111,112,113]; // "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"
-	// digest256 = std::sha256::digest(y);
-	// assert(digest256 == [36,141,106,97,210,6,56,184,229,192,38,147,12,62,96,57,163,60,228,89,100,255,33,103,246,236,237,212,25,219,6,193]);
+	// Two-block SHA256 test. Taken from https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/SHA256.pdf
+	let y: [u8; 56] = [97,98,99,100,98,99,100,101,99,100,101,102,100,101,102,103,101,102,103,104,102,103,104,105,103,104,105,106,104,105,106,107,105,106,107,108,106,107,108,109,107,108,109,110,108,109,110,111,109,110,111,112,110,111,112,113]; // "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"
+	digest256 = std::sha256::digest(y);
+	assert(digest256 == [36,141,106,97,210,6,56,184,229,192,38,147,12,62,96,57,163,60,228,89,100,255,33,103,246,236,237,212,25,219,6,193]);
 
-	// // Two-block SHA256 test. Taken from https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/SHA512.pdf
-	// let z: [u8; 112] = [97,98,99,100,101,102,103,104,98,99,100,101,102,103,104,105,99,100,101,102,103,104,105,106,100,101,102,103,104,105,106,107,101,102,103,104,105,106,107,108,102,103,104,105,106,107,108,109,103,104,105,106,107,108,109,110,104,105,106,107,108,109,110,111,105,106,107,108,109,110,111,112,106,107,108,109,110,111,112,113,107,108,109,110,111,112,113,114,108,109,110,111,112,113,114,115,109,110,111,112,113,114,115,116,110,111,112,113,114,115,116,117]; // "abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmnoijklmnopjklmnopqklmnopqrlmnopqrsmnopqrstnopqrstu"
-	// digest512 = std::sha512::digest(z);
-	// assert(digest512 == [142,149,155,117,218,227,19,218,140,244,247,40,20,252,20,63,143,119,121,198,235,159,127,161,114,153,174,173,182,136,144,24,80,29,40,158,73,0,247,228,51,27,153,222,196,181,67,58,199,211,41,238,182,221,38,84,94,150,229,91,135,75,233,9]);
+	// Two-block SHA256 test. Taken from https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/SHA512.pdf
+	let z: [u8; 112] = [97,98,99,100,101,102,103,104,98,99,100,101,102,103,104,105,99,100,101,102,103,104,105,106,100,101,102,103,104,105,106,107,101,102,103,104,105,106,107,108,102,103,104,105,106,107,108,109,103,104,105,106,107,108,109,110,104,105,106,107,108,109,110,111,105,106,107,108,109,110,111,112,106,107,108,109,110,111,112,113,107,108,109,110,111,112,113,114,108,109,110,111,112,113,114,115,109,110,111,112,113,114,115,116,110,111,112,113,114,115,116,117]; // "abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmnoijklmnopjklmnopqklmnopqrlmnopqrsmnopqrstnopqrstu"
+	digest512 = std::sha512::digest(z);
+	assert(digest512 == [142,149,155,117,218,227,19,218,140,244,247,40,20,252,20,63,143,119,121,198,235,159,127,161,114,153,174,173,182,136,144,24,80,29,40,158,73,0,247,228,51,27,153,222,196,181,67,58,199,211,41,238,182,221,38,84,94,150,229,91,135,75,233,9]);
 }

From c612b45592e4d5b20b7fcf08095c7c9e0c790f20 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 15:13:11 +0000
Subject: [PATCH 24/28] remove unnecessary clone and unused methods

---
 crates/noirc_evaluator/src/ssa/ir/post_order.rs |  1 -
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs   | 17 -----------------
 2 files changed, 18 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/ir/post_order.rs b/crates/noirc_evaluator/src/ssa/ir/post_order.rs
index e405fc4604..e3bdbd491d 100644
--- a/crates/noirc_evaluator/src/ssa/ir/post_order.rs
+++ b/crates/noirc_evaluator/src/ssa/ir/post_order.rs
@@ -13,7 +13,6 @@ enum Visit {
     Last,
 }
 
-#[derive(Clone)]
 pub(crate) struct PostOrder(Vec<BasicBlockId>);
 
 impl PostOrder {
diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 252e8e82fa..569462dd89 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -181,23 +181,6 @@ impl PerFunctionContext {
         protected_allocations
     }
 
-    fn find_remaining_loads(
-        &mut self,
-        dfg: &mut DataFlowGraph,
-        block_id: BasicBlockId,
-    ) -> HashSet<AllocId> {
-        let mut protected_allocations = HashSet::new();
-        let block = &dfg[block_id];
-
-        for instruction_id in block.instructions() {
-            if let Instruction::Load { address } = &dfg[*instruction_id] {
-                protected_allocations.insert(*address);
-            }
-        }
-
-        protected_allocations
-    }
-
     // This method will fetch already saved load values to substitute for a given address.
     // The search starts at the block supplied as a parameter. If there is not a load to substitute
     // the CFG is analyzed to determine whether a predecessor block has a load value to substitute.

From ef10232435483d3057acd936328e432a1c4670a0 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 15:33:13 +0000
Subject: [PATCH 25/28] bring back loops check inside fetch methods

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 44 ++++++++++++-------
 crates/noirc_frontend/src/graph/mod.rs        |  2 +-
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 569462dd89..0675185994 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -17,6 +17,8 @@ use crate::ssa::{
     ssa_gen::Ssa,
 };
 
+use super::unrolling::{find_all_loops, Loops};
+
 impl Ssa {
     /// Attempts to remove any load instructions that recover values that are already available in
     /// scope, and attempts to remove stores that are subsequently redundant.
@@ -61,9 +63,7 @@ struct PerFunctionContext {
     cfg: ControlFlowGraph,
     post_order: PostOrder,
     dom_tree: DominatorTree,
-    // Maps block id -> bool stating whether the block shares a common successor
-    // with one of its predecessors
-    has_common_successor: BTreeMap<BasicBlockId, bool>,
+    loops: Loops,
 }
 
 impl PerFunctionContext {
@@ -78,7 +78,7 @@ impl PerFunctionContext {
             cfg,
             post_order,
             dom_tree,
-            has_common_successor: BTreeMap::new(),
+            loops: find_all_loops(function),
         }
     }
 }
@@ -101,14 +101,16 @@ impl PerFunctionContext {
         let mut protected_allocations = HashSet::new();
         let block = &dfg[block_id];
 
+        // Check whether the block has a common successor here to avoid analyzing
+        // the CFG for every block instruction.
         let has_common_successor = self.has_common_successor(block_id);
-        // Maintain a map for whether a block has a common successor to avoid
-        // analyzing the CFG for successors on every store or load
-        self.has_common_successor.insert(block_id, has_common_successor);
 
         for instruction_id in block.instructions() {
             match &dfg[*instruction_id] {
                 Instruction::Store { mut address, value } => {
+                    if has_common_successor {
+                        protected_allocations.insert(address);
+                    }
                     address = self.fetch_load_value_to_substitute(block_id, address);
 
                     self.last_stores_with_block.insert((address, block_id), *value);
@@ -138,10 +140,6 @@ impl PerFunctionContext {
                         // is used by reference aliases in different blocks
                         protected_allocations.insert(dfg.resolve(address));
                     }
-
-                    if has_common_successor {
-                        protected_allocations.insert(address);
-                    }
                 }
                 Instruction::Call { arguments, .. } => {
                     for arg in arguments {
@@ -200,8 +198,16 @@ impl PerFunctionContext {
             // as this pass can occur before loop unrolling and flattening.
             // The mem2reg pass should be ran again following all optimization passes as new values
             // may be able to be promoted
-            if self.has_common_successor[&block_id] {
-                return address;
+            // if self.has_common_successor[&block_id] {
+            //     return address;
+            // }
+            for l in self.loops.yet_to_unroll.iter() {
+                // We do not want to substitute loads that take place within loops as this pass
+                // can occur before loop unrolling
+                // The pass should be ran again following loop unrolling as new values
+                if l.blocks.contains(&block) {
+                    return address;
+                }
             }
 
             // Check whether there is a load value to substitute in the current block.
@@ -246,8 +252,16 @@ impl PerFunctionContext {
             // as this pass can occur before loop unrolling and flattening.
             // The mem2reg pass should be ran again following all optimization passes as new values
             // may be able to be promoted
-            if self.has_common_successor[&block_id] {
-                return false;
+            // if self.has_common_successor[&block_id] {
+            //     return false;
+            // }
+            for l in self.loops.yet_to_unroll.iter() {
+                // We do not want to substitute loads that take place within loops as this pass
+                // can occur before loop unrolling
+                // The pass should be ran again following loop unrolling as new values
+                if l.blocks.contains(&block) {
+                    return false;
+                }
             }
 
             // Check whether there has been a store instruction in the current block
diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs
index 7a1daef45f..90490c47c3 100644
--- a/crates/noirc_frontend/src/graph/mod.rs
+++ b/crates/noirc_frontend/src/graph/mod.rs
@@ -30,7 +30,7 @@ impl CrateId {
 pub struct CrateName(SmolStr);
 
 impl CrateName {
-   fn is_valid_name(name: &str) -> bool {
+    fn is_valid_name(name: &str) -> bool {
         !name.is_empty() && name.chars().all(|n| !CHARACTER_BLACK_LIST.contains(&n))
     }
 }

From 8d535eb0a73bca0b9cdbde7445354028d87ca7cf Mon Sep 17 00:00:00 2001
From: Maxim Vezenov <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 21:02:26 +0100
Subject: [PATCH 26/28] Update
 crates/nargo_cli/tests/execution_success/regression/src/main.nr

Co-authored-by: jfecher <jake@aztecprotocol.com>
---
 crates/nargo_cli/tests/execution_success/regression/src/main.nr | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crates/nargo_cli/tests/execution_success/regression/src/main.nr b/crates/nargo_cli/tests/execution_success/regression/src/main.nr
index 103161482b..54769c3970 100644
--- a/crates/nargo_cli/tests/execution_success/regression/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/regression/src/main.nr
@@ -76,7 +76,6 @@ fn main(x: [u8; 5], z: Field)
     //Issue 1144
 	let (nib, len) = compact_decode(x,z);
 	assert(len == 5);
-	// dep::std::println(nib);
 	assert([nib[0], nib[1], nib[2], nib[3], nib[4]] == [15, 1, 12, 11, 8]);
 
 	// Issue 1169

From f989fe931dac51ec3d99ccbcae9f15007114f9a6 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 20:11:27 +0000
Subject: [PATCH 27/28] guarantee that x == 3 in references regressio nfor 2218

---
 crates/nargo_cli/tests/execution_success/references/src/main.nr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/nargo_cli/tests/execution_success/references/src/main.nr b/crates/nargo_cli/tests/execution_success/references/src/main.nr
index 8b45e130eb..ec23f2f3a3 100644
--- a/crates/nargo_cli/tests/execution_success/references/src/main.nr
+++ b/crates/nargo_cli/tests/execution_success/references/src/main.nr
@@ -36,7 +36,7 @@ fn main(mut x: Field) {
     regression_2030();
     regression_2255();
 
-    // x == 3 at this point
+    assert(x == 3);
     regression_2218_if_inner_if(x, 10);
     regression_2218_if_inner_else(20, x);
     regression_2218_else(x, 3);

From 12ee7ad486090b23813f7fbb33e8c7d28e6df375 Mon Sep 17 00:00:00 2001
From: vezenovm <mvezenov@gmail.com>
Date: Tue, 15 Aug 2023 20:18:54 +0000
Subject: [PATCH 28/28] remove leftover comments

---
 crates/noirc_evaluator/src/ssa/opt/mem2reg.rs | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
index 0675185994..be0ded802b 100644
--- a/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
+++ b/crates/noirc_evaluator/src/ssa/opt/mem2reg.rs
@@ -198,9 +198,6 @@ impl PerFunctionContext {
             // as this pass can occur before loop unrolling and flattening.
             // The mem2reg pass should be ran again following all optimization passes as new values
             // may be able to be promoted
-            // if self.has_common_successor[&block_id] {
-            //     return address;
-            // }
             for l in self.loops.yet_to_unroll.iter() {
                 // We do not want to substitute loads that take place within loops as this pass
                 // can occur before loop unrolling
@@ -252,9 +249,6 @@ impl PerFunctionContext {
             // as this pass can occur before loop unrolling and flattening.
             // The mem2reg pass should be ran again following all optimization passes as new values
             // may be able to be promoted
-            // if self.has_common_successor[&block_id] {
-            //     return false;
-            // }
             for l in self.loops.yet_to_unroll.iter() {
                 // We do not want to substitute loads that take place within loops as this pass
                 // can occur before loop unrolling