Merge 096dcb3 into e70f719

Author: vezenovm

.github/benchmark_projects.yml

@@ -5,7 +5,7 @@ projects:
     ref: *AZ_COMMIT
     path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner
     num_runs: 5
-    compilation-timeout: 2.5
+    compilation-timeout: 3
     execution-timeout: 0.08
     compilation-memory-limit: 350
     execution-memory-limit: 250

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

@@ -89,10 +89,6 @@ 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"
-        );
         predecessor_node.successors.insert(to);
         let successor_node = self.data.entry(to).or_default();
         successor_node.predecessors.insert(from);
@@ -125,7 +121,6 @@ impl ControlFlowGraph {
     }
 
     /// Reverse the control flow graph
-    #[cfg(test)]
     pub(crate) fn reverse(&self) -> Self {
         let mut reversed_cfg = ControlFlowGraph::default();
 

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

@@ -9,7 +9,7 @@ use std::cmp::Ordering;
 use super::{
     basic_block::BasicBlockId, cfg::ControlFlowGraph, function::Function, post_order::PostOrder,
 };
-use fxhash::FxHashMap as HashMap;
+use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
 
 /// Dominator tree node. We keep one of these per reachable block.
 #[derive(Clone, Default)]
@@ -178,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.
@@ -276,18 +276,67 @@ impl DominatorTree {
         debug_assert_eq!(block_a_id, block_b_id, "Unreachable block passed to common_dominator?");
         block_a_id
     }
+
+    /// Computes the dominance frontier for all blocks in the dominator tree.
+    ///
+    /// This method uses the algorithm specified under Cooper, Keith D. et al. “A Simple, Fast Dominance Algorithm.” (1999).
+    /// As referenced in the paper a dominance frontier is the set of all CFG nodes, y, such that
+    /// b dominates a predecessor of y but does not strictly d.
+    ///
+    /// This method expects the appropriate CFG depending on whether we are operating over
+    /// 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.
+    pub(crate) fn compute_dominance_frontiers(
+        &mut self,
+        cfg: &ControlFlowGraph,
+    ) -> HashMap<BasicBlockId, HashSet<BasicBlockId>> {
+        let mut dominance_frontiers: HashMap<BasicBlockId, HashSet<BasicBlockId>> =
+            HashMap::default();
+
+        let nodes = self.nodes.keys().copied().collect::<Vec<_>>();
+        for block_id in nodes {
+            let predecessors = cfg.predecessors(block_id);
+            // Dominance frontier nodes must have more than one predecessor
+            if predecessors.len() > 1 {
+                // Iterate over the predecessors of the current block
+                for pred_id in predecessors {
+                    let mut runner = pred_id;
+                    // We start by checking if the current block dominates the predecessor
+                    while let Some(immediate_dominator) = self.immediate_dominator(block_id) {
+                        if immediate_dominator != runner {
+                            dominance_frontiers.entry(runner).or_default().insert(block_id);
+                            let Some(runner_immediate_dom) = self.immediate_dominator(runner)
+                            else {
+                                break;
+                            };
+                            runner = runner_immediate_dom;
+                        } else {
+                            break;
+                        }
+                    }
+                }
+            }
+        }
+
+        dominance_frontiers
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use std::cmp::Ordering;
 
+    use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
+
     use crate::ssa::{
         function_builder::FunctionBuilder,
         ir::{
-            basic_block::BasicBlockId, call_stack::CallStackId, dom::DominatorTree,
-            function::Function, instruction::TerminatorInstruction, map::Id, types::Type,
+            basic_block::BasicBlockId, call_stack::CallStackId, cfg::ControlFlowGraph,
+            dom::DominatorTree, function::Function, instruction::TerminatorInstruction, map::Id,
+            post_order::PostOrder, types::Type,
         },
+        ssa_gen::Ssa,
     };
 
     #[test]
@@ -542,6 +591,164 @@ mod tests {
         assert!(post_dom.dominates(block2_id, block2_id));
     }
 
+    #[test]
+    fn dom_frontiers_backwards_layout() {
+        let (func, ..) = backwards_layout_setup();
+        let mut dt = DominatorTree::with_function(&func);
+
+        let cfg = ControlFlowGraph::with_function(&func);
+        let dom_frontiers = dt.compute_dominance_frontiers(&cfg);
+        assert!(dom_frontiers.is_empty());
+    }
+
+    #[test]
+    fn post_dom_frontiers_backwards_layout() {
+        let (func, ..) = backwards_layout_setup();
+        let mut post_dom = DominatorTree::with_function_post_dom(&func);
+
+        let cfg = ControlFlowGraph::with_function(&func);
+        let dom_frontiers = post_dom.compute_dominance_frontiers(&cfg);
+        assert!(dom_frontiers.is_empty());
+    }
+
+    fn loop_with_conditional() -> Ssa {
+        let src = "
+        brillig(inline) predicate_pure fn main f0 {
+          b0(v1: u32, v2: u32):
+            v5 = eq v1, u32 5
+            jmp b1(u32 0)
+          b1(v3: u32):
+            v8 = lt v3, u32 4
+            jmpif v8 then: b2, else: b3
+          b2():
+            jmpif v5 then: b4, else: b5
+          b3():
+            return
+          b4():
+            v9 = mul u32 4294967295, v2
+            constrain v9 == u32 12
+            jmp b5()
+          b5():
+            v12 = unchecked_add v3, u32 1
+            jmp b1(v12)
+        }
+        ";
+        Ssa::from_str(src).unwrap()
+    }
+
+    #[test]
+    fn dom_frontiers() {
+        let ssa = loop_with_conditional();
+        let main = ssa.main();
+
+        let cfg = ControlFlowGraph::with_function(main);
+        let post_order = PostOrder::with_cfg(&cfg);
+
+        let mut dt = DominatorTree::with_cfg_and_post_order(&cfg, &post_order);
+        let dom_frontiers = dt.compute_dominance_frontiers(&cfg);
+        dbg!(dom_frontiers.clone());
+
+        // Convert BasicBlockIds to their underlying u32 values for easy comparisons
+        let dom_frontiers = dom_frontiers
+            .into_iter()
+            .map(|(block, frontier)| {
+                (
+                    block.to_u32(),
+                    frontier.into_iter().map(|block| block.to_u32()).collect::<HashSet<_>>(),
+                )
+            })
+            .collect::<HashMap<_, _>>();
+
+        // b0 is the entry block which dominates all other blocks
+        // Thus, it has an empty set for its dominance frontier
+        assert!(!dom_frontiers.contains_key(&0));
+
+        // b1 is in its own DF due to the loop b5 -> b1
+        // b1 dominates b5 which is a predecessor of b1, but b1 does not strictly dominate b1
+        let b1_df = &dom_frontiers[&1];
+        assert_eq!(b1_df.len(), 1);
+        assert!(b1_df.contains(&1));
+
+        // b2 has DF { b1 } also due to the loop b5 -> b1.
+        // b2 dominates b5 which is a predecessor of b1, but b2 does not strictly dominate b1
+        let b2_df = &dom_frontiers[&2];
+        assert_eq!(b2_df.len(), 1);
+        assert!(b2_df.contains(&1));
+
+        // b3 is the exit block which does not dominate any blocks
+        assert!(!dom_frontiers.contains_key(&3));
+
+        // b4 has DF { b5 } because b4 jumps to b5 (thus being a predecessor to b5)
+        // b4 dominates itself but b5 is not strictly dominated by b4.
+        let b4_df = &dom_frontiers[&4];
+        assert_eq!(b4_df.len(), 1);
+        assert!(b4_df.contains(&5));
+
+        // b5 has DF { b1 } also due to the loop b5 -> b1
+        let b5_df = &dom_frontiers[&5];
+        assert_eq!(b5_df.len(), 1);
+        assert!(b5_df.contains(&1));
+    }
+
+    #[test]
+    fn post_dom_frontiers() {
+        let ssa = loop_with_conditional();
+        let main = ssa.main();
+
+        let cfg = ControlFlowGraph::with_function(main);
+        let reversed_cfg = cfg.reverse();
+        let post_order = PostOrder::with_cfg(&reversed_cfg);
+
+        let mut post_dom = DominatorTree::with_cfg_and_post_order(&reversed_cfg, &post_order);
+        let post_dom_frontiers = post_dom.compute_dominance_frontiers(&reversed_cfg);
+        dbg!(post_dom_frontiers.clone());
+
+        // Convert BasicBlockIds to their underlying u32 values for easy comparisons
+        let post_dom_frontiers = post_dom_frontiers
+            .into_iter()
+            .map(|(block, frontier)| {
+                (
+                    block.to_u32(),
+                    frontier.into_iter().map(|block| block.to_u32()).collect::<HashSet<_>>(),
+                )
+            })
+            .collect::<HashMap<_, _>>();
+
+        // Another way to think about the post-dominator frontier for a node n,
+        // is that we can reach a block in the PDF during execution without going through n.
+
+        // b0 is the entry node of the program and the exit block of the post-dominator tree.
+        // Thus, it has an empty set for its post-dominance frontier (PDF)
+        assert!(!post_dom_frontiers.contains_key(&0));
+
+        // b1 is in its own PDF due to the loop b5 -> b1
+        // b1 post-dominates b2 and b5. b1 post-dominates itself but not strictly post-dominate itself.
+        let b1_pdf = &post_dom_frontiers[&1];
+        assert_eq!(b1_pdf.len(), 1);
+        assert!(b1_pdf.contains(&1));
+
+        // b2 has DF { b1 } also due to the loop b5 -> b1.
+        // b1 post-dominates itself is a predecessor of b2, but b1 does not strictly post-dominate b2.
+        let b2_pdf = &post_dom_frontiers[&2];
+        assert_eq!(b2_pdf.len(), 1);
+        assert!(b2_pdf.contains(&1));
+
+        // b3 is the exit block of the program, but the starting node of the post-dominator tree
+        // Thus, it has an empty PDF
+        assert!(!post_dom_frontiers.contains_key(&3));
+
+        // b4 has DF { b2 } because b2 post-dominates itself and is a predecessor to b4.
+        // b2 does not strictly post-dominate b4.
+        let b4_pdf = &post_dom_frontiers[&4];
+        assert_eq!(b4_pdf.len(), 1);
+        assert!(b4_pdf.contains(&2));
+
+        // b5 has DF { b1 } also due to the loop b5 -> b1
+        let b5_pdf = &post_dom_frontiers[&5];
+        assert_eq!(b5_pdf.len(), 1);
+        assert!(b5_pdf.contains(&1));
+    }
+
     #[test]
     fn test_find_map_dominator() {
         let (dt, b0, b1, b2, _b3) = unreachable_node_setup();