Auto merge of #43713 - arielb1:legacy-dataflow, r=eddyb

rustc::middle::dataflow - visit the CFG in RPO

We used to propagate bits in node-id order, which sometimes caused an
excessive number of iterations, especially when macros were present. As
everyone knows, visiting the CFG in RPO bounds the number of iterators
by 1 plus the depth of the most deeply nested loop (times the height of
the lattice, which is 1).

I have no idea how this affects borrowck perf in the non-worst-case, so it's probably a good idea to not roll this up so we can see the effects.

Fixes #43704.

r? @eddyb

Author: bors

src/librustc/middle/dataflow.rs

@@ -22,6 +22,9 @@ use std::mem;
 use std::usize;
 use syntax::ast;
 use syntax::print::pprust::PrintState;
+
+use rustc_data_structures::graph::OUTGOING;
+
 use util::nodemap::NodeMap;
 use hir;
 use hir::intravisit::{self, IdRange};
@@ -523,12 +526,16 @@ impl<'a, 'tcx, O:DataFlowOperator+Clone+'static> DataFlowContext<'a, 'tcx, O> {
                 changed: true
             };
 
+            let nodes_po = cfg.graph.nodes_in_postorder(OUTGOING, cfg.entry);
             let mut temp = vec![0; words_per_id];
+            let mut num_passes = 0;
             while propcx.changed {
+                num_passes += 1;
                 propcx.changed = false;
                 propcx.reset(&mut temp);
-                propcx.walk_cfg(cfg, &mut temp);
+                propcx.walk_cfg(cfg, &nodes_po, &mut temp);
             }
+            debug!("finished in {} iterations", num_passes);
         }
 
         debug!("Dataflow result for {}:", self.analysis_name);
@@ -543,12 +550,15 @@ impl<'a, 'tcx, O:DataFlowOperator+Clone+'static> DataFlowContext<'a, 'tcx, O> {
 impl<'a, 'b, 'tcx, O:DataFlowOperator> PropagationContext<'a, 'b, 'tcx, O> {
     fn walk_cfg(&mut self,
                 cfg: &cfg::CFG,
+                nodes_po: &[CFGIndex],
                 in_out: &mut [usize]) {
         debug!("DataFlowContext::walk_cfg(in_out={}) {}",
                bits_to_string(in_out), self.dfcx.analysis_name);
         assert!(self.dfcx.bits_per_id > 0);
 
-        cfg.graph.each_node(|node_index, node| {
+        // Iterate over nodes in reverse postorder
+        for &node_index in nodes_po.iter().rev() {
+            let node = cfg.graph.node(node_index);
             debug!("DataFlowContext::walk_cfg idx={:?} id={} begin in_out={}",
                    node_index, node.data.id(), bits_to_string(in_out));
 
@@ -563,8 +573,7 @@ impl<'a, 'b, 'tcx, O:DataFlowOperator> PropagationContext<'a, 'b, 'tcx, O> {
 
             // Propagate state on-exit from node into its successors.
             self.propagate_bits_into_graph_successors_of(in_out, cfg, node_index);
-            true // continue to next node
-        });
+        }
     }
 
     fn reset(&mut self, bits: &mut [usize]) {

src/librustc_data_structures/graph/mod.rs

@@ -308,6 +308,42 @@ impl<N: Debug, E: Debug> Graph<N, E> {
         DepthFirstTraversal::with_start_node(self, start, direction)
     }
 
+    pub fn nodes_in_postorder<'a>(&'a self,
+                                  direction: Direction,
+                                  entry_node: NodeIndex)
+                                  -> Vec<NodeIndex>
+    {
+        let mut visited = BitVector::new(self.len_nodes());
+        let mut stack = vec![];
+        let mut result = Vec::with_capacity(self.len_nodes());
+        let mut push_node = |stack: &mut Vec<_>, node: NodeIndex| {
+            if visited.insert(node.0) {
+                stack.push((node, self.adjacent_edges(node, direction)));
+            }
+        };
+
+        for node in Some(entry_node).into_iter()
+            .chain(self.enumerated_nodes().map(|(node, _)| node))
+        {
+            push_node(&mut stack, node);
+            while let Some((node, mut iter)) = stack.pop() {
+                if let Some((_, child)) = iter.next() {
+                    let target = child.source_or_target(direction);
+                    // the current node needs more processing, so
+                    // add it back to the stack
+                    stack.push((node, iter));
+                    // and then push the new node
+                    push_node(&mut stack, target);
+                } else {
+                    result.push(node);
+                }
+            }
+        }
+
+        assert_eq!(result.len(), self.len_nodes());
+        result
+    }
+
     /// Whether or not a node can be reached from itself.
     pub fn is_node_cyclic(&self, starting_node_index: NodeIndex) -> bool {
         // This is similar to depth traversal below, but we

src/librustc_data_structures/graph/tests.rs

@@ -175,3 +175,46 @@ fn is_node_cyclic_b() {
     let graph = create_graph_with_cycle();
     assert!(graph.is_node_cyclic(NodeIndex(1)));
 }
+
+#[test]
+fn nodes_in_postorder() {
+    let expected = vec![
+        ("A", vec!["C", "E", "D", "B", "A", "F"]),
+        ("B", vec!["C", "E", "D", "B", "A", "F"]),
+        ("C", vec!["C", "E", "D", "B", "A", "F"]),
+        ("D", vec!["C", "E", "D", "B", "A", "F"]),
+        ("E", vec!["C", "E", "D", "B", "A", "F"]),
+        ("F", vec!["C", "E", "D", "B", "F", "A"])
+    ];
+
+    let graph = create_graph();
+
+    for ((idx, node), &(node_name, ref expected))
+        in graph.enumerated_nodes().zip(&expected)
+    {
+        assert_eq!(node.data, node_name);
+        assert_eq!(expected,
+                   &graph.nodes_in_postorder(OUTGOING, idx)
+                   .into_iter().map(|idx| *graph.node_data(idx))
+                   .collect::<Vec<&str>>());
+    }
+
+    let expected = vec![
+        ("A", vec!["D", "C", "B", "A"]),
+        ("B", vec!["D", "C", "B", "A"]),
+        ("C", vec!["B", "D", "C", "A"]),
+        ("D", vec!["C", "B", "D", "A"]),
+    ];
+
+    let graph = create_graph_with_cycle();
+
+    for ((idx, node), &(node_name, ref expected))
+        in graph.enumerated_nodes().zip(&expected)
+    {
+        assert_eq!(node.data, node_name);
+        assert_eq!(expected,
+                   &graph.nodes_in_postorder(OUTGOING, idx)
+                   .into_iter().map(|idx| *graph.node_data(idx))
+                   .collect::<Vec<&str>>());
+    }
+}