[vm,compiler] Avoid passing anything except flow graph to code generation

alexmarkov · Commit Queue · commit 4c3ab230464c · 2024-12-11T06:50:40.000-08:00
This is a preparation to detach code generation from flow graph building and optimization passes. Extra inlining info (inline_id_to_function, inline_id_to_token_pos and caller_inline_id) is now added to FlowGraph and can be serialized and deserialized. ic_data_array is not needed for optimized compilation and is no longer passed. A separate CompilerPassState is created for code generation. TEST=ci (refactoring) Change-Id: Ib61af47c2ddde1353b9a0fe24995d29d6e85acad Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/399883 Commit-Queue: Alexander Markov <alexmarkov@google.com> Reviewed-by: Slava Egorov <vegorov@google.com>
diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc
@@ -344,9 +344,7 @@ class PrecompileParsedFunctionHelper : public ValueObject {
   ParsedFunction* parsed_function() const { return parsed_function_; }
   Thread* thread() const { return thread_; }
 
-  bool GenerateCode(FlowGraph* flow_graph,
-                    CompilerPassState* pass_state,
-                    ZoneGrowableArray<const ICData*>* ic_data_array);
+  bool GenerateCode(FlowGraph* flow_graph);
 
   void FinalizeCompilation(compiler::Assembler* assembler,
                            FlowGraphCompiler* graph_compiler,
@@ -3465,10 +3463,7 @@ static void GenerateNecessaryAllocationStubs(FlowGraph* flow_graph) {
   }
 }
 
-bool PrecompileParsedFunctionHelper::GenerateCode(
-    FlowGraph* flow_graph,
-    CompilerPassState* pass_state,
-    ZoneGrowableArray<const ICData*>* ic_data_array) {
+bool PrecompileParsedFunctionHelper::GenerateCode(FlowGraph* flow_graph) {
   // We may reattempt compilation if the function needs to be assembled using
   // far branches on ARM. In the else branch of the setjmp call, done is set to
   // false, and use_far_branches is set to true if there is a longjmp from the
@@ -3508,10 +3503,11 @@ bool PrecompileParsedFunctionHelper::GenerateCode(
 
       FlowGraphCompiler graph_compiler(
           &assembler, flow_graph, *parsed_function(), /*is_optimizing=*/true,
-          pass_state->inline_id_to_function, pass_state->inline_id_to_token_pos,
-          pass_state->caller_inline_id, ic_data_array, function_stats);
-      pass_state->graph_compiler = &graph_compiler;
-      CompilerPass::GenerateCode(pass_state);
+          /*deopt_id_to_ic_data=*/nullptr, function_stats);
+
+      CompilerPassState pass_state(thread(), flow_graph, precompiler_);
+      pass_state.graph_compiler = &graph_compiler;
+      CompilerPass::GenerateCode(&pass_state);
       {
         COMPILER_TIMINGS_TIMER_SCOPE(thread(), FinalizeCode);
         TIMELINE_DURATION(thread(), CompilerVerbose, "FinalizeCompilation");
@@ -3595,7 +3591,6 @@ bool PrecompileParsedFunctionHelper::Compile() {
   HANDLESCOPE(thread());
 
   FlowGraph* flow_graph = nullptr;
-  ZoneGrowableArray<const ICData*>* ic_data_array = nullptr;
   const Function& function = parsed_function()->function();
   ASSERT(!function.IsIrregexpFunction());
   ASSERT(function.IsOptimizable());
@@ -3606,7 +3601,8 @@ bool PrecompileParsedFunctionHelper::Compile() {
   compiler_state.set_function(function);
 
   {
-    ic_data_array = new (zone) ZoneGrowableArray<const ICData*>();
+    ZoneGrowableArray<const ICData*>* ic_data_array =
+        new (zone) ZoneGrowableArray<const ICData*>();
 
     TIMELINE_DURATION(thread(), CompilerVerbose, "BuildFlowGraph");
     COMPILER_TIMINGS_TIMER_SCOPE(thread(), BuildGraph);
@@ -3621,20 +3617,16 @@ bool PrecompileParsedFunctionHelper::Compile() {
 
   flow_graph->PopulateWithICData(function);
 
-  CompilerPassState pass_state(thread(), flow_graph, precompiler_);
-
   {
     TIMELINE_DURATION(thread(), CompilerVerbose, "OptimizationPasses");
 
     AotCallSpecializer call_specializer(precompiler_, flow_graph);
+    CompilerPassState pass_state(thread(), flow_graph, precompiler_);
     pass_state.call_specializer = &call_specializer;
 
     flow_graph = CompilerPass::RunPipeline(CompilerPass::kAOT, &pass_state);
   }
 
-  ASSERT(pass_state.inline_id_to_function.length() ==
-         pass_state.caller_inline_id.length());
-
   ASSERT(precompiler_ != nullptr);
 
   // When generating code in bare instruction mode all code objects
@@ -3647,7 +3639,7 @@ bool PrecompileParsedFunctionHelper::Compile() {
   // failure to commit object pool into the global object pool.
   GenerateNecessaryAllocationStubs(flow_graph);
 
-  return GenerateCode(flow_graph, &pass_state, ic_data_array);
+  return GenerateCode(flow_graph);
 }
 
 void Precompiler::CompileFunction(Precompiler* precompiler,
diff --git a/runtime/vm/compiler/backend/flow_graph.cc b/runtime/vm/compiler/backend/flow_graph.cc
@@ -70,6 +70,7 @@ FlowGraph::FlowGraph(const ParsedFunction& parsed_function,
       loop_invariant_loads_(nullptr),
       captured_parameters_(new(zone()) BitVector(zone(), variable_count())),
       inlining_id_(-1),
+      inlining_info_(&parsed_function.function()),
       should_print_(false),
       should_omit_check_bounds_(
           dart::ShouldOmitCheckBoundsIn(parsed_function.function())) {
diff --git a/runtime/vm/compiler/backend/flow_graph.h b/runtime/vm/compiler/backend/flow_graph.h
@@ -118,6 +118,28 @@ struct PrologueInfo {
   }
 };
 
+struct InliningInfo {
+  // Maps inline_id_to_function[inline_id] -> function. Top scope
+  // function has inline_id 0. The map is populated by the inliner.
+  GrowableArray<const Function*> inline_id_to_function;
+  // Token position where inlining occurred.
+  GrowableArray<TokenPosition> inline_id_to_token_pos;
+  // For a given inlining-id(index) specifies the caller's inlining-id.
+  GrowableArray<intptr_t> caller_inline_id;
+
+  explicit InliningInfo(const Function* function) {
+    // Top scope function is at inlining id 0.
+    inline_id_to_function.Add(function);
+    // Top scope function has no caller (-1).
+    caller_inline_id.Add(-1);
+    // We do not add a token position for the top scope function to
+    // |inline_id_to_token_pos| because it is not (currently) inlined into
+    // another graph at a given token position. A side effect of this is that
+    // the length of |inline_id_to_function| and |caller_inline_id| is always
+    // larger than the length of |inline_id_to_token_pos| by one.
+  }
+};
+
 // Class to encapsulate the construction and manipulation of the flow graph.
 class FlowGraph : public ZoneAllocated {
  public:
@@ -476,6 +498,9 @@ class FlowGraph : public ZoneAllocated {
   intptr_t inlining_id() const { return inlining_id_; }
   void set_inlining_id(intptr_t value) { inlining_id_ = value; }
 
+  InliningInfo& inlining_info() { return inlining_info_; }
+  const InliningInfo& inlining_info() const { return inlining_info_; }
+
   // Returns true if any instructions were canonicalized away.
   bool Canonicalize();
 
@@ -745,7 +770,10 @@ class FlowGraph : public ZoneAllocated {
   DirectChainedHashMap<ConstantPoolTrait> constant_instr_pool_;
   BitVector* captured_parameters_;
 
+  // Inlining related fields.
   intptr_t inlining_id_;
+  InliningInfo inlining_info_;
+
   bool should_print_;
   const bool should_omit_check_bounds_;
   uint8_t* compiler_pass_filters_ = nullptr;
diff --git a/runtime/vm/compiler/backend/flow_graph_checker.h b/runtime/vm/compiler/backend/flow_graph_checker.h
@@ -34,11 +34,11 @@ class FlowGraphChecker : public FlowGraphVisitor {
   // Constructs graph checker. The checker uses some custom-made
   // visitation to perform additional checks, and uses the
   // FlowGraphVisitor structure for anything else.
-  FlowGraphChecker(FlowGraph* flow_graph,
-                   const GrowableArray<const Function*>& inline_id_to_function)
+  explicit FlowGraphChecker(FlowGraph* flow_graph)
       : FlowGraphVisitor(flow_graph->preorder()),
         flow_graph_(flow_graph),
-        inline_id_to_function_(inline_id_to_function),
+        inline_id_to_function_(
+            flow_graph->inlining_info().inline_id_to_function),
         script_(Script::Handle(flow_graph_->zone())),
         current_block_(nullptr) {}
 
diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.cc b/runtime/vm/compiler/backend/flow_graph_compiler.cc
@@ -137,9 +137,6 @@ FlowGraphCompiler::FlowGraphCompiler(
     FlowGraph* flow_graph,
     const ParsedFunction& parsed_function,
     bool is_optimizing,
-    const GrowableArray<const Function*>& inline_id_to_function,
-    const GrowableArray<TokenPosition>& inline_id_to_token_pos,
-    const GrowableArray<intptr_t>& caller_inline_id,
     ZoneGrowableArray<const ICData*>* deopt_id_to_ic_data,
     CodeStatistics* stats /* = nullptr */)
     : thread_(Thread::Current()),
@@ -194,13 +191,18 @@ FlowGraphCompiler::FlowGraphCompiler(
 #else
   const bool stack_traces_only = false;
 #endif
+
+  const auto& inlining_info = flow_graph->inlining_info();
   // Make sure that the function is at the position for inline_id 0.
-  ASSERT(inline_id_to_function.length() >= 1);
-  ASSERT(inline_id_to_function[0]->ptr() ==
+  ASSERT(inlining_info.inline_id_to_function.length() >= 1);
+  ASSERT(inlining_info.inline_id_to_function[0]->ptr() ==
          flow_graph->parsed_function().function().ptr());
-  code_source_map_builder_ = new (zone_)
-      CodeSourceMapBuilder(zone_, stack_traces_only, caller_inline_id,
-                           inline_id_to_token_pos, inline_id_to_function);
+  ASSERT(inlining_info.inline_id_to_function.length() ==
+         inlining_info.caller_inline_id.length());
+  code_source_map_builder_ = new (zone_) CodeSourceMapBuilder(
+      zone_, stack_traces_only, inlining_info.caller_inline_id,
+      inlining_info.inline_id_to_token_pos,
+      inlining_info.inline_id_to_function);
 
   ArchSpecificInitialization();
 }
diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.h b/runtime/vm/compiler/backend/flow_graph_compiler.h
@@ -380,9 +380,6 @@ class FlowGraphCompiler : public ValueObject {
                     FlowGraph* flow_graph,
                     const ParsedFunction& parsed_function,
                     bool is_optimizing,
-                    const GrowableArray<const Function*>& inline_id_to_function,
-                    const GrowableArray<TokenPosition>& inline_id_to_token_pos,
-                    const GrowableArray<intptr_t>& caller_inline_id,
                     ZoneGrowableArray<const ICData*>* deopt_id_to_ic_data,
                     CodeStatistics* stats = nullptr);
 
diff --git a/runtime/vm/compiler/backend/il_serializer.cc b/runtime/vm/compiler/backend/il_serializer.cc
@@ -658,7 +658,6 @@ void FlowGraphSerializer::WriteFlowGraph(
 
   Write<intptr_t>(flow_graph.current_ssa_temp_index());
   Write<intptr_t>(flow_graph.max_block_id());
-  Write<intptr_t>(flow_graph.inlining_id());
   Write<const Array&>(flow_graph.coverage_array());
 
   PrologueInfo prologue_info = flow_graph.prologue_info();
@@ -709,12 +708,27 @@ void FlowGraphSerializer::WriteFlowGraph(
     }
     Write<GrowableArray<intptr_t>>(indices);
   }
+
+  Write<intptr_t>(flow_graph.inlining_id());
+
+  const InliningInfo& inlining_info = flow_graph.inlining_info();
+  Write<intptr_t>(inlining_info.inline_id_to_function.length());
+  ASSERT(inlining_info.inline_id_to_function.length() ==
+         inlining_info.caller_inline_id.length());
+  ASSERT(inlining_info.inline_id_to_function.length() ==
+         inlining_info.inline_id_to_token_pos.length() + 1);
+
+  for (intptr_t i = 1, n = inlining_info.inline_id_to_function.length(); i < n;
+       ++i) {
+    Write<const Function&>(*(inlining_info.inline_id_to_function[i]));
+    Write<intptr_t>(inlining_info.caller_inline_id[i]);
+    Write<TokenPosition>(inlining_info.inline_id_to_token_pos[i - 1]);
+  }
 }
 
 FlowGraph* FlowGraphDeserializer::ReadFlowGraph() {
   const intptr_t current_ssa_temp_index = Read<intptr_t>();
   const intptr_t max_block_id = Read<intptr_t>();
-  const intptr_t inlining_id = Read<intptr_t>();
   const Array& coverage_array = Read<const Array&>();
   const PrologueInfo prologue_info(Read<intptr_t>(), Read<intptr_t>());
 
@@ -750,7 +764,6 @@ FlowGraph* FlowGraphDeserializer::ReadFlowGraph() {
   flow_graph->set_current_ssa_temp_index(current_ssa_temp_index);
   flow_graph->CreateCommonConstants();
   flow_graph->disallow_licm();
-  flow_graph->set_inlining_id(inlining_id);
   flow_graph->set_coverage_array(coverage_array);
 
   {
@@ -771,6 +784,19 @@ FlowGraph* FlowGraphDeserializer::ReadFlowGraph() {
     }
   }
 
+  flow_graph->set_inlining_id(Read<intptr_t>());
+
+  auto& inlining_info = flow_graph->inlining_info();
+  const intptr_t inlining_info_len = Read<intptr_t>();
+  ASSERT(inlining_info.inline_id_to_function.length() == 1);
+  ASSERT(inlining_info.caller_inline_id.length() == 1);
+  ASSERT(inlining_info.inline_id_to_token_pos.length() == 0);
+  for (intptr_t i = 1; i < inlining_info_len; ++i) {
+    inlining_info.inline_id_to_function.Add(&Read<const Function&>());
+    inlining_info.caller_inline_id.Add(Read<intptr_t>());
+    inlining_info.inline_id_to_token_pos.Add(Read<TokenPosition>());
+  }
+
   return flow_graph;
 }
 
diff --git a/runtime/vm/compiler/backend/il_test_helper.cc b/runtime/vm/compiler/backend/il_test_helper.cc
@@ -218,14 +218,10 @@ void TestPipeline::CompileGraphAndAttachFunction() {
   const intptr_t far_branch_level = 1;
 #endif
 
-  ASSERT(pass_state_->inline_id_to_function.length() ==
-         pass_state_->caller_inline_id.length());
   compiler::ObjectPoolBuilder object_pool_builder;
   compiler::Assembler assembler(&object_pool_builder, far_branch_level);
-  FlowGraphCompiler graph_compiler(
-      &assembler, flow_graph_, *parsed_function_, optimized,
-      pass_state_->inline_id_to_function, pass_state_->inline_id_to_token_pos,
-      pass_state_->caller_inline_id, ic_data_array_);
+  FlowGraphCompiler graph_compiler(&assembler, flow_graph_, *parsed_function_,
+                                   optimized, ic_data_array_);
 
   graph_compiler.CompileGraph();
 
diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc
@@ -1324,12 +1324,7 @@ class CallSiteInliner : public ValueObject {
           callee_graph->set_current_ssa_temp_index(
               caller_graph_->current_ssa_temp_index());
 #if defined(DEBUG)
-          // The inlining IDs of instructions in the callee graph are unset
-          // until we call SetInliningID later.
-          GrowableArray<const Function*> callee_inline_id_to_function;
-          callee_inline_id_to_function.Add(&function);
-          FlowGraphChecker(callee_graph, callee_inline_id_to_function)
-              .Check("Builder (callee)");
+          FlowGraphChecker(callee_graph).Check("Builder (callee)");
 #endif
           CalleeGraphValidator::Validate(callee_graph);
         }
@@ -1406,12 +1401,7 @@ class CallSiteInliner : public ValueObject {
           COMPILER_TIMINGS_TIMER_SCOPE(thread(), ComputeSSA);
           callee_graph->ComputeSSA(param_stubs);
 #if defined(DEBUG)
-          // The inlining IDs of instructions in the callee graph are unset
-          // until we call SetInliningID later.
-          GrowableArray<const Function*> callee_inline_id_to_function;
-          callee_inline_id_to_function.Add(&function);
-          FlowGraphChecker(callee_graph, callee_inline_id_to_function)
-              .Check("SSA (callee)");
+          FlowGraphChecker(callee_graph).Check("SSA (callee)");
 #endif
         }
 
@@ -2387,16 +2377,14 @@ bool PolymorphicInliner::Inline() {
   return true;
 }
 
-FlowGraphInliner::FlowGraphInliner(
-    FlowGraph* flow_graph,
-    GrowableArray<const Function*>* inline_id_to_function,
-    GrowableArray<TokenPosition>* inline_id_to_token_pos,
-    GrowableArray<intptr_t>* caller_inline_id,
-    Precompiler* precompiler)
+FlowGraphInliner::FlowGraphInliner(FlowGraph* flow_graph,
+                                   Precompiler* precompiler)
     : flow_graph_(flow_graph),
-      inline_id_to_function_(inline_id_to_function),
-      inline_id_to_token_pos_(inline_id_to_token_pos),
-      caller_inline_id_(caller_inline_id),
+      inline_id_to_function_(
+          &(flow_graph->inlining_info().inline_id_to_function)),
+      inline_id_to_token_pos_(
+          &(flow_graph->inlining_info().inline_id_to_token_pos)),
+      caller_inline_id_(&(flow_graph->inlining_info().caller_inline_id)),
       trace_inlining_(FLAG_trace_inlining && flow_graph->should_print()),
       precompiler_(precompiler) {}
 
diff --git a/runtime/vm/compiler/backend/inliner.h b/runtime/vm/compiler/backend/inliner.h
@@ -32,11 +32,7 @@ class TargetEntryInstr;
 
 class FlowGraphInliner : ValueObject {
  public:
-  FlowGraphInliner(FlowGraph* flow_graph,
-                   GrowableArray<const Function*>* inline_id_to_function,
-                   GrowableArray<TokenPosition>* inline_id_to_token_pos,
-                   GrowableArray<intptr_t>* caller_inline_id,
-                   Precompiler* precompiler);
+  FlowGraphInliner(FlowGraph* flow_graph, Precompiler* precompiler);
 
   // The flow graph is destructively updated upon inlining.  Returns the max
   // depth that we inlined.
diff --git a/runtime/vm/compiler/compiler_pass.cc b/runtime/vm/compiler/compiler_pass.cc
@@ -60,17 +60,7 @@ CompilerPassState::CompilerPassState(Thread* thread,
       sinking(nullptr),
       call_specializer(nullptr),
       sticky_flags(0),
-      flow_graph_(flow_graph) {
-  // Top scope function is at inlining id 0.
-  inline_id_to_function.Add(&flow_graph->parsed_function().function());
-  // Top scope function has no caller (-1).
-  caller_inline_id.Add(-1);
-  // We do not add a token position for the top scope function to
-  // |inline_id_to_token_pos| because it is not (currently) inlined into
-  // another graph at a given token position. A side effect of this is that
-  // the length of |inline_id_to_function| and |caller_inline_id| is always
-  // larger than the length of |inline_id_to_token_pos| by one.
-}
+      flow_graph_(flow_graph) {}
 
 CompilerPass* CompilerPass::passes_[CompilerPass::kNumPasses] = {nullptr};
 uint8_t CompilerPass::flags_[CompilerPass::kNumPasses] = {0};
@@ -231,8 +221,7 @@ void CompilerPass::Run(CompilerPassState* state) const {
     PrintGraph(state, kTraceAfter, round);
 #if defined(DEBUG)
     if (CompilerState::Current().is_optimizing()) {
-      FlowGraphChecker(state->flow_graph(), state->inline_id_to_function)
-          .Check(name());
+      FlowGraphChecker(state->flow_graph()).Check(name());
     }
 #endif
     CompilerState::Current().set_current_pass(nullptr, nullptr);
@@ -392,9 +381,7 @@ COMPILER_PASS(SetOuterInliningId, {
 });
 
 COMPILER_PASS(Inlining, {
-  FlowGraphInliner inliner(flow_graph, &state->inline_id_to_function,
-                           &state->inline_id_to_token_pos,
-                           &state->caller_inline_id, state->precompiler);
+  FlowGraphInliner inliner(flow_graph, state->precompiler);
   state->inlining_depth = inliner.Inline();
 });
 
diff --git a/runtime/vm/compiler/compiler_pass.h b/runtime/vm/compiler/compiler_pass.h
diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc
