[vm,compiler] Cleanup catch entries from GraphEntry and fix IL serialization

After https://dart-review.googlesource.com/c/sdk/+/356311,
GraphEntryInstr::catch_entries() is no longer needed and can be replaced
with FlowGraph::try_entries().

This change also fixes crash during IL deserialization if flow graph
contains a catch block.

TEST=vm/dart/checked_parameter_assert_assignable_stacktrace_test

Change-Id: Iad398e142bbd00110457e3b121bd446c32a5ac46
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401860
Commit-Queue: Alexander Markov <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>

Author: alexmarkov

runtime/tests/vm/dart/checked_parameter_assert_assignable_stacktrace_test.dart

@@ -7,6 +7,7 @@
 //
 // VMOptions=--save-debugging-info=$TEST_COMPILATION_DIR/debug.so
 // VMOptions=--dwarf-stack-traces --save-debugging-info=$TEST_COMPILATION_DIR/debug.so
+// VMOptions=--test_il_serialization --save-debugging-info=$TEST_COMPILATION_DIR/debug.so
 
 import 'awaiter_stacks/harness.dart' as harness;
 

runtime/vm/compiler/backend/flow_graph.cc

@@ -1371,10 +1371,8 @@ void FlowGraph::Rename(GrowableArray<PhiInstr*>* live_phis,
   env.FillWith(constant_dead(), 0, num_direct_parameters());
   env.FillWith(constant_null(), num_direct_parameters(), num_stack_locals());
 
-  if (entry->catch_entries().is_empty()) {
-    ASSERT(entry->unchecked_entry() != nullptr ? entry->SuccessorCount() == 2
-                                               : entry->SuccessorCount() == 1);
-  }
+  ASSERT(entry->unchecked_entry() != nullptr ? entry->SuccessorCount() == 2
+                                             : entry->SuccessorCount() == 1);
 
   // For OSR on a non-empty stack, insert synthetic phis on every joining entry.
   // These phis are synthetic since they are not driven by live variable
@@ -1895,7 +1893,7 @@ void FlowGraph::ValidatePhis() {
 void FlowGraph::RemoveDeadPhis(GrowableArray<PhiInstr*>* live_phis) {
   // Augment live_phis with those that have implicit real used at
   // potentially throwing instructions if there is a try-catch in this graph.
-  if (!graph_entry()->catch_entries().is_empty()) {
+  if (!try_entries().is_empty()) {
     for (BlockIterator it(postorder_iterator()); !it.Done(); it.Advance()) {
       JoinEntryInstr* join = it.Current()->AsJoinEntry();
       if (join == nullptr) continue;

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -429,7 +429,7 @@ void FlowGraphCompiler::RecordCatchEntryMoves(Environment* env) {
   if (is_optimizing() && env != nullptr && (try_index != kInvalidTryIndex)) {
     env = env->Outermost();
     CatchBlockEntryInstr* catch_block =
-        flow_graph().graph_entry()->GetCatchEntry(try_index);
+        flow_graph().GetCatchBlockByTryIndex(try_index);
     const GrowableArray<Definition*>* idefs =
         catch_block->initial_definitions();
     catch_entry_moves_maps_builder_->NewMapping(assembler()->CodeSize());

runtime/vm/compiler/backend/il.cc

@@ -1244,7 +1244,6 @@ GraphEntryInstr::GraphEntryInstr(const ParsedFunction& parsed_function,
                                 deopt_id,
                                 /*stack_depth*/ 0),
       parsed_function_(parsed_function),
-      catch_entries_(),
       indirect_entries_(),
       osr_id_(osr_id),
       entry_count_(0),
@@ -1261,15 +1260,6 @@ ConstantInstr* GraphEntryInstr::constant_null() {
   return nullptr;
 }
 
-CatchBlockEntryInstr* GraphEntryInstr::GetCatchEntry(intptr_t index) {
-  // TODO(fschneider): Sort the catch entries by catch_try_index to avoid
-  // searching.
-  for (intptr_t i = 0; i < catch_entries_.length(); ++i) {
-    if (catch_entries_[i]->catch_try_index() == index) return catch_entries_[i];
-  }
-  return nullptr;
-}
-
 bool GraphEntryInstr::IsCompiledForOsr() const {
   return osr_id_ != Compiler::kNoOSRDeoptId;
 }

runtime/vm/compiler/backend/il.h

@@ -1989,10 +1989,6 @@ class GraphEntryInstr : public BlockEntryWithInitialDefs {
   virtual intptr_t SuccessorCount() const;
   virtual BlockEntryInstr* SuccessorAt(intptr_t index) const;
 
-  void AddCatchEntry(CatchBlockEntryInstr* entry) { catch_entries_.Add(entry); }
-
-  CatchBlockEntryInstr* GetCatchEntry(intptr_t index);
-
   void AddIndirectEntry(IndirectEntryInstr* entry) {
     indirect_entries_.Add(entry);
   }
@@ -2037,17 +2033,11 @@ class GraphEntryInstr : public BlockEntryWithInitialDefs {
 
   const ParsedFunction& parsed_function() const { return parsed_function_; }
 
-  const GrowableArray<CatchBlockEntryInstr*>& catch_entries() const {
-    return catch_entries_;
-  }
-
   const GrowableArray<IndirectEntryInstr*>& indirect_entries() const {
     return indirect_entries_;
   }
 
-  bool HasSingleEntryPoint() const {
-    return catch_entries().is_empty() && unchecked_entry() == nullptr;
-  }
+  bool HasSingleEntryPoint() const { return unchecked_entry() == nullptr; }
 
   PRINT_BLOCK_HEADER_TO_SUPPORT
   DECLARE_CUSTOM_SERIALIZATION(GraphEntryInstr)
@@ -2065,7 +2055,6 @@ class GraphEntryInstr : public BlockEntryWithInitialDefs {
   FunctionEntryInstr* normal_entry_ = nullptr;
   FunctionEntryInstr* unchecked_entry_ = nullptr;
   OsrEntryInstr* osr_entry_ = nullptr;
-  GrowableArray<CatchBlockEntryInstr*> catch_entries_;
   // Indirect targets are blocks reachable only through indirect gotos.
   GrowableArray<IndirectEntryInstr*> indirect_entries_;
   const intptr_t osr_id_;

runtime/vm/compiler/backend/il_serializer.cc

@@ -988,7 +988,6 @@ void GraphEntryInstr::WriteExtra(FlowGraphSerializer* s) {
   s->WriteRef<FunctionEntryInstr*>(normal_entry_);
   s->WriteRef<FunctionEntryInstr*>(unchecked_entry_);
   s->WriteRef<OsrEntryInstr*>(osr_entry_);
-  s->WriteGrowableArrayOfRefs<CatchBlockEntryInstr*>(catch_entries_);
   s->WriteGrowableArrayOfRefs<IndirectEntryInstr*>(indirect_entries_);
 }
 
@@ -997,7 +996,6 @@ void GraphEntryInstr::ReadExtra(FlowGraphDeserializer* d) {
   normal_entry_ = d->ReadRef<FunctionEntryInstr*>();
   unchecked_entry_ = d->ReadRef<FunctionEntryInstr*>();
   osr_entry_ = d->ReadRef<OsrEntryInstr*>();
-  catch_entries_ = d->ReadGrowableArrayOfRefs<CatchBlockEntryInstr*>();
   indirect_entries_ = d->ReadGrowableArrayOfRefs<IndirectEntryInstr*>();
 }
 

runtime/vm/compiler/backend/linearscan.cc

@@ -640,7 +640,7 @@ void FlowGraphAllocator::BuildLiveRanges() {
 
     CatchBlockEntryInstr* surrounding_catch_block =
         block->InsideTryBlock()
-            ? flow_graph_.graph_entry()->GetCatchEntry(block->try_index())
+            ? flow_graph_.GetCatchBlockByTryIndex(block->try_index())
             : nullptr;
     // Now process all instructions in reverse order.
     while (current != block) {

runtime/vm/compiler/backend/redundancy_elimination.cc

@@ -4214,7 +4214,11 @@ class TryCatchAnalyzer : public ValueObject {
   // Assign sequential ids to each ParameterInstr in each CatchEntryBlock.
   // Collect reverse mapping from try indexes to corresponding catches.
   void NumberCatchEntryParameters() {
-    for (auto catch_entry : flow_graph_->graph_entry()->catch_entries()) {
+    for (TryEntryInstr* try_entry : flow_graph_->try_entries()) {
+      if (try_entry == nullptr) {
+        continue;
+      }
+      CatchBlockEntryInstr* const catch_entry = try_entry->catch_target();
       const GrowableArray<Definition*>& idefs =
           *catch_entry->initial_definitions();
       for (auto idef : idefs) {
@@ -4418,7 +4422,7 @@ class TryCatchAnalyzer : public ValueObject {
 };
 
 void OptimizeCatchEntryStates(FlowGraph* flow_graph, bool is_aot) {
-  if (flow_graph->graph_entry()->catch_entries().is_empty()) {
+  if (flow_graph->try_entries().is_empty()) {
     return;
   }
 

runtime/vm/compiler/backend/redundancy_elimination_test.cc

@@ -85,7 +85,7 @@ static void TryCatchOptimizerTest(
   // Finally run TryCatchAnalyzer on the graph (in AOT mode).
   OptimizeCatchEntryStates(graph, /*is_aot=*/true);
 
-  EXPECT_EQ(1, graph->graph_entry()->catch_entries().length());
+  EXPECT_EQ(1, graph->try_entries().length());
   auto scope = graph->parsed_function().scope();
 
   GrowableArray<LocalVariable*> env;
@@ -104,7 +104,7 @@ static void TryCatchOptimizerTest(
     }
   }
 
-  CatchBlockEntryInstr* catch_entry = graph->graph_entry()->catch_entries()[0];
+  CatchBlockEntryInstr* catch_entry = graph->GetCatchBlockByTryIndex(0);
 
   // We should only synchronize state for variables from the synchronized list.
   for (auto defn : *catch_entry->initial_definitions()) {

runtime/vm/compiler/compiler_pass.cc

@@ -491,7 +491,7 @@ COMPILER_PASS(DelayAllocations, { DelayAllocations::Optimize(flow_graph); });
 
 COMPILER_PASS(AllocationSinking_Sink, {
   // TODO(vegorov): Support allocation sinking with try-catch.
-  if (flow_graph->graph_entry()->catch_entries().is_empty()) {
+  if (flow_graph->try_entries().is_empty()) {
     state->sinking = new AllocationSinking(flow_graph);
     state->sinking->Optimize();
   }