Fix handling of CFG triangles in compute_postorder.

For example, in `loops_one`, ebb3 is the bottom of a triangle, so
postorder should order it after the rest of the triangle.

cranelift/tests/cfg_traversal.rs

@@ -51,7 +51,7 @@ fn simple_traversal() {
                 trap user0
         }
     ",
-        vec![0, 1, 3, 2, 4, 5],
+        vec![0, 2, 5, 4, 1, 3],
     );
 }
 
@@ -71,7 +71,7 @@ fn loops_one() {
                 return
         }
     ",
-        vec![0, 1, 3, 2],
+        vec![0, 1, 2, 3],
     );
 }
 
@@ -98,7 +98,7 @@ fn loops_two() {
                 return
         }
     ",
-        vec![0, 1, 2, 4, 3, 5],
+        vec![0, 2, 1, 3, 4, 5],
     );
 }
 
@@ -130,7 +130,7 @@ fn loops_three() {
                 return
         }
     ",
-        vec![0, 1, 2, 4, 3, 6, 7, 5],
+        vec![0, 2, 1, 3, 4, 6, 7, 5],
     );
 }
 

lib/cretonne/src/dominator_tree.rs

@@ -36,7 +36,7 @@ pub struct DominatorTree {
     postorder: Vec<Ebb>,
 
     // Scratch memory used by `compute_postorder()`.
-    stack: Vec<Ebb>,
+    stack: Vec<(Ebb, usize)>,
 
     valid: bool,
 }
@@ -255,39 +255,27 @@ impl DominatorTree {
 
         // During this algorithm only, use `rpo_number` to hold the following state:
         //
-        // 0: EBB never reached.
+        // 0: EBB is not yet on the stack.
-        // 2: EBB has been pushed once, so it shouldn't be pushed again.
+        // 1: EBB is on the stack or in postorder.
-        // 1: EBB has already been popped once, and should be added to the post-order next time.
+        const SEEN: u32 = 1;
-        const SEEN: u32 = 2;
-        const DONE: u32 = 1;
 
         match func.layout.entry_block() {
             Some(ebb) => {
+                self.stack.push((ebb, 0));
                 self.nodes[ebb].rpo_number = SEEN;
-                self.stack.push(ebb)
             }
             None => return,
         }
 
-        while let Some(ebb) = self.stack.pop() {
+        while let Some((ebb, succ_index)) = self.stack.pop() {
-            match self.nodes[ebb].rpo_number {
+            if let Some(&succ) = cfg.get_successors(ebb).get(succ_index) {
-                // This is the first time we visit `ebb`, forming a pre-order.
+                self.stack.push((ebb, succ_index + 1));
-                SEEN => {
+                if self.nodes[succ].rpo_number == 0 {
-                    // Mark it as done and re-queue it to be visited after its children.
+                    self.stack.push((succ, 0));
-                    self.nodes[ebb].rpo_number = DONE;
+                    self.nodes[succ].rpo_number = SEEN;
-                    self.stack.push(ebb);
-                    for &succ in cfg.get_successors(ebb) {
-                        // Only push children that haven't been seen before.
-                        if self.nodes[succ].rpo_number == 0 {
-                            self.nodes[succ].rpo_number = SEEN;
-                            self.stack.push(succ);
-                        }
-                    }
                 }
-                // This is the second time we popped `ebb`, so all its children have been visited.
+            } else {
-                // This is the post-order.
+                self.postorder.push(ebb);
-                DONE => self.postorder.push(ebb),
-                _ => panic!("Inconsistent stack rpo_number"),
             }
         }
     }
@@ -522,7 +510,7 @@ mod test {
             Ordering::Less
         );
 
-        assert_eq!(dt.cfg_postorder(), &[ebb2, ebb0, ebb1, ebb3]);
+        assert_eq!(dt.cfg_postorder(), &[ebb0, ebb2, ebb1, ebb3]);
     }
 
     #[test]

lib/cretonne/src/loop_analysis.rs

@@ -326,16 +326,16 @@ mod test {
         let loops = loop_analysis.loops().collect::<Vec<Loop>>();
         assert_eq!(loops.len(), 3);
         assert_eq!(loop_analysis.loop_header(loops[0]), ebb0);
-        assert_eq!(loop_analysis.loop_header(loops[1]), ebb1);
+        assert_eq!(loop_analysis.loop_header(loops[1]), ebb3);
-        assert_eq!(loop_analysis.loop_header(loops[2]), ebb3);
+        assert_eq!(loop_analysis.loop_header(loops[2]), ebb1);
         assert_eq!(loop_analysis.loop_parent(loops[1]), Some(loops[0]));
         assert_eq!(loop_analysis.loop_parent(loops[2]), Some(loops[0]));
         assert_eq!(loop_analysis.loop_parent(loops[0]), None);
         assert_eq!(loop_analysis.is_in_loop(ebb0, loops[0]), true);
-        assert_eq!(loop_analysis.is_in_loop(ebb1, loops[1]), true);
+        assert_eq!(loop_analysis.is_in_loop(ebb3, loops[1]), true);
-        assert_eq!(loop_analysis.is_in_loop(ebb2, loops[1]), true);
+        assert_eq!(loop_analysis.is_in_loop(ebb4, loops[1]), true);
-        assert_eq!(loop_analysis.is_in_loop(ebb3, loops[2]), true);
+        assert_eq!(loop_analysis.is_in_loop(ebb1, loops[2]), true);
-        assert_eq!(loop_analysis.is_in_loop(ebb4, loops[2]), true);
+        assert_eq!(loop_analysis.is_in_loop(ebb2, loops[2]), true);
         assert_eq!(loop_analysis.is_in_loop(ebb5, loops[0]), true);
     }
 }