Refactored FlowGraphBuilder into a separate FlowGraph representation.

runtime/vm/compiler.cc

Index: runtime/vm/compiler.cc
diff --git a/runtime/vm/compiler.cc b/runtime/vm/compiler.cc
index da0eeb7d36faa2255836fb5b51324d1583211a72..05f5ef6164c9c60056193c00617f1f435520b3da 100644
--- a/runtime/vm/compiler.cc
+++ b/runtime/vm/compiler.cc
@@ -13,6 +13,7 @@
 #include "vm/disassembler.h"
 #include "vm/exceptions.h"
 #include "vm/flags.h"
+#include "vm/flow_graph.h"
 #include "vm/flow_graph_allocator.h"
 #include "vm/flow_graph_builder.h"
 #include "vm/flow_graph_compiler.h"
@@ -132,7 +133,7 @@ static bool CompileParsedFunctionHelper(
   LongJump bailout_jump;
   isolate->set_long_jump_base(&bailout_jump);
   if (setjmp(*bailout_jump.Set()) == 0) {
-    GrowableArray<BlockEntryInstr*> block_order;
+    FlowGraph flow_graph(parsed_function);

[Kevin Millikin (Google), 2012/08/16 08:09:57]

It's a bit strange (to me) that there is such a thing as an unbuilt graph.  For
some reason I expect something more like:

FlowGraphBuilder builder(...);
FlowGraph* graph = builder.BuildGraph();

Also, I'd put this declaration down closer to its first use (after the
TimerScope).

[zerny-google, 2012/08/16 11:52:27]

Sure.
Unfortunately the graph/block_order is used after the TimerScope.

     // TimerScope needs an isolate to be properly terminated in case of a
     // LongJump.
     {
@@ -156,22 +157,33 @@ static bool CompileParsedFunctionHelper(
               ExtractTypeFeedbackArray(unoptimized_code));
         }
       }
-      FlowGraphBuilder graph_builder(parsed_function);
-      graph_builder.BuildGraph(optimized, use_ssa);
-
-      // The non-optimizing compiler compiles blocks in reverse postorder,
-      // because it is a 'natural' order for the human reader of the
-      // generated code.
-      intptr_t length = graph_builder.postorder_block_entries().length();
-      for (intptr_t i = length - 1; i >= 0; --i) {
-        block_order.Add(graph_builder.postorder_block_entries()[i]);
+
+      // Build the flow graph.
+      flow_graph.BuildGraph();
+
+      // Transform to SSA.
+      if (optimized && use_ssa) flow_graph.ComputeSSA();
+
+      if (FLAG_print_flow_graph) {
+        // Print flow graph to stdout.

[Kevin Millikin (Google), 2012/08/16 08:09:57]

Did we lose a "Before Optimizations:\n" that used to be printed here?

[zerny-google, 2012/08/16 11:52:27]

I can't seem to find that in git-svn, but will be happy to add it.

+        FlowGraphPrinter printer(parsed_function.function(),

[Kevin Millikin (Google), 2012/08/16 08:09:57]

I seems like this constructor could just take the graph as input, but feel free
to make that in a separate cleanup change.

[zerny-google, 2012/08/16 11:52:27]

Yes. The other utilities should really take the graph instead of the block entry
arrays.

+                                 flow_graph.reverse_postorder());
+        printer.PrintBlocks();
       }
+      if (Dart::flow_graph_writer() != NULL) {
+        // Write flow graph to file.
+        FlowGraphVisualizer printer(parsed_function.function(),

[Kevin Millikin (Google), 2012/08/16 08:09:57]

Also here and most of the constructors below, the graph could be the input.

It's just a bit weird that a class that mutates the graph (e.g.,
FlowGraphOptimizer) does not take the graph itself as an argument.

[zerny-google, 2012/08/16 11:52:27]

Yes.

+                                    flow_graph.reverse_postorder());
+        printer.PrintFunction();
+      }
+
       if (optimized) {
-        FlowGraphOptimizer optimizer(block_order);
+        FlowGraphOptimizer optimizer(flow_graph.reverse_postorder());
         optimizer.ApplyICData();
 
         // Propagate types and eliminate more type tests.
-        FlowGraphTypePropagator propagator(parsed_function, block_order);
+        FlowGraphTypePropagator propagator(parsed_function,
+                                           flow_graph.reverse_postorder());
         propagator.PropagateTypes();
 
         // Do optimizations that depend on the propagated type information.
@@ -179,12 +191,14 @@ static bool CompileParsedFunctionHelper(
 
         if (use_ssa) {
           // Perform register allocation on the SSA graph.
-          FlowGraphAllocator allocator(block_order, &graph_builder);
+          FlowGraphAllocator allocator(&flow_graph);
           allocator.AllocateRegisters();
         }
+
         if (FLAG_print_flow_graph) {
           OS::Print("After Optimizations:\n");
-          FlowGraphPrinter printer(Function::Handle(), block_order);
+          FlowGraphPrinter printer(Function::Handle(),
+                                   flow_graph.reverse_postorder());
           printer.PrintBlocks();
         }
       }
@@ -192,14 +206,14 @@ static bool CompileParsedFunctionHelper(
 
     bool is_leaf = false;
     if (optimized) {
-      FlowGraphAnalyzer analyzer(block_order);
+      FlowGraphAnalyzer analyzer(flow_graph.reverse_postorder());
       analyzer.Analyze();
       is_leaf = analyzer.is_leaf();
     }
     Assembler assembler;
     FlowGraphCompiler graph_compiler(&assembler,
                                      parsed_function,
-                                     block_order,
+                                     flow_graph.reverse_postorder(),
                                      optimized,
                                      optimized && use_ssa,
                                      is_leaf);