Refactored FlowGraphBuilder into a separate FlowGraph representation.

runtime/vm/compiler.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/compiler.h"
 
 #include "vm/assembler.h"
 #include "vm/ast_printer.h"
 #include "vm/code_generator.h"
 #include "vm/code_patcher.h"
 #include "vm/dart_entry.h"
 #include "vm/debugger.h"
 #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"
 #include "vm/flow_graph_optimizer.h"
 #include "vm/il_printer.h"
 #include "vm/longjump.h"
 #include "vm/object.h"
 #include "vm/object_store.h"
 #include "vm/os.h"
 #include "vm/parser.h"
@@ skipping 99 matching lines @@
   TimerScope timer(FLAG_compiler_stats, &CompilerStats::codegen_timer);
   bool is_compiled = false;
   Isolate* isolate = Isolate::Current();
   ASSERT(isolate->ic_data_array() == Array::null());  // Must be reset to null.
   const intptr_t prev_deopt_id = isolate->deopt_id();
   isolate->set_deopt_id(0);
   LongJump* old_base = isolate->long_jump_base();
   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.
     {
       TimerScope timer(FLAG_compiler_stats,
                        &CompilerStats::graphbuilder_timer,
                        isolate);
       if (optimized) {
         // Transition to optimized code only from unoptimized code ...
         // for now.
         ASSERT(parsed_function.function().HasCode());
         ASSERT(!parsed_function.function().HasOptimizedCode());
         // Extract type feedback before the graph is built, as the graph
         // builder uses it to attach it to nodes.
         // Do not use type feedback to optimize a function that was
         // deoptimized too often.
         if (parsed_function.function().deoptimization_counter() <
             FLAG_deoptimization_counter_threshold) {
           const Code& unoptimized_code =
               Code::Handle(parsed_function.function().unoptimized_code());
           isolate->set_ic_data_array(
               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.
         optimizer.OptimizeComputations();
 
         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();
         }
       }
     }
 
     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);
     {
       TimerScope timer(FLAG_compiler_stats,
                        &CompilerStats::graphcompiler_timer,
                        isolate);
       graph_compiler.CompileGraph();
     }
     {
@@ skipping 318 matching lines @@
     result = isolate->object_store()->sticky_error();
     isolate->object_store()->clear_sticky_error();
     isolate->set_long_jump_base(base);
     return result.raw();
   }
   UNREACHABLE();
   return Object::null();
 }
 
 }  // namespace dart