[vm/compiler] Move reorder_blocks onto the graph.

Make FlowGraph constructor compute whether we are planning to reorder
blocks before code generation or not. This makes the state of the
graph at the end of the compilation more clear.

This is just a refactoring without any functional changes.

TEST=ci

Change-Id: Ieefb02237cc1ebd69d5d2b217bdc8ebbfdbf15c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358446
Commit-Queue: Slava Egorov <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>

Author: mraleph

runtime/vm/compiler/aot/precompiler.cc

@@ -3516,8 +3516,6 @@ bool PrecompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
 
       CompilerPassState pass_state(thread(), flow_graph, &speculative_policy,
                                    precompiler_);
-      pass_state.reorder_blocks =
-          FlowGraph::ShouldReorderBlocks(function, optimized());
 
       if (function.ForceOptimize()) {
         ASSERT(optimized());

runtime/vm/compiler/backend/block_scheduler.cc

@@ -156,6 +156,10 @@ static void Union(GrowableArray<Chain*>* chains,
 }
 
 void BlockScheduler::ReorderBlocks(FlowGraph* flow_graph) {
+  if (!flow_graph->should_reorder_blocks()) {
+    return;
+  }
+
   if (CompilerState::Current().is_aot()) {
     ReorderBlocksAOT(flow_graph);
   } else {
@@ -164,10 +168,6 @@ void BlockScheduler::ReorderBlocks(FlowGraph* flow_graph) {
 }
 
 void BlockScheduler::ReorderBlocksJIT(FlowGraph* flow_graph) {
-  if (!FLAG_reorder_basic_blocks) {
-    return;
-  }
-
   // Add every block to a chain of length 1 and compute a list of edges
   // sorted by weight.
   intptr_t block_count = flow_graph->preorder().length();
@@ -219,10 +219,10 @@ void BlockScheduler::ReorderBlocksJIT(FlowGraph* flow_graph) {
   // Ensure the checked entry remains first to avoid needing another offset on
   // Instructions, compare Code::EntryPointOf.
   GraphEntryInstr* graph_entry = flow_graph->graph_entry();
-  flow_graph->CodegenBlockOrder(true)->Add(graph_entry);
+  flow_graph->CodegenBlockOrder()->Add(graph_entry);
   FunctionEntryInstr* checked_entry = graph_entry->normal_entry();
   if (checked_entry != nullptr) {
-    flow_graph->CodegenBlockOrder(true)->Add(checked_entry);
+    flow_graph->CodegenBlockOrder()->Add(checked_entry);
   }
   // Build a new block order.  Emit each chain when its first block occurs
   // in the original reverse postorder ordering (which gives a topological
@@ -231,7 +231,7 @@ void BlockScheduler::ReorderBlocksJIT(FlowGraph* flow_graph) {
     if (chains[i]->first->block == flow_graph->postorder()[i]) {
       for (Link* link = chains[i]->first; link != nullptr; link = link->next) {
         if ((link->block != checked_entry) && (link->block != graph_entry)) {
-          flow_graph->CodegenBlockOrder(true)->Add(link->block);
+          flow_graph->CodegenBlockOrder()->Add(link->block);
         }
       }
     }
@@ -241,10 +241,6 @@ void BlockScheduler::ReorderBlocksJIT(FlowGraph* flow_graph) {
 // Moves blocks ending in a throw/rethrow, as well as any block post-dominated
 // by such a throwing block, to the end.
 void BlockScheduler::ReorderBlocksAOT(FlowGraph* flow_graph) {
-  if (!FLAG_reorder_basic_blocks) {
-    return;
-  }
-
   auto& reverse_postorder = flow_graph->reverse_postorder();
   const intptr_t block_count = reverse_postorder.length();
   GrowableArray<bool> is_terminating(block_count);
@@ -283,7 +279,7 @@ void BlockScheduler::ReorderBlocksAOT(FlowGraph* flow_graph) {
 
   // Emit code in reverse postorder but move any throwing blocks (except the
   // function entry, which needs to come first) to the very end.
-  auto codegen_order = flow_graph->CodegenBlockOrder(true);
+  auto codegen_order = flow_graph->CodegenBlockOrder();
   for (intptr_t i = 0; i < block_count; ++i) {
     auto block = reverse_postorder[i];
     const intptr_t preorder_nr = block->preorder_number();

runtime/vm/compiler/backend/flow_graph.cc

@@ -34,10 +34,17 @@ DEFINE_FLAG(bool, prune_dead_locals, true, "optimize dead locals away");
 // Quick access to the current zone.
 #define Z (zone())
 
+static bool ShouldReorderBlocks(const Function& function,
+                                FlowGraph::CompilationMode mode) {
+  return (mode == FlowGraph::CompilationMode::kOptimized) &&
+         FLAG_reorder_basic_blocks && !function.IsFfiCallbackTrampoline();
+}
+
 FlowGraph::FlowGraph(const ParsedFunction& parsed_function,
                      GraphEntryInstr* graph_entry,
                      intptr_t max_block_id,
-                     PrologueInfo prologue_info)
+                     PrologueInfo prologue_info,
+                     CompilationMode compilation_mode)
     : thread_(Thread::Current()),
       parent_(),
       current_ssa_temp_index_(0),
@@ -56,6 +63,8 @@ FlowGraph::FlowGraph(const ParsedFunction& parsed_function,
       constant_null_(nullptr),
       constant_dead_(nullptr),
       licm_allowed_(true),
+      should_reorder_blocks_(
+          ShouldReorderBlocks(parsed_function.function(), compilation_mode)),
       prologue_info_(prologue_info),
       loop_hierarchy_(nullptr),
       loop_invariant_loads_(nullptr),
@@ -152,16 +161,14 @@ void FlowGraph::ReplaceCurrentInstruction(ForwardInstructionIterator* iterator,
   iterator->RemoveCurrentFromGraph();
 }
 
-bool FlowGraph::ShouldReorderBlocks(const Function& function,
-                                    bool is_optimized) {
-  return is_optimized && FLAG_reorder_basic_blocks &&
-         !function.is_intrinsic() && !function.IsFfiCallbackTrampoline();
+GrowableArray<BlockEntryInstr*>* FlowGraph::CodegenBlockOrder() {
+  return should_reorder_blocks() ? &optimized_block_order_
+                                 : &reverse_postorder_;
 }
 
-GrowableArray<BlockEntryInstr*>* FlowGraph::CodegenBlockOrder(
-    bool is_optimized) {
-  return ShouldReorderBlocks(function(), is_optimized) ? &optimized_block_order_
-                                                       : &reverse_postorder_;
+const GrowableArray<BlockEntryInstr*>* FlowGraph::CodegenBlockOrder() const {
+  return should_reorder_blocks() ? &optimized_block_order_
+                                 : &reverse_postorder_;
 }
 
 ConstantInstr* FlowGraph::GetExistingConstant(

runtime/vm/compiler/backend/flow_graph.h

@@ -113,10 +113,17 @@ struct PrologueInfo {
 // Class to encapsulate the construction and manipulation of the flow graph.
 class FlowGraph : public ZoneAllocated {
  public:
+  enum class CompilationMode {
+    kUnoptimized,
+    kOptimized,
+    kIntrinsic,
+  };
+
   FlowGraph(const ParsedFunction& parsed_function,
             GraphEntryInstr* graph_entry,
             intptr_t max_block_id,
-            PrologueInfo prologue_info);
+            PrologueInfo prologue_info,
+            CompilationMode compilation_mode);
 
   // Function properties.
   const ParsedFunction& parsed_function() const { return parsed_function_; }
@@ -203,8 +210,8 @@ class FlowGraph : public ZoneAllocated {
   const GrowableArray<BlockEntryInstr*>& optimized_block_order() const {
     return optimized_block_order_;
   }
-  static bool ShouldReorderBlocks(const Function& function, bool is_optimized);
-  GrowableArray<BlockEntryInstr*>* CodegenBlockOrder(bool is_optimized);
+  GrowableArray<BlockEntryInstr*>* CodegenBlockOrder();
+  const GrowableArray<BlockEntryInstr*>* CodegenBlockOrder() const;
 
   // Iterators.
   BlockIterator reverse_postorder_iterator() const {
@@ -491,6 +498,8 @@ class FlowGraph : public ZoneAllocated {
     return compiler_pass_filters_;
   }
 
+  bool should_reorder_blocks() const { return should_reorder_blocks_; }
+
   //
   // High-level utilities.
   //
@@ -562,6 +571,11 @@ class FlowGraph : public ZoneAllocated {
   static intptr_t ComputeArgumentsSizeInWords(const Function& function,
                                               intptr_t arguments_count);
 
+  static constexpr CompilationMode CompilationModeFrom(bool is_optimizing) {
+    return is_optimizing ? CompilationMode::kOptimized
+                         : CompilationMode::kUnoptimized;
+  }
+
  private:
   friend class FlowGraphCompiler;  // TODO(ajcbik): restructure
   friend class FlowGraphChecker;
@@ -678,6 +692,7 @@ class FlowGraph : public ZoneAllocated {
   bool licm_allowed_;
   bool unmatched_representations_allowed_ = true;
   bool huge_method_ = false;
+  const bool should_reorder_blocks_;
 
   const PrologueInfo prologue_info_;
 

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -142,7 +142,7 @@ FlowGraphCompiler::FlowGraphCompiler(
       assembler_(assembler),
       parsed_function_(parsed_function),
       flow_graph_(*flow_graph),
-      block_order_(*flow_graph->CodegenBlockOrder(is_optimizing)),
+      block_order_(*flow_graph->CodegenBlockOrder()),
       current_block_(nullptr),
       exception_handlers_list_(nullptr),
       pc_descriptors_list_(nullptr),

runtime/vm/compiler/backend/il_serializer.cc

@@ -743,8 +743,9 @@ FlowGraph* FlowGraphDeserializer::ReadFlowGraph() {
     instr->ReadExtra(this);
   }
 
-  FlowGraph* flow_graph = new (Z)
-      FlowGraph(parsed_function(), graph_entry_, max_block_id, prologue_info);
+  FlowGraph* flow_graph =
+      new (Z) FlowGraph(parsed_function(), graph_entry_, max_block_id,
+                        prologue_info, FlowGraph::CompilationMode::kOptimized);
   flow_graph->set_current_ssa_temp_index(current_ssa_temp_index);
   flow_graph->CreateCommonConstants();
   flow_graph->disallow_licm();
@@ -754,7 +755,7 @@ FlowGraph* FlowGraphDeserializer::ReadFlowGraph() {
   {
     const intptr_t num_blocks = Read<intptr_t>();
     if (num_blocks != 0) {
-      auto* codegen_block_order = flow_graph->CodegenBlockOrder(true);
+      auto* codegen_block_order = flow_graph->CodegenBlockOrder();
       ASSERT(codegen_block_order == &flow_graph->optimized_block_order());
       for (intptr_t i = 0; i < num_blocks; ++i) {
         codegen_block_order->Add(ReadRef<BlockEntryInstr*>());

runtime/vm/compiler/backend/il_test_helper.cc

@@ -140,15 +140,12 @@ FlowGraph* TestPipeline::RunPasses(
     flow_graph_->PopulateWithICData(function_);
   }
 
-  const bool reorder_blocks =
-      FlowGraph::ShouldReorderBlocks(function_, optimized);
-  if (mode_ == CompilerPass::kJIT && reorder_blocks) {
+  if (mode_ == CompilerPass::kJIT && flow_graph_->should_reorder_blocks()) {
     BlockScheduler::AssignEdgeWeights(flow_graph_);
   }
 
   pass_state_ =
       new CompilerPassState(thread, flow_graph_, speculative_policy_.get());
-  pass_state_->reorder_blocks = reorder_blocks;
 
   if (optimized) {
     JitCallSpecializer jit_call_specializer(flow_graph_,

runtime/vm/compiler/backend/il_test_helper.h

@@ -272,7 +272,9 @@ class FlowGraphBuilderHelper {
  public:
   explicit FlowGraphBuilderHelper(intptr_t num_parameters = 0)
       : state_(CompilerState::Current()),
-        flow_graph_(MakeDummyGraph(Thread::Current(), num_parameters)) {
+        flow_graph_(MakeDummyGraph(Thread::Current(),
+                                   num_parameters,
+                                   state_.is_optimizing())) {
     flow_graph_.CreateCommonConstants();
   }
 
@@ -375,7 +377,9 @@ class FlowGraphBuilderHelper {
   FlowGraph* flow_graph() { return &flow_graph_; }
 
  private:
-  static FlowGraph& MakeDummyGraph(Thread* thread, intptr_t num_parameters) {
+  static FlowGraph& MakeDummyGraph(Thread* thread,
+                                   intptr_t num_parameters,
+                                   bool is_optimizing) {
     const FunctionType& signature =
         FunctionType::ZoneHandle(FunctionType::New());
     signature.set_num_fixed_parameters(num_parameters);
@@ -403,7 +407,8 @@ class FlowGraphBuilderHelper {
         new FunctionEntryInstr(graph_entry, block_id, kInvalidTryIndex,
                                CompilerState::Current().GetNextDeoptId()));
     return *new FlowGraph(*parsed_function, graph_entry, block_id,
-                          PrologueInfo{-1, -1});
+                          PrologueInfo{-1, -1},
+                          FlowGraph::CompilationModeFrom(is_optimizing));
   }
 
   CompilerState& state_;

runtime/vm/compiler/compiler_pass.cc

@@ -60,7 +60,6 @@ CompilerPassState::CompilerPassState(
       sinking(nullptr),
       call_specializer(nullptr),
       speculative_policy(speculative_policy),
-      reorder_blocks(false),
       sticky_flags(0),
       flow_graph_(flow_graph) {
   // Top scope function is at inlining id 0.
@@ -571,9 +570,7 @@ COMPILER_PASS(AllocateRegistersForGraphIntrinsic, {
 });
 
 COMPILER_PASS(ReorderBlocks, {
-  if (state->reorder_blocks) {
-    BlockScheduler::ReorderBlocks(flow_graph);
-  }
+  BlockScheduler::ReorderBlocks(flow_graph);
 
   // This is the last compiler pass.
   // Test that round-trip IL serialization works before generating code.

runtime/vm/compiler/compiler_pass.h

@@ -94,8 +94,6 @@ struct CompilerPassState {
 
   SpeculativeInliningPolicy* speculative_policy;
 
-  bool reorder_blocks;
-
   intptr_t sticky_flags;
 
   FlowGraphCompiler* graph_compiler = nullptr;

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

@@ -69,8 +69,10 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFieldInitializer() {
   if (B->IsCompiledForOsr()) {
     B->graph_entry_->RelinkToOsrEntry(Z, B->last_used_block_id_ + 1);
   }
-  return new (Z) FlowGraph(*parsed_function(), B->graph_entry_,
-                           B->last_used_block_id_, prologue_info);
+  return new (Z) FlowGraph(
+      *parsed_function(), B->graph_entry_, B->last_used_block_id_,
+      prologue_info,
+      FlowGraph::CompilationModeFrom(flow_graph_builder_->optimizing_));
 }
 
 void StreamingFlowGraphBuilder::SetupDefaultParameterValues() {
@@ -892,9 +894,10 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFunction(
     graph_entry->RelinkToOsrEntry(Z,
                                   flow_graph_builder_->last_used_block_id_ + 1);
   }
-  return new (Z)
-      FlowGraph(*parsed_function(), graph_entry,
-                flow_graph_builder_->last_used_block_id_, prologue_info);
+  return new (Z) FlowGraph(
+      *parsed_function(), graph_entry, flow_graph_builder_->last_used_block_id_,
+      prologue_info,
+      FlowGraph::CompilationModeFrom(flow_graph_builder_->optimizing_));
 }
 
 FlowGraph* StreamingFlowGraphBuilder::BuildGraph() {