feat(ssa): Basic control dependent LICM

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

@@ -19,11 +19,14 @@ struct CfgNode {
 }
 
 #[derive(Clone, Default)]
-/// The Control Flow Graph maintains a mapping of blocks to their predecessors
+/// The Control Flow Graph (CFG) maintains a mapping of blocks to their predecessors
 /// and successors where predecessors are basic blocks and successors are
 /// basic blocks.
 pub(crate) struct ControlFlowGraph {
     data: HashMap<BasicBlockId, CfgNode>,
+    /// Flag stating whether this CFG has been reversed.
+    /// In a reversed CFG, successors become predecessors.
+    reversed: bool,
 }
 
 impl ControlFlowGraph {
@@ -37,7 +40,7 @@ impl ControlFlowGraph {
         let mut data = HashMap::default();
         data.insert(entry_block, empty_node);
 
-        let mut cfg = ControlFlowGraph { data };
+        let mut cfg = ControlFlowGraph { data, reversed: false };
         cfg.compute(func);
         cfg
     }
@@ -89,10 +92,12 @@ impl ControlFlowGraph {
     /// Add a directed edge making `from` a predecessor of `to`.
     fn add_edge(&mut self, from: BasicBlockId, to: BasicBlockId) {
         let predecessor_node = self.data.entry(from).or_default();
-        assert!(
-            predecessor_node.successors.len() < 2,
-            "ICE: A cfg node cannot have more than two successors"
-        );
+        if !self.reversed {
+            assert!(
+                predecessor_node.successors.len() < 2,
+                "ICE: A cfg node cannot have more than two successors"
+            );
+        }
         predecessor_node.successors.insert(to);
         let successor_node = self.data.entry(to).or_default();
         successor_node.predecessors.insert(from);
@@ -125,9 +130,8 @@ impl ControlFlowGraph {
     }
 
     /// Reverse the control flow graph
-    #[cfg(test)]
     pub(crate) fn reverse(&self) -> Self {
-        let mut reversed_cfg = ControlFlowGraph::default();
+        let mut reversed_cfg = ControlFlowGraph { reversed: true, ..Default::default() };
 
         for (block_id, node) in &self.data {
             // For each block, reverse the edges

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

@@ -9,10 +9,7 @@ use std::cmp::Ordering;
 use super::{
     basic_block::BasicBlockId, cfg::ControlFlowGraph, function::Function, post_order::PostOrder,
 };
-
-use fxhash::FxHashMap as HashMap;
-#[allow(unused_imports)]
-use fxhash::FxHashSet as HashSet;
+use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
 
 /// Dominator tree node. We keep one of these per reachable block.
 #[derive(Clone, Default)]
@@ -181,10 +178,10 @@ impl DominatorTree {
     /// "Simple, Fast Dominator Algorithm."
     fn compute_dominator_tree(&mut self, cfg: &ControlFlowGraph, post_order: &PostOrder) {
         // We'll be iterating over a reverse post-order of the CFG, skipping the entry block.
-        let (entry_block_id, entry_free_post_order) = post_order
-            .as_slice()
-            .split_last()
-            .expect("ICE: functions always have at least one block");
+        let Some((entry_block_id, entry_free_post_order)) = post_order.as_slice().split_last()
+        else {
+            return;
+        };
 
         // Do a first pass where we assign reverse post-order indices to all reachable nodes. The
         // entry block will be the only node with no immediate dominator.
@@ -290,7 +287,6 @@ impl DominatorTree {
     /// a dominator tree (standard CFG) or a post-dominator tree (reversed CFG).
     /// Calling this method on a dominator tree will return a function's dominance frontiers,
     /// while on a post-dominator tree the method will return the function's reverse (or post) dominance frontiers.
-    #[cfg(test)]
     pub(crate) fn compute_dominance_frontiers(
         &mut self,
         cfg: &ControlFlowGraph,

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

@@ -525,6 +525,74 @@ impl Instruction {
         }
     }
 
+    /// Indicates if the instruction can be safely hoisted out of a loop.
+    /// If `hoist_with_predicate` is set, we assume we're hoisting the instruction
+    /// and its predicate, rather than just the instruction. Setting this means instructions that
+    /// rely on predicates can be hoisted as well.
+    ///
+    /// Certain instructions can be hoisted because they implicitly depend on a predicate.
+    /// However, to avoid tight coupling between passes, we make the hoisting
+    /// conditional on whether the caller wants the predicate to be taken into account or not.
+    ///
+    /// This differs from `can_be_deduplicated` as that method assumes there is a matching instruction
+    /// with the same inputs. Hoisting is for lone instructions, meaning a mislabeled hoist could cause
+    /// unexpected failures if the instruction was never meant to be executed.
+    pub(crate) fn can_be_hoisted(&self, function: &Function, hoist_with_predicate: bool) -> bool {
+        use Instruction::*;
+
+        match self {
+            // These either have side-effects or interact with memory
+            EnableSideEffectsIf { .. }
+            | Allocate
+            | Load { .. }
+            | Store { .. }
+            | IncrementRc { .. }
+            | DecrementRc { .. } => false,
+
+            Call { func, .. } => match function.dfg[*func] {
+                Value::Intrinsic(intrinsic) => intrinsic.can_be_deduplicated(false),
+                Value::Function(id) => match function.dfg.purity_of(id) {
+                    Some(Purity::Pure) => true,
+                    Some(Purity::PureWithPredicate) => false,
+                    Some(Purity::Impure) => false,
+                    None => false,
+                },
+                _ => false,
+            },
+
+            // We cannot hoist these instructions, even if we know the predicate is the same.
+            // This is because an loop with dynamic bounds may never execute its loop body.
+            // If the instruction were to trigger a failure, our program may fail inadvertently.
+            // If we know a loop's upper bound is greater than its lower bound we can hoist these instructions,
+            // but we do not want to assume that the caller of this method has accounted
+            // for this case. Thus, we block hoisting on these instructions.
+            Constrain(..) | ConstrainNotEqual(..) | RangeCheck { .. } => false,
+
+            // Noop instructions can always be hoisted, although they're more likely to be
+            // removed entirely.
+            Noop => true,
+
+            // Cast instructions can always be hoisted
+            Cast(_, _) => true,
+
+            // Arrays can be mutated in unconstrained code so code that handles this case must
+            // take care to track whether the array was possibly mutated or not before
+            // hoisted. Since we don't know if the containing pass checks for this, we
+            // can only assume these are safe to hoist in constrained code.
+            MakeArray { .. } => function.runtime().is_acir(),
+
+            // These can have different behavior depending on the predicate.
+            Binary(_)
+            | Not(_)
+            | Truncate { .. }
+            | IfElse { .. }
+            | ArrayGet { .. }
+            | ArraySet { .. } => {
+                hoist_with_predicate || !self.requires_acir_gen_predicate(&function.dfg)
+            }
+        }
+    }
+
     pub(crate) fn can_eliminate_if_unused(&self, function: &Function, flattened: bool) -> bool {
         use Instruction::*;
         match self {